-
-
Notifications
You must be signed in to change notification settings - Fork 423
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
[TODO] updates the macroatom_state based on the flag #2936
base: master
Are you sure you want to change the base?
Conversation
*beep* *bop* 7 G004 [ ] Logging statement uses f-string
1 RET505 [ ] Unnecessary `else` after `return` statement
1 RET506 [ ] Unnecessary `else` after `raise` statement
1 I001 [*] Import block is un-sorted or un-formatted
1 TRY300 [ ] Consider moving this statement to an `else` block
Complete output(might be large): tardis/opacities/macro_atom/macroatom_state.py:1:1: I001 [*] Import block is un-sorted or un-formatted
tardis/simulation/base.py:197:13: RET506 Unnecessary `else` after `raise` statement
tardis/simulation/base.py:261:17: G004 Logging statement uses f-string
tardis/simulation/base.py:268:9: RET505 Unnecessary `else` after `return` statement
tardis/simulation/base.py:445:13: G004 Logging statement uses f-string
tardis/simulation/base.py:543:13: G004 Logging statement uses f-string
tardis/simulation/base.py:650:25: G004 Logging statement uses f-string
tardis/simulation/base.py:653:13: G004 Logging statement uses f-string
tardis/simulation/base.py:658:13: G004 Logging statement uses f-string
tardis/simulation/base.py:709:13: TRY300 Consider moving this statement to an `else` block
tardis/simulation/base.py:711:26: G004 Logging statement uses f-string
Found 11 errors.
[*] 1 fixable with the `--fix` option.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2936 +/- ##
==========================================
- Coverage 70.39% 70.03% -0.36%
==========================================
Files 222 222
Lines 16157 16157
==========================================
- Hits 11373 11316 -57
- Misses 4784 4841 +57 ☔ View full report in Codecov by Sentry. |
*beep* *bop* Significantly changed benchmarks: All benchmarks: Benchmarks that have stayed the same:
| Change | Before [0fc2c763] <master> | After [8cd7e785] | Ratio | Benchmark (Parameter) |
|----------|------------------------------|---------------------|---------|-------------------------------------------------------------------------------------------------------------------------------------|
| | 100±40μs | 49.0±30μs | ~0.49 | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_line_scatter |
| | 2.98±0.2μs | 3.23±0.5μs | 1.08 | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_bad_vpacket |
| | 591±200ns | 601±100ns | 1.02 | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_pair_creation_opacity_calculation |
| | 3.63±0.01ms | 3.66±0.01ms | 1.01 | opacities_opacity_state.BenchmarkOpacitiesOpacityState.time_opacity_state_initialize('macroatom') |
| | 6.10±1μs | 6.14±0.9μs | 1.01 | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket |
| | 6.91±2μs | 6.95±2μs | 1.01 | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket_volley |
| | 571±100ns | 572±200ns | 1.00 | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_photoabsorption_opacity_calculation |
| | 2.08±0m | 2.08±0m | 1.00 | spectrum_formal_integral.BenchmarkTransportMontecarloFormalIntegral.time_FormalIntegrator_functions |
| | 719±2ns | 717±0.4ns | 1.00 | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_thomson_scatter |
| | 37.9±0.05μs | 38.1±0.07μs | 1.00 | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_generate_rpacket_tracker_list |
| | 3.28±0.3μs | 3.27±0.5μs | 1.00 | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket_within_shell |
| | 2.63±0ms | 2.59±0.01ms | 0.99 | opacities_opacity_state.BenchmarkOpacitiesOpacityState.time_opacity_state_initialize('scatter') |
| | 38.6±0.05s | 38.1±0.01s | 0.99 | run_tardis.BenchmarkRunTardis.time_run_tardis |
| | 1.04±0m | 1.03±0m | 0.99 | run_tardis.BenchmarkRunTardis.time_run_tardis_rpacket_tracking |
| | 1.44±0.3μs | 1.42±0.4μs | 0.99 | transport_geometry_calculate_distances.BenchmarkTransportGeometryCalculateDistances.time_calculate_distance_line |
| | 1.69±0.01ms | 1.68±0.01ms | 0.99 | transport_montecarlo_main_loop.BenchmarkTransportMontecarloMontecarloMainLoop.time_montecarlo_main_loop |
| | 2.51±0.4ms | 2.49±0.4ms | 0.99 | transport_montecarlo_single_packet_loop.BenchmarkTransportMontecarloSinglePacketLoop.time_single_packet_loop |
| | 210±0.01ns | 206±2ns | 0.98 | spectrum_formal_integral.BenchmarkTransportMontecarloFormalIntegral.time_intensity_black_body |
| | 1.20±0μs | 1.18±0μs | 0.98 | transport_geometry_calculate_distances.BenchmarkTransportGeometryCalculateDistances.time_calculate_distance_boundary |
| | 581±100ns | 561±100ns | 0.97 | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_compton_opacity_calculation |
| | 49.4±20μs | 48.1±20μs | 0.97 | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_line_emission |
| | 2.11±2μs | 1.98±1μs | 0.94 | transport_montecarlo_estimators_radfield_estimator_calcs.BenchmarkMontecarloMontecarloNumbaPacket.time_update_line_estimators |
| | 65.3±0.3ms | 61.5±0.05ms | 0.94 | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_rpacket_trackers_to_dataframe |
| | 22.2±6μs | 20.5±5μs | 0.93 | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_generate_rpacket_last_interaction_tracker_list |
If you want to see the graph of the results, you can check it here |
Hi @atharva-2001 @andrewfullard @jvshields , |
@@ -71,9 +71,7 @@ def from_legacy_plasma(cls, plasma): | |||
plasma.atomic_data.lines_upper2macro_reference_idx | |||
) | |||
|
|||
if ( | |||
montecarlo_globals.CONTINUUM_PROCESSES_ENABLED | |||
): # TODO: Unify this in the plasma solver |
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.
Why remove this?
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.
you particularly mean the # TODO: Unify this in the plasma solver
right?
📝 Description
Type: 🪲
bugfix
| 🚀feature
| ☣️breaking change
| 🚦testing
| 📝documentation
| 🎢infrastructure
TODO: Based on the
CONTINUUM_PROCESSES_ENABLED
flag, themacroatom_state
is initialised.📌 Resources
Examples, notebooks, and links to useful references.
🚦 Testing
How did you test these changes?
☑️ Checklist
build_docs
label