-
-
Notifications
You must be signed in to change notification settings - Fork 426
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
Fix init bug for: line_interaction_type=scatter #2984
base: master
Are you sure you want to change the base?
Fix init bug for: line_interaction_type=scatter #2984
Conversation
*beep* *bop* Hi, human. I'm the @tardis-bot and couldn't find your records in my database. I think we don't know each other, or you changed your credentials recently. Please add your name and email to In case you need to map an existing alias, follow this example. |
*beep* *bop* 7 G004 [ ] Logging statement uses f-string
1 RET505 [ ] Unnecessary `else` after `return` statement
1 RET506 [ ] Unnecessary `else` after `raise` statement
1 E999 [ ] SyntaxError: Expected an expression
1 W292 [*] No newline at end of file
1 W293 [*] Blank line contains whitespace
1 TRY300 [ ] Consider moving this statement to an `else` block
Complete output(might be large): .mailmap:1:38: E999 SyntaxError: Expected an expression
.mailmap:294:37: W292 [*] No newline at end of file
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:447:1: W293 [*] Blank line contains whitespace
tardis/simulation/base.py:547:13: G004 Logging statement uses f-string
tardis/simulation/base.py:654:25: G004 Logging statement uses f-string
tardis/simulation/base.py:657:13: G004 Logging statement uses f-string
tardis/simulation/base.py:662:13: G004 Logging statement uses f-string
tardis/simulation/base.py:713:13: TRY300 Consider moving this statement to an `else` block
tardis/simulation/base.py:715:26: G004 Logging statement uses f-string
Found 13 errors.
[*] 2 fixable with the `--fix` option.
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2984 +/- ##
==========================================
- Coverage 69.52% 69.16% -0.36%
==========================================
Files 228 228
Lines 16429 16432 +3
==========================================
- Hits 11422 11365 -57
- Misses 5007 5067 +60 ☔ View full report in Codecov by Sentry. |
*beep* *bop* Significantly changed benchmarks: All benchmarks: Benchmarks that have stayed the same:
| Change | Before [cec8b531] <master> | After [921b52f5] | Ratio | Benchmark (Parameter) |
|----------|------------------------------|---------------------|---------|-------------------------------------------------------------------------------------------------------------------------------------|
| | 46.7±10μs | 40.6±10μs | ~0.87 | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_line_emission |
| | 2.87±0.4ms | 2.49±0.4ms | ~0.87 | transport_montecarlo_single_packet_loop.BenchmarkTransportMontecarloSinglePacketLoop.time_single_packet_loop |
| | 55.3±20μs | 47.4±20μs | ~0.86 | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_line_scatter |
| | 21.4±5μs | 22.0±5μs | 1.03 | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_generate_rpacket_last_interaction_tracker_list |
| | 38.4±0.03μs | 39.4±0.2μs | 1.02 | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_generate_rpacket_tracker_list |
| | 37.9±0.05s | 38.2±0.09s | 1.01 | run_tardis.BenchmarkRunTardis.time_run_tardis |
| | 1.03±0m | 1.04±0m | 1.01 | run_tardis.BenchmarkRunTardis.time_run_tardis_rpacket_tracking |
| | 1.96±0.8μs | 1.98±0.9μs | 1.01 | transport_montecarlo_estimators_radfield_estimator_calcs.BenchmarkMontecarloMontecarloNumbaPacket.time_update_line_estimators |
| | 2.96±0.7μs | 3.00±0.5μs | 1.01 | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket_within_shell |
| | 2.08±0m | 2.08±0m | 1.00 | spectrum_formal_integral.BenchmarkTransportMontecarloFormalIntegral.time_FormalIntegrator_functions |
| | 711±0.3ns | 714±0.8ns | 1.00 | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_thomson_scatter |
| | 210±0.03ns | 208±0.1ns | 0.99 | spectrum_formal_integral.BenchmarkTransportMontecarloFormalIntegral.time_intensity_black_body |
| | 1.20±0μs | 1.20±0μs | 0.99 | transport_geometry_calculate_distances.BenchmarkTransportGeometryCalculateDistances.time_calculate_distance_boundary |
| | 1.70±0.01ms | 1.67±0.01ms | 0.98 | transport_montecarlo_main_loop.BenchmarkTransportMontecarloMontecarloMainLoop.time_montecarlo_main_loop |
| | 3.75±0.04ms | 3.63±0ms | 0.97 | opacities_opacity_state.BenchmarkOpacitiesOpacityState.time_opacity_state_initialize('macroatom') |
| | 551±200ns | 530±200ns | 0.96 | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_photoabsorption_opacity_calculation |
| | 3.26±0.5μs | 3.14±0.6μs | 0.96 | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_bad_vpacket |
| | 2.60±0ms | 2.48±0ms | 0.95 | opacities_opacity_state.BenchmarkOpacitiesOpacityState.time_opacity_state_initialize('scatter') |
| | 66.2±0.5ms | 62.8±0.2ms | 0.95 | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_rpacket_trackers_to_dataframe |
| | 561±200ns | 530±200ns | 0.94 | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_pair_creation_opacity_calculation |
| | 572±200ns | 531±100ns | 0.93 | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_compton_opacity_calculation |
| | 6.29±1μs | 5.85±1μs | 0.93 | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket |
| | 7.49±2μs | 6.87±1μs | 0.92 | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket_volley |
| | 1.46±0.3μs | 1.33±0.4μs | 0.91 | transport_geometry_calculate_distances.BenchmarkTransportGeometryCalculateDistances.time_calculate_distance_line |
If you want to see the graph of the results, you can check it here |
b755ac5
to
46fcfbc
Compare
*beep* *bop* Hi, human. I'm the @tardis-bot and couldn't find your records in my database. I think we don't know each other, or you changed your credentials recently. Please add your name and email to In case you need to map an existing alias, follow this example. |
46fcfbc
to
921b52f
Compare
*beep* *bop* Hi, human. I'm the @tardis-bot and couldn't find your records in my database. I think we don't know each other, or you changed your credentials recently. Please add your name and email to In case you need to map an existing alias, follow this example. |
📝 Description
Type: 🪲
bugfix
This commit fixes an initialization bug affecting runs with "line_interaction_type"=scatter. The issue was caused by two variables (macro_atom_state and plasma.beta_sobolev) that were not properly initialized. They are now set to None.
These variables are not needed for scatter physics, they were only intended to prevent the run from collapsing.
📌 Resources
This is what happend in every run with: "line_interaction_type"=scatter

🚦 Testing
How did you test these changes?
☑️ Checklist
Although I did not have reviewers for this pull request and it is my first contribution, this is a straightforward simple bug fix.
build_docs
labelNote: Im not allowed to perform any action, please help :)
@wkerzendorf @ftsamis @ssim