Move and Rename SDECData to VisualizationData

[KasukabeDefenceForce]

📝 Description

Type: 🪲 bugfix | 🚀 feature | ☣️ breaking change | 🚦 testing | 📝 documentation | 🎢 infrastructure

This PR moves and shifts sdec data to VisualizationData for clarity purposes and also refactors some of its methods to improve readability.

📌 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.

[tardis-bot]

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

3	RET505	[ ] Unnecessary `else` after `return` statement
3	RET506	[ ] Unnecessary `elif` after `raise` statement
2	F401  	[*] `matplotlib.cm` imported but unused

Complete output(might be large):

tardis/visualization/plot_util.py:47:5: RET505 Unnecessary `else` after `return` statement
tardis/visualization/tools/liv_plot.py:4:25: F401 [*] `matplotlib.cm` imported but unused
tardis/visualization/tools/liv_plot.py:7:18: F401 [*] `pandas` imported but unused
tardis/visualization/tools/liv_plot.py:313:13: RET506 Unnecessary `elif` after `raise` statement
tardis/visualization/tools/sdec_plot.py:105:13: RET506 Unnecessary `else` after `raise` statement
tardis/visualization/tools/sdec_plot.py:273:13: RET506 Unnecessary `else` after `raise` statement
tardis/visualization/tools/visualization_data.py:213:9: RET505 Unnecessary `else` after `return` statement
tardis/visualization/tools/visualization_data.py:345:9: RET505 Unnecessary `else` after `return` statement
Found 8 errors.
[*] 2 fixable with the `--fix` option.

[codecov]

Codecov Report

Attention: Patch coverage is 65.88235% with 29 lines in your changes missing coverage. Please review.

Project coverage is 70.04%. Comparing base (7e37d5f) to head (9b996fb).

Files with missing lines	Patch %	Lines
tardis/visualization/tools/visualization_data.py	67.24%	19 Missing ⚠️
tardis/visualization/plot_util.py	46.66%	8 Missing ⚠️
tardis/visualization/tools/liv_plot.py	90.00%	1 Missing ⚠️
tardis/visualization/tools/sdec_plot.py	50.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2928      +/-   ##
==========================================
- Coverage   70.39%   70.04%   -0.35%     
==========================================
  Files         221      222       +1     
  Lines       16147    16165      +18     
==========================================
- Hits        11366    11323      -43     
- Misses       4781     4842      +61     

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

[tardis-bot]

*beep* *bop*
Hi human,
I ran benchmarks as you asked comparing master (7e37d5f) and the latest commit (b71e734).
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 [7e37d5fe] <master>   | After [b71e734c]    | Ratio   | Benchmark (Parameter)                                                                                                               |
|----------|------------------------------|---------------------|---------|-------------------------------------------------------------------------------------------------------------------------------------|
|          | 1.91±5μs                     | 1.37±0.4μs          | ~0.72   | transport_geometry_calculate_distances.BenchmarkTransportGeometryCalculateDistances.time_calculate_distance_line                    |
|          | 511±200ns                    | 531±100ns           | 1.04    | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_compton_opacity_calculation                                      |
|          | 48.7±20μs                    | 50.7±30μs           | 1.04    | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_line_emission                                         |
|          | 46.1±20μs                    | 48.0±20μs           | 1.04    | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_line_scatter                                          |
|          | 2.73±0ms                     | 2.77±0.01ms         | 1.01    | opacities_opacity_state.BenchmarkOpacitiesOpacityState.time_opacity_state_initialize('scatter')                                     |
|          | 735±0.3ns                    | 742±1ns             | 1.01    | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_thomson_scatter                                       |
|          | 62.8±0.01ms                  | 63.1±0.2ms          | 1.01    | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_rpacket_trackers_to_dataframe                  |
|          | 2.08±0m                      | 2.08±0m             | 1.00    | spectrum_formal_integral.BenchmarkTransportMontecarloFormalIntegral.time_FormalIntegrator_functions                                 |
|          | 37.8±0.04μs                  | 37.7±0.01μs         | 1.00    | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_generate_rpacket_tracker_list                  |
|          | 7.88±2μs                     | 7.91±2μs            | 1.00    | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket_volley                                    |
|          | 3.73±0.02ms                  | 3.71±0.01ms         | 0.99    | opacities_opacity_state.BenchmarkOpacitiesOpacityState.time_opacity_state_initialize('macroatom')                                   |
|          | 206±0.2ns                    | 203±0.1ns           | 0.99    | spectrum_formal_integral.BenchmarkTransportMontecarloFormalIntegral.time_intensity_black_body                                       |
|          | 571±100ns                    | 561±200ns           | 0.98    | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_photoabsorption_opacity_calculation                              |
|          | 39.2±0.03s                   | 38.4±0s             | 0.98    | run_tardis.BenchmarkRunTardis.time_run_tardis                                                                                       |
|          | 1.06±0m                      | 1.04±0m             | 0.98    | run_tardis.BenchmarkRunTardis.time_run_tardis_rpacket_tracking                                                                      |
|          | 1.22±0μs                     | 1.18±0μs            | 0.97    | transport_geometry_calculate_distances.BenchmarkTransportGeometryCalculateDistances.time_calculate_distance_boundary                |
|          | 2.53±0.4ms                   | 2.45±0.5ms          | 0.97    | transport_montecarlo_single_packet_loop.BenchmarkTransportMontecarloSinglePacketLoop.time_single_packet_loop                        |
|          | 23.2±6μs                     | 22.3±5μs            | 0.96    | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_generate_rpacket_last_interaction_tracker_list |
|          | 1.74±0.02ms                  | 1.66±0ms            | 0.95    | transport_montecarlo_main_loop.BenchmarkTransportMontecarloMontecarloMainLoop.time_montecarlo_main_loop                             |
|          | 3.10±0.6μs                   | 2.95±0.5μs          | 0.95    | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_bad_vpacket                                       |
|          | 6.33±1μs                     | 5.96±1μs            | 0.94    | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket                                           |
|          | 571±100ns                    | 532±200ns           | 0.93    | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_pair_creation_opacity_calculation                                |
|          | 2.05±2μs                     | 1.90±2μs            | 0.93    | transport_montecarlo_estimators_radfield_estimator_calcs.BenchmarkMontecarloMontecarloNumbaPacket.time_update_line_estimators       |
|          | 3.21±0.5μs                   | 2.98±0.5μs          | 0.93    | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket_within_shell                              |

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

[wkerzendorf]

Let's make a notebook showing these in actions. Each plot should be redone with plotly and Matplotlib to check how well that works.

[andrewfullard]

The class is duplicating a lot of information from the simulation. Could the spectrum, in particular, be separated and simply used via the spectrum class rather than copying the data into this new class? Similarly, the geometry information (v_inner, r_inner etc) is being duplicated. It may be more useful to duplicate the entire geometry class from the simulation if that is what is needed for visualization. If the geometry parts are NOT all needed, then referencing the simulation rather than duplicating the data would be best.

As far as the notebook goes, it would be most useful to demonstrate the packet tracking information that is contained in the class, since the spectrum information is already plotted elsewhere e.g. in the quickstart notebook.

Please comment etc with your ideas for any better ways to package the data necessary for the SDEC and LIV plots and how that might look. Multiple classes may be needed, but in general it would be good to limit duplication as I noted above.