LIV Plot Tests

[sarthak-dv]

📝 Description

Type: 🚦 testing

Tests for the Last Interaction Velocity (LIV) Plot.
Add regression data to tardis-regression-data first: tardis-sn/tardis-regression-data#16

📌 Resources

Examples, notebooks, and links to useful references.

🚦 Testing

How did you test these changes?

• Testing pipeline
• Other method (describe)
• My changes can't be tested (explain why)

☑️ Checklist

• I requested two reviewers for this pull request
• I updated the documentation according to my changes
• I built the documentation by applying the build_docs label

Note: If you are not allowed to perform any of these actions, ping (@) a contributor.

[codecov]

Codecov Report

Attention: Patch coverage is 94.38202% with 10 lines in your changes missing coverage. Please review.

Project coverage is 70.74%. Comparing base (5fe6806) to head (4e49e28).
Report is 1 commits behind head on master.

Files	Patch %	Lines
tardis/visualization/tools/tests/test_liv_plot.py	95.06%	8 Missing ⚠️
tardis/visualization/tools/liv_plot.py	87.50%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2723      +/-   ##
==========================================
+ Coverage   69.88%   70.74%   +0.86%     
==========================================
  Files         208      209       +1     
  Lines       15453    15614     +161     
==========================================
+ Hits        10799    11046     +247     
+ Misses       4654     4568      -86     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[tardis-bot]

*beep* *bop*
Hi human,
I ran ruff on the latest commit (4e49e28).
Here are the outputs produced.
Results can also be downloaded as artifacts here.
Summarised output:

2	I001  	[*] Import block is un-sorted or un-formatted
2	D202  	[*] No blank lines allowed after function docstring (found 1)
1	ISC003	[ ] Explicitly concatenated string should be implicitly concatenated
1	RET506	[ ] Unnecessary `elif` after `raise` statement

Complete output(might be large):

tardis/visualization/tools/liv_plot.py:1:1: I001 [*] Import block is un-sorted or un-formatted
tardis/visualization/tools/liv_plot.py:25:9: D202 [*] No blank lines allowed after function docstring (found 1)
tardis/visualization/tools/liv_plot.py:48:9: D202 [*] No blank lines allowed after function docstring (found 1)
tardis/visualization/tools/liv_plot.py:341:13: RET506 Unnecessary `elif` after `raise` statement
tardis/visualization/tools/tests/test_liv_plot.py:1:1: I001 [*] Import block is un-sorted or un-formatted
tardis/visualization/tools/tests/test_liv_plot.py:352:29: ISC003 Explicitly concatenated string should be implicitly concatenated
Found 6 errors.
[*] 4 fixable with the `--fix` option.

[jaladh-singhal]

@sarthak-dv this looks like a good start. I noted that tests are failing because we need refdata PR to be merged (I've left comment there).

It seems that you're adapting test_sdec like you did when writing module. But sdec tests are using reference data approach. There's a new fixture regression_data which is what you need to use. It looks to me you might be needing sync_hdf_store() but you should assess how to use it - take a look at tests that are using it for inspiration.

[tardis-bot]

*beep* *bop*
Hi human,
I ran benchmarks as you asked comparing master (5fe6806) and the latest commit (4e49e28).
Here are the logs produced by ASV.
Results can also be downloaded as artifacts here.

Significantly changed benchmarks:

All benchmarks:

Benchmarks that have stayed the same:

| Change   | Before [5fe6806d] <master>   | After [4e49e284]    | Ratio   | Benchmark (Parameter)                                                                                                               |
|----------|------------------------------|---------------------|---------|-------------------------------------------------------------------------------------------------------------------------------------|
|          | 35.7±10μs                    | 46.2±20μs           | ~1.29   | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_generate_rpacket_last_interaction_tracker_list |
|          | 2.97±0.5ms                   | 3.39±0.5ms          | ~1.14   | transport_montecarlo_single_packet_loop.BenchmarkTransportMontecarloSinglePacketLoop.time_single_packet_loop                        |
|          | 681±200ns                    | 602±100ns           | ~0.88   | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_pair_creation_opacity_calculation                                |
|          | 2.07±0.03ms                  | 1.79±0.03ms         | ~0.87   | transport_montecarlo_main_loop.BenchmarkTransportMontecarloMontecarloMainLoop.time_montecarlo_main_loop                             |
|          | 8.67±2μs                     | 7.48±2μs            | ~0.86   | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket_volley                                    |
|          | 4.44±0.3ms                   | 3.74±0.05ms         | ~0.84   | opacities_opacity_state.BenchmarkOpacitiesOpacityState.time_opacity_state_initialize('macroatom')                                   |
|          | 3.92±0.3μs                   | 3.01±0.5μs          | ~0.77   | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_bad_vpacket                                       |
|          | 1.69±0.4μs                   | 1.25±0.4μs          | ~0.74   | transport_geometry_calculate_distances.BenchmarkTransportGeometryCalculateDistances.time_calculate_distance_line                    |
|          | 3.68±0.02ms                  | 2.69±0ms            | ~0.73   | opacities_opacity_state.BenchmarkOpacitiesOpacityState.time_opacity_state_initialize('scatter')                                     |
|          | 69.1±40μs                    | 46.7±30μs           | ~0.68   | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_line_scatter                                          |
|          | 6.76±0.8μs                   | 7.29±1μs            | 1.08    | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket                                           |
|          | 592±100ns                    | 601±100ns           | 1.02    | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_photoabsorption_opacity_calculation                              |
|          | 730±0.4ns                    | 743±0.8ns           | 1.02    | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_thomson_scatter                                       |
|          | 65.9±0.2ms                   | 67.1±0.4ms          | 1.02    | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_rpacket_trackers_to_dataframe                  |
|          | 203±0.2ns                    | 204±0.2ns           | 1.01    | spectrum_formal_integral.BenchmarkTransportMontecarloFormalIntegral.time_intensity_black_body                                       |
|          | 541±100ns                    | 541±100ns           | 1.00    | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_compton_opacity_calculation                                      |
|          | 40.8±0.2s                    | 40.7±0.02s          | 1.00    | run_tardis.BenchmarkRunTardis.time_run_tardis                                                                                       |
|          | 1.10±0m                      | 1.10±0m             | 1.00    | run_tardis.BenchmarkRunTardis.time_run_tardis_rpacket_tracking                                                                      |
|          | 1.21±0μs                     | 1.20±0μs            | 1.00    | transport_geometry_calculate_distances.BenchmarkTransportGeometryCalculateDistances.time_calculate_distance_boundary                |
|          | 30.7±0.01μs                  | 30.8±0.02μs         | 1.00    | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_generate_rpacket_tracker_list                  |
|          | 2.13±0.01m                   | 2.09±0m             | 0.98    | spectrum_formal_integral.BenchmarkTransportMontecarloFormalIntegral.time_FormalIntegrator_functions                                 |
|          | 45.5±30μs                    | 44.4±20μs           | 0.98    | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_line_emission                                         |
|          | 2.43±1μs                     | 2.35±1μs            | 0.97    | transport_montecarlo_estimators_radfield_estimator_calcs.BenchmarkMontecarloMontecarloNumbaPacket.time_update_line_estimators       |
|          | 3.66±0.3μs                   | 3.47±0.6μs          | 0.95    | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket_within_shell                              |

If you want to see the graph of the results, you can check it here

[andrewfullard]

Looks like the data that were merged were not identical to the test outputs.

[sarthak-dv]

Looks like the data that were merged were not identical to the test outputs.

Yeah, but if that is the case then macOS tests should also fail along with Ubuntu tests but they are passing.

[andrewfullard]

Please check black and docstr-cov

[andrewfullard]

Thank you for putting a lot of effort into this!

[jaladh-singhal]

Thank you so much for making this work, I know switching to regression data mid-way might be challenging for you @sarthak-dv.

Looks good to me! I see stardis-test failure is unrelated

[jaladh-singhal]

nice use of itertools

[jamesgillanders]

Fantastic work @sarthak-dv -- really impressed with the effort you've put into this!