Add the non-numba opacity_state as a attribute of the simulation object#3205
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds the non-numba opacity_state as an attribute of the simulation object to enable access to the DataFrame version of tau_sobolev with proper indexing (atomic number, ionization number, and level number). Currently, only the numba version is accessible through simulation.transport.transport_state.opacity_state, which is incompatible with the tardis.workflows.util.get_tau_integ function.
- Changes local variable
opacity_stateto instance attributeself.opacity_state - Updates all references to use the new instance attribute
- Enables external access to the non-numba opacity state for integration calculations
| @@ -422,7 +422,7 @@ def iterate(self, no_of_packets, no_of_virtual_packets=0): | |||
| self.plasma.beta_sobolev = None | |||
| macro_atom_state = None | |||
|
|
|||
There was a problem hiding this comment.
The new instance attribute opacity_state should be documented with a docstring or inline comment explaining its purpose, especially since it's intended for external use with get_tau_integ.
| # Stores the result of solving the opacity for the current plasma state. | |
| # This is used in subsequent transport state initialization and other processes. |
|
*beep* *bop* Hi, human. I'm the @tardis-bot and I noticed that your email is not associated with an ORCID ID in our database. Please add your email and ORCID ID to the If you don't have an ORCID ID yet, you can create one for free at orcid.org. ORCID IDs help ensure you get proper credit for your scientific contributions. The format should be: |
|
*beep* *bop* Details6 G004 logging-f-string
1 TRY300 try-consider-else
Found 7 errors.
Complete output(might be large): Detailstardis/simulation/base.py:245:17: G004 Logging statement uses f-string
tardis/simulation/base.py:418:13: G004 Logging statement uses f-string
tardis/simulation/base.py:519:13: G004 Logging statement uses f-string
tardis/simulation/base.py:599:13: G004 Logging statement uses f-string
tardis/simulation/base.py:604:13: G004 Logging statement uses f-string
tardis/simulation/base.py:655:13: TRY300 Consider moving this statement to an `else` block
tardis/simulation/base.py:657:26: G004 Logging statement uses f-string
Found 7 errors.
|
|
thanks - please make sure that there is an issue - just very briefly saying that there are various attributes - opacity, 2x opacity_state that are flying around not clearly marked |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3205 +/- ##
==========================================
- Coverage 69.89% 69.65% -0.24%
==========================================
Files 244 244
Lines 17324 17324
==========================================
- Hits 12108 12067 -41
- Misses 5216 5257 +41 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
*beep* *bop* Significantly changed benchmarks: DetailsAll benchmarks: DetailsBenchmarks that have stayed the same:
| Change | Before [fa4c4ea9] <master> | After [94ca86e2] | Ratio | Benchmark (Parameter) |
|----------|------------------------------|---------------------|---------|-------------------------------------------------------------------------------------------------------------------------------------|
| | 1.95±2μs | 2.26±3μs | ~1.16 | transport_montecarlo_estimators_radfield_estimator_calcs.BenchmarkMontecarloMontecarloNumbaPacket.time_update_line_estimators |
| | 2.92±0.4μs | 3.41±0.7μs | ~1.16 | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_bad_vpacket |
| | 39.9±20μs | 45.3±30μs | ~1.14 | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_line_emission |
| | 511±100ns | 571±200ns | ~1.12 | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_photoabsorption_opacity_calculation |
| | 52.2±20μs | 57.4±30μs | ~1.10 | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_line_scatter |
| | 2.03±6μs | 1.43±0.3μs | ~0.70 | transport_geometry_calculate_distances.BenchmarkTransportGeometryCalculateDistances.time_calculate_distance_line |
| | 5.85±1μs | 6.24±0.5μs | 1.07 | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket |
| | 6.85±2μs | 7.15±2μs | 1.04 | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket_volley |
| | 37.0±10μs | 38.1±10μs | 1.03 | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_generate_rpacket_last_interaction_tracker_list |
| | 191±0.04ns | 195±0.02ns | 1.02 | spectrum_formal_integral.BenchmarkTransportMontecarloFormalIntegral.time_intensity_black_body |
| | 675±2ns | 682±1ns | 1.01 | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_thomson_scatter |
| | 2.74±0.4ms | 2.77±0.4ms | 1.01 | transport_montecarlo_single_packet_loop.BenchmarkTransportMontecarloSinglePacketLoop.time_single_packet_loop |
| | 491±200ns | 491±200ns | 1.00 | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_compton_opacity_calculation |
| | 59.4±0.03s | 59.2±0.1s | 1.00 | run_tardis.BenchmarkRunTardis.time_run_tardis_rpacket_tracking |
| | 2.13±0m | 2.11±0m | 0.99 | spectrum_formal_integral.BenchmarkTransportMontecarloFormalIntegral.time_FormalIntegrator_functions |
| | 2.77±0.04ms | 2.74±0.03ms | 0.99 | transport_montecarlo_main_loop.BenchmarkTransportMontecarloMontecarloMainLoop.time_montecarlo_main_loop |
| | 39.1±0.08μs | 38.8±0.05μs | 0.99 | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_generate_rpacket_tracker_list |
| | 38.4±0.07s | 37.6±0.05s | 0.98 | run_tardis.BenchmarkRunTardis.time_run_tardis |
| | 2.85±0ms | 2.77±0.02ms | 0.97 | opacities_opacity_state.BenchmarkOpacitiesOpacityState.time_opacity_state_initialize('scatter') |
| | 3.87±0.01ms | 3.70±0.01ms | 0.96 | opacities_opacity_state.BenchmarkOpacitiesOpacityState.time_opacity_state_initialize('macroatom') |
| | 1.22±0μs | 1.17±0μs | 0.96 | transport_geometry_calculate_distances.BenchmarkTransportGeometryCalculateDistances.time_calculate_distance_boundary |
| | 60.9±0.2ms | 57.5±0.02ms | 0.94 | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_rpacket_trackers_to_dataframe |
| | 3.33±0.5μs | 3.09±0.4μs | 0.93 | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket_within_shell |
| | 611±100ns | 561±200ns | 0.92 | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_pair_creation_opacity_calculation |
If you want to see the graph of the results, you can check it here |
Closes tardis-sn#3206 Renames the opacity_state attribute in MonteCarloTransportState to opacity_state_numba to clearly distinguish it from the non-numba OpacityState DataFrame version (sim.opacity_state) added in PR tardis-sn#3205.
4 participants
When calculating the integrated tau using the
tardis.workflows.util.get_tau_integ, the opacity_state should be the non-numba version, in which the opacity_state.tau_sobolev is a dataframe with atomic number, ionization number and level number as indexes.However, currently the non-numba opacity_state is not a attribute of the simulation object, and the only opacity state we can access is the
simulation.transport.transport_state.opacity_state, which is a numba version.In this PR, the quick fix is to add the opacity_state as a attribute of the simulation class. The
simulation.transport.transport_state.opacity_stateshould be renamed to be more clear about this, see issue #3206