-
-
Notifications
You must be signed in to change notification settings - Fork 405
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Macroatom restructure #2786
Macroatom restructure #2786
Conversation
…city state with the solver
…ked in the future
…k we should just to a to_numba method here or not require it)
*beep* *bop* Significantly changed benchmarks: | Change | Before [2df54e5d] <master> | After [537562c1] | Ratio | Benchmark (Parameter) |
|----------|------------------------------|---------------------|---------|----------------------------------------------------------------------------------------------------------------------|
| - | 1.62±0.04μs | 1.21±0μs | 0.75 | transport_geometry_calculate_distances.BenchmarkTransportGeometryCalculateDistances.time_calculate_distance_boundary |
All benchmarks: Benchmarks that have improved:
| Change | Before [2df54e5d] <master> | After [537562c1] | Ratio | Benchmark (Parameter) |
|----------|------------------------------|---------------------|---------|----------------------------------------------------------------------------------------------------------------------|
| - | 1.62±0.04μs | 1.21±0μs | 0.75 | transport_geometry_calculate_distances.BenchmarkTransportGeometryCalculateDistances.time_calculate_distance_boundary |
Benchmarks that have stayed the same:
| Change | Before [2df54e5d] <master> | After [537562c1] | Ratio | Benchmark (Parameter) |
|----------|------------------------------|---------------------|---------|-------------------------------------------------------------------------------------------------------------------------------------|
| | 501±100ns | 621±200ns | ~1.24 | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_compton_opacity_calculation |
| | 44.1±20μs | 52.7±20μs | ~1.20 | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_line_emission |
| | 45.7±20μs | 51.5±30μs | ~1.13 | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_line_scatter |
| | 3.05±0.6μs | 3.41±0.6μs | ~1.12 | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket_within_shell |
| | 561±200ns | 621±200ns | ~1.11 | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_photoabsorption_opacity_calculation |
| | 2.81±0.8μs | 3.02±0.6μs | 1.07 | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_bad_vpacket |
| | 6.37±2μs | 6.83±1μs | 1.07 | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket |
| | 3.45±0ms | 3.65±0.04ms | 1.06 | opacities_opacity_state.BenchmarkOpacitiesOpacityState.time_opacity_state_initialize('macroatom') |
| | 18.8±4μs | 19.8±4μs | 1.05 | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_generate_rpacket_last_interaction_tracker_list |
| | 530±200ns | 551±200ns | 1.04 | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_pair_creation_opacity_calculation |
| | 1.44±0.4μs | 1.50±0.3μs | 1.04 | transport_geometry_calculate_distances.BenchmarkTransportGeometryCalculateDistances.time_calculate_distance_line |
| | 202±0.07ns | 206±0.1ns | 1.02 | spectrum_formal_integral.BenchmarkTransportMontecarloFormalIntegral.time_intensity_black_body |
| | 2.51±0.4ms | 2.56±0.4ms | 1.02 | transport_montecarlo_single_packet_loop.BenchmarkTransportMontecarloSinglePacketLoop.time_single_packet_loop |
| | 1.66±0ms | 1.68±0.01ms | 1.01 | transport_montecarlo_main_loop.BenchmarkTransportMontecarloMontecarloMainLoop.time_montecarlo_main_loop |
| | 2.07±0m | 2.07±0m | 1.00 | spectrum_formal_integral.BenchmarkTransportMontecarloFormalIntegral.time_FormalIntegrator_functions |
| | 63.2±0.2ms | 63.4±0.04ms | 1.00 | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_rpacket_trackers_to_dataframe |
| | 39.3±0.05s | 38.9±0.02s | 0.99 | run_tardis.BenchmarkRunTardis.time_run_tardis |
| | 1.06±0m | 1.04±0m | 0.99 | run_tardis.BenchmarkRunTardis.time_run_tardis_rpacket_tracking |
| | 2.25±1μs | 2.23±2μs | 0.99 | transport_montecarlo_estimators_radfield_estimator_calcs.BenchmarkMontecarloMontecarloNumbaPacket.time_update_line_estimators |
| | 31.1±0.08μs | 30.4±0.02μs | 0.98 | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_generate_rpacket_tracker_list |
| | 7.97±2μs | 7.67±2μs | 0.96 | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket_volley |
| | 2.79±0.01ms | 2.60±0ms | 0.93 | opacities_opacity_state.BenchmarkOpacitiesOpacityState.time_opacity_state_initialize('scatter') |
| | 770±0.3ns | 719±0.5ns | 0.93 | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_thomson_scatter |
If you want to see the graph of the results, you can check it here |
opacity_state_numba = opacity_state_to_numba( | ||
opacity_state, self.opacity_solver.line_interaction_type | ||
opacity_state, self.line_interaction_type | ||
) | ||
opacity_state_numba = opacity_state_numba[ | ||
simulation_state.geometry.v_inner_boundary_index : simulation_state.geometry.v_outer_boundary_index |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggests we perhaps need some slice handling by the solver? Or something similar. Not directly relevant to this PR.
return macro_atom_data.transition_probability.values[np.newaxis].T | ||
|
||
|
||
def get_macro_atom_data(atomic_data): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this shows us that we don't need all the atomic data to be passed around all the time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Definitely not, some kind of MacroAtomData structure could be useful
self.solve_non_markov_transition_probabilities( | ||
atomic_data, | ||
legacy_plasma, | ||
tau_sobolev, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that is part of the opacity state - I presume?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what do you mean?
📝 Restructures the MacroAtom to make it more independent from the plasma
Type: 🚀
feature
The properties of the macro atom used for montecarlo/opacity purposes should be decoupled from the plasma, this PR attempts to restructure the macro atom workflow.
🚦 Testing
How did you test these changes?
☑️ Checklist
build_docs
label