tardis-sn / tardis

TARDIS - Temperature And Radiative Diffusion In Supernovae
https://tardis-sn.github.io/tardis
201 stars 404 forks source link

Get line interaction #2780

Open Sumit112192 opened 1 month ago

Sumit112192 commented 1 month ago

:pencil: Description

Type: :rocket: feature

This PR builds up on PR #2736. Just like boundary interaction this PR introduces the line interaction data to the tracker.

Sumit112192 commented 1 month ago

I have to implement tests for this new functionality

tardis-bot commented 1 month ago

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

```diff 3 I001 [*] Import block is un-sorted or un-formatted 2 PIE790 [*] Unnecessary `pass` statement 2 D411 [*] Missing blank line before section ("Parameters") 2 UP004 [*] Class `RPacketLastInteractionTracker` inherits from `object` 1 B009 [*] Do not call `getattr` with a constant attribute value. It is not any safer than normal property access. ```

Complete output(might be large):

```diff tardis/transport/montecarlo/packet_trackers.py:1:1: I001 [*] Import block is un-sorted or un-formatted tardis/transport/montecarlo/packet_trackers.py:52:22: UP004 [*] Class `RPacketTracker` inherits from `object` tardis/transport/montecarlo/packet_trackers.py:55:5: D411 [*] Missing blank line before section ("Parameters") tardis/transport/montecarlo/packet_trackers.py:273:48: B009 [*] Do not call `getattr` with a constant attribute value. It is not any safer than normal property access. tardis/transport/montecarlo/packet_trackers.py:293:37: UP004 [*] Class `RPacketLastInteractionTracker` inherits from `object` tardis/transport/montecarlo/packet_trackers.py:296:5: D411 [*] Missing blank line before section ("Parameters") tardis/transport/montecarlo/packet_trackers.py:338:9: PIE790 [*] Unnecessary `pass` statement tardis/transport/montecarlo/packet_trackers.py:345:9: PIE790 [*] Unnecessary `pass` statement tardis/transport/montecarlo/single_packet_loop.py:1:1: I001 [*] Import block is un-sorted or un-formatted tardis/transport/montecarlo/tests/test_rpacket_tracker.py:1:1: I001 [*] Import block is un-sorted or un-formatted Found 10 errors. [*] 10 fixable with the `--fix` option. ```
Sumit112192 commented 1 month ago

@wkerzendorf @andrewfullard please review.

tardis-bot commented 1 month ago

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

Significantly changed benchmarks:

```diff | Change | Before [18dcec87] | After [ac8189dc] | Ratio | Benchmark (Parameter) | |----------|------------------------------|---------------------|---------|-----------------------------------------------------------------------------------------------------------------------------| | + | 48.3±0.02μs | 65.1±0.02μs | 1.35 | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_generate_rpacket_tracker_list(100, 50) | | + | 7.53±0.01μs | 9.32±0.01μs | 1.24 | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_generate_rpacket_tracker_list(10, 50) | ```

All benchmarks:

```diff Benchmarks that have stayed the same: | Change | Before [18dcec87] | After [ac8189dc] | Ratio | Benchmark (Parameter) | |----------|------------------------------|---------------------|---------|----------------------------------------------------------------------------------------------------------------------------------------------| | | 52.8±0.2μs | 67.0±0.02μs | ~1.27 | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_generate_rpacket_tracker_list(100, 10) | | | 7.23±0.01μs | 8.57±0.04μs | ~1.19 | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_generate_rpacket_tracker_list(10, 10) | | | 18.1±5μs | 21.0±6μs | ~1.16 | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_generate_rpacket_last_interaction_tracker_list(10, 10) | | | 521±100ns | 581±200ns | ~1.12 | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_compton_opacity_calculation | | | 3.44±1μs | 3.85±0.6μs | ~1.12 | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket_within_shell | | | 19.7±5μs | 21.9±7μs | ~1.11 | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_generate_rpacket_last_interaction_tracker_list(10, 50) | | | 2.87±0.01ms | 2.54±0ms | ~0.89 | opacities_opacity_state.BenchmarkOpacitiesOpacityState.time_opacity_state_initialize('scatter') | | | 1.62±0.4μs | 1.38±0.3μs | ~0.85 | transport_geometry_calculate_distances.BenchmarkTransportGeometryCalculateDistances.time_calculate_distance_line | | | 1.71±0.01ms | 1.88±0.01ms | 1.09 | transport_montecarlo_main_loop.BenchmarkTransportMontecarloMontecarloMainLoop.time_montecarlo_main_loop | | | 61.6±0.2ms | 67.0±0.1ms | 1.09 | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_rpacket_trackers_to_dataframe(100, 10) | | | 612±100ns | 651±100ns | 1.06 | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_photoabsorption_opacity_calculation | | | 1.02±0m | 1.08±0m | 1.06 | run_tardis.BenchmarkRunTardis.time_run_tardis_rpacket_tracking | | | 46.5±20μs | 49.2±30μs | 1.06 | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_line_scatter | | | 42.9±20μs | 44.8±30μs | 1.05 | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_line_emission | | | 2.96±0.6ms | 3.08±0.6ms | 1.04 | transport_montecarlo_single_packet_loop.BenchmarkTransportMontecarloSinglePacketLoop.time_single_packet_loop | | | 6.48±1μs | 6.77±0.8μs | 1.04 | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket | | | 63.8±0.2ms | 65.9±0.2ms | 1.03 | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_rpacket_trackers_to_dataframe(10, 50) | | | 551±200ns | 561±200ns | 1.02 | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_pair_creation_opacity_calculation | | | 63.7±0.05ms | 64.9±0.1ms | 1.02 | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_rpacket_trackers_to_dataframe(10, 10) | | | 62.8±0.2ms | 64.3±0.1ms | 1.02 | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_rpacket_trackers_to_dataframe(100, 50) | | | 39.6±0.04s | 40.0±0.02s | 1.01 | run_tardis.BenchmarkRunTardis.time_run_tardis | | | 2.07±0m | 2.07±0m | 1.00 | spectrum_formal_integral.BenchmarkTransportMontecarloFormalIntegral.time_FormalIntegrator_functions | | | 203±0.3ns | 202±0.01ns | 1.00 | spectrum_formal_integral.BenchmarkTransportMontecarloFormalIntegral.time_intensity_black_body | | | 1.20±0μs | 1.17±0μs | 0.98 | transport_geometry_calculate_distances.BenchmarkTransportGeometryCalculateDistances.time_calculate_distance_boundary | | | 742±4ns | 729±0.9ns | 0.98 | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_thomson_scatter | | | 2.90±0.9μs | 2.85±0.8μs | 0.98 | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_bad_vpacket | | | 7.98±2μs | 7.64±2μs | 0.96 | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket_volley | | | 25.3±5μs | 24.1±4μs | 0.95 | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_generate_rpacket_last_interaction_tracker_list(100, 50) | | | 2.15±2μs | 2.03±1μs | 0.94 | transport_montecarlo_estimators_radfield_estimator_calcs.BenchmarkMontecarloMontecarloNumbaPacket.time_update_line_estimators | | | 26.1±5μs | 24.2±4μs | 0.93 | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_generate_rpacket_last_interaction_tracker_list(100, 10) | | | 3.88±0ms | 3.56±0ms | 0.92 | opacities_opacity_state.BenchmarkOpacitiesOpacityState.time_opacity_state_initialize('macroatom') | Benchmarks that have got worse: | Change | Before [18dcec87] | After [ac8189dc] | Ratio | Benchmark (Parameter) | |----------|------------------------------|---------------------|---------|-----------------------------------------------------------------------------------------------------------------------------| | + | 48.3±0.02μs | 65.1±0.02μs | 1.35 | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_generate_rpacket_tracker_list(100, 50) | | + | 7.53±0.01μs | 9.32±0.01μs | 1.24 | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_generate_rpacket_tracker_list(10, 50) | ```

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

Sumit112192 commented 1 month ago

@andrewfullard Can you look at the segregation of line interaction as before line interaction and after line interaction in the track_line_interaction function and suggest some changes or comments?

andrewfullard commented 1 month ago

@andrewfullard Can you look at the segregation of line interaction as before line interaction and after line interaction in the track_line_interaction function and suggest some changes or comments?

Given that you need to detect the line interaction before and after the interaction process, I'm not sure what else can be done apart from essentially duplicating the method instead of having the if else switch.

Sumit112192 commented 1 month ago

The reason rpacket_tracker's boundary_interaction tests are failing because both line_interactions and boundary_interactions have a common field event_id more specifically the interaction number. Since, the line_interactions are added, the event_id of boundary_interactions are changed. So, I have to update the regression data for the boundary_interaction.