Refactor Monte Carlo transport and interaction handling #3276
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ionType class in interaction.py
…date related tests
…initialization and documentation
…ment RPacket and VPacket with associated methods
- Moved packet-related classes and functions into a new 'packets' submodule for better organization. - Updated import statements throughout the codebase to reflect the new structure. - Enhanced type annotations and docstrings for clarity and improved code readability. - Refactored the PacketCollection, LastInteractionTracker, RPacket, VPacket, and associated tracker classes to utilize Numba's JIT compilation more effectively. - Improved the handling of packet properties and interactions, ensuring consistency across the Monte Carlo transport processes. - Updated tests to accommodate the new structure and ensure all functionalities remain intact.
- Updated import paths in `packet_source.py` to reflect new module structure.
Co-authored-by: Copilot <[email protected]>
…c_subpackage_packets
…Monte Carlo simulation
- Implemented `GammaRayPacketSource` for generating gamma ray packets from radioactive decay data, including positronium formation and Doppler effects. - Created unit tests for packet source classes, including tests for `BasePacketSource`, `BlackBodyWeightedSource`, and `BlackBodySimpleSource`. - Added configuration for pytest fixtures to facilitate testing of packet sources. - Removed deprecated `weighted_packet_source.py` and refactored relevant tests to use the new structure. - Ensured all new classes and methods are covered by tests to maintain code quality and reliability.
…subpackage_packet_source
…bpackage_packet_source
…Source to use consistent module structure
…re/vpacket_subprocess
… code examples for independent execution
…arity and consistency
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the Monte Carlo transport module to improve interaction type handling by introducing and standardizing the use of the InteractionType enum throughout the codebase. The changes replace hardcoded magic numbers with meaningful enum values for better readability and maintainability.
- Replace hardcoded interaction type values (-1, 1, 2, etc.) with
InteractionTypeenum constants - Update enum definition to include
NO_INTERACTION = -1and reorganize existing values - Modify packet initialization and tracking to use enum values instead of magic numbers
Reviewed Changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tardis/transport/montecarlo/packets/radiative_packet.py | Updated InteractionType enum definition and packet initialization |
| tardis/transport/montecarlo/single_packet_loop.py | Replaced hardcoded values with enum constants for interaction types |
| tardis/transport/montecarlo/packets/packet_collections.py | Updated packet collections to use enum values for initialization |
| tardis/visualization/plot_util.py | Replaced magic number comparisons with enum constants |
| tardis/visualization/sdec/util.py | Updated filtering logic to use InteractionType enum |
| tardis/visualization/tests/test_plot_util.py | Updated test assertions to use enum values |
| tardis/transport/montecarlo/packets/tests/test_packet.py | Modified test to use enum constant instead of hardcoded value |
| tardis/transport/montecarlo/tests/test_numba_interface.py | Added InteractionType import for consistency |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Contributor
|
*beep* *bop* Details7478 [ ] syntax-error
55 W293 [ ] blank-line-with-whitespace
38 E701 [ ] multiple-statements-on-one-line-colon
22 W291 [ ] trailing-whitespace
18 I001 [*] unsorted-imports
12 E902 [ ] io-error
12 RET505 [*] superfluous-else-return
9 FA100 [ ] future-rewritable-type-annotation
8 E702 [ ] multiple-statements-on-one-line-semicolon
8 F401 [*] unused-import
8 F405 [ ] undefined-local-with-import-star-usage
8 UP008 [ ] super-call-with-parameters
5 W292 [ ] missing-newline-at-end-of-file
5 G004 [ ] logging-f-string
4 E722 [ ] bare-except
4 D202 [*] blank-line-after-function
3 B011 [ ] assert-false
3 C408 [ ] unnecessary-collection-call
3 D209 [*] new-line-after-last-paragraph
3 F821 [ ] undefined-name
3 FA102 [ ] future-required-type-annotation
3 PT015 [ ] pytest-assert-always-false
3 RET506 [*] superfluous-else-raise
3 RUF022 [*] unsorted-dunder-all
2 COM818 [ ] trailing-comma-on-bare-tuple
2 F811 [ ] redefined-while-unused
2 NPY201 [ ] numpy2-deprecation
1 E402 [ ] module-import-not-at-top-of-file
1 D411 [*] no-blank-line-before-section
1 F403 [ ] undefined-local-with-import-star
1 FURB187 [ ] list-reverse-copy
1 G001 [ ] logging-string-format
1 ICN001 [ ] unconventional-import-alias
1 PIE790 [*] unnecessary-placeholder
1 RUF021 [*] parenthesize-chained-operators
1 UP024 [*] os-error-alias
1 UP034 [*] extraneous-parentheses
Found 7734 errors.
[*] 77 fixable with the `--fix` option (22 hidden fixes can be enabled with the `--unsafe-fixes` option).
Complete output(might be large): Detailstardis/analysis.py:96:9: E722 Do not use bare `except`
tardis/analysis.py:110:9: E722 Do not use bare `except`
tardis/analysis.py:121:9: E722 Do not use bare `except`
tardis/analysis.py:136:9: E722 Do not use bare `except`
tardis/analysis.py:374:9: RET505 [*] Unnecessary `else` after `return` statement
tardis/analysis.py:396:9: RET505 [*] Unnecessary `else` after `return` statement
tardis/analysis.py:421:9: RET505 [*] Unnecessary `else` after `return` statement
tardis/configuration/__init__.py:1:1: E902 No such file or directory (os error 2)
tardis/configuration/sorting_globals.py:1:1: E902 No such file or directory (os error 2)
tardis/energy_input/main_gamma_ray_loop.py:1:1: I001 [*] Import block is un-sorted or un-formatted
tardis/energy_input/main_gamma_ray_loop.py:30:23: FA100 Add `from __future__ import annotations` to simplify `typing.Optional`
tardis/energy_input/samplers.py:1:1: I001 [*] Import block is un-sorted or un-formatted
tardis/energy_input/samplers.py:155:49: NPY201 `np.trapz` will be removed in NumPy 2.0. Use `numpy.trapezoid` on NumPy 2.0, or ignore this warning on earlier versions.
tardis/energy_input/samplers.py:178:1: W293 [*] Blank line contains whitespace
tardis/energy_input/samplers.py:190:53: W291 [*] Trailing whitespace
tardis/energy_input/samplers.py:191:35: W291 [*] Trailing whitespace
tardis/energy_input/samplers.py:192:28: W292 [*] No newline at end of file
tardis/energy_input/tests/test_energy_source.py:35:5: PT015 Assertion always fails, replace with `pytest.fail()`
tardis/energy_input/tests/test_energy_source.py:35:12: B011 Do not `assert False` (`python -O` removes these calls), raise `AssertionError()`
tardis/energy_input/tests/test_energy_source.py:41:5: PT015 Assertion always fails, replace with `pytest.fail()`
tardis/energy_input/tests/test_energy_source.py:41:12: B011 Do not `assert False` (`python -O` removes these calls), raise `AssertionError()`
tardis/energy_input/tests/test_energy_source.py:47:5: PT015 Assertion always fails, replace with `pytest.fail()`
tardis/energy_input/tests/test_energy_source.py:47:12: B011 Do not `assert False` (`python -O` removes these calls), raise `AssertionError()`
tardis/gui/widgets.py:7:5: I001 [*] Import block is un-sorted or un-formatted
tardis/gui/widgets.py:9:5: I001 [*] Import block is un-sorted or un-formatted
tardis/gui/widgets.py:17:1: I001 [*] Import block is un-sorted or un-formatted
tardis/gui/widgets.py:17:8: ICN001 `matplotlib` should be imported as `mpl`
tardis/gui/widgets.py:18:1: F403 `from matplotlib.figure import *` used; unable to detect undefined names
tardis/gui/widgets.py:25:32: F401 [*] `matplotlib.patches.Circle` imported but unused
tardis/gui/widgets.py:30:30: F401 [*] `tardis.util` imported but unused
tardis/gui/widgets.py:37:9: D202 [*] No blank lines allowed after function docstring (found 1)
tardis/gui/widgets.py:44:23: F405 `Figure` may be undefined, or defined from star imports
tardis/gui/widgets.py:55:14: UP008 Use `super()` instead of `super(__class__, self)`
tardis/gui/widgets.py:56:14: UP008 Use `super()` instead of `super(__class__, self)`
tardis/gui/widgets.py:59:14: UP008 Use `super()` instead of `super(__class__, self)`
tardis/gui/widgets.py:165:27: C408 Unnecessary `dict()` call (rewrite as a literal)
tardis/gui/widgets.py:168:26: C408 Unnecessary `dict()` call (rewrite as a literal)
tardis/gui/widgets.py:169:23: C408 Unnecessary `dict()` call (rewrite as a literal)
tardis/gui/widgets.py:187:9: RET505 [*] Unnecessary `elif` after `return` statement
tardis/gui/widgets.py:199:14: UP008 Use `super()` instead of `super(__class__, self)`
tardis/gui/widgets.py:227:14: UP008 Use `super()` instead of `super(__class__, self)`
tardis/gui/widgets.py:230:22: F405 `yaml` may be undefined, or defined from star imports
tardis/gui/widgets.py:230:61: F405 `yaml` may be undefined, or defined from star imports
tardis/gui/widgets.py:354:24: F405 `TreeModel` may be undefined, or defined from star imports
tardis/gui/widgets.py:359:38: F405 `TreeDelegate` may be undefined, or defined from star imports
tardis/gui/widgets.py:430:35: UP024 [*] Replace aliased errors with `OSError`
tardis/gui/widgets.py:445:9: PIE790 [*] Unnecessary `pass` statement
tardis/gui/widgets.py:529:9: D202 [*] No blank lines allowed after function docstring (found 1)
tardis/gui/widgets.py:647:41: F405 `np` may be undefined, or defined from star imports
tardis/gui/widgets.py:658:9: D209 [*] Multi-line docstring closing quotes should be on a separate line
tardis/gui/widgets.py:661:41: F405 `np` may be undefined, or defined from star imports
tardis/gui/widgets.py:689:41: F405 `np` may be undefined, or defined from star imports
tardis/gui/widgets.py:816:14: UP008 Use `super()` instead of `super(__class__, self)`
tardis/gui/widgets.py:852:9: D209 [*] Multi-line docstring closing quotes should be on a separate line
tardis/gui/widgets.py:928:9: D209 [*] Multi-line docstring closing quotes should be on a separate line
tardis/gui/widgets.py:930:14: UP008 Use `super()` instead of `super(__class__, self)`
tardis/gui/widgets.py:1125:14: UP008 Use `super()` instead of `super(__class__, self)`
tardis/gui/widgets.py:1232:9: D202 [*] No blank lines allowed after function docstring (found 1)
tardis/io/atom_data/base.py:197:34: G004 Logging statement uses f-string
tardis/io/atom_data/base.py:259:17: G004 Logging statement uses f-string
tardis/io/atom_data/base.py:263:21: G001 Logging statement uses `str.format`
tardis/io/atom_data/base.py:707:17: G004 Logging statement uses f-string
tardis/model/matter/composition.py:173:9: RET505 [*] Unnecessary `else` after `return` statement
tardis/model/matter/decay.py:101:21: G004 Logging statement uses f-string
tardis/opacities/macro_atom/macroatom_state.py:1:1: I001 [*] Import block is un-sorted or un-formatted
tardis/opacities/macro_atom/macroatom_state.py:127:9: D411 [*] Missing blank line before section ("Returns")
tardis/opacities/macro_atom/macroatom_state.py:168:9: D202 [*] No blank lines allowed after function docstring (found 1)
tardis/opacities/macro_atom/transition_probabilities.py:15:11: RUF022 [*] `__all__` is not sorted
tardis/plasma/equilibrium/rates/heating_cooling_rates.py:1:1: E902 No such file or directory (os error 2)
tardis/plasma/equilibrium/tests/test_heating_cooling_rates.py:1:1: E902 No such file or directory (os error 2)
tardis/plasma/equilibrium/tests/test_ion_populations.py:5:8: F401 [*] `pytest` imported but unused
tardis/plasma/equilibrium/thermal_balance.py:1:1: E902 No such file or directory (os error 2)
tardis/plasma/properties/atomic.py:27:11: RUF022 [*] `__all__` is not sorted
tardis/plasma/properties/atomic.py:554:9: RET505 [*] Unnecessary `else` after `return` statement
tardis/plasma/properties/atomic.py:585:9: RET505 [*] Unnecessary `else` after `return` statement
tardis/plasma/properties/atomic.py:598:17: G004 Logging statement uses f-string
tardis/plasma/properties/atomic.py:639:9: RET505 [*] Unnecessary `else` after `return` statement
tardis/plasma/properties/continuum_processes/rates.py:22:11: RUF022 [*] `__all__` is not sorted
tardis/plasma/properties/continuum_processes/rates.py:156:13: FURB187 Use of assignment of `reversed` on list `idx_arrays`
tardis/spectrum/formal_integral/formal_integral_cuda.py:1:1: I001 [*] Import block is un-sorted or un-formatted
tardis/spectrum/formal_integral/formal_integral_cuda.py:85:5: RET505 [*] Unnecessary `else` after `return` statement
tardis/spectrum/formal_integral/formal_integral_cuda.py:124:5: RET505 [*] Unnecessary `else` after `return` statement
tardis/spectrum/formal_integral/formal_integral_solver.py:38:13: FA102 Missing `from __future__ import annotations`, but uses PEP 604 union
tardis/spectrum/formal_integral/formal_integral_solver.py:41:61: FA102 Missing `from __future__ import annotations`, but uses PEP 604 union
tardis/spectrum/formal_integral/formal_integral_solver.py:312:10: FA102 Missing `from __future__ import annotations`, but uses PEP 585 collection
tardis/spectrum/formal_integral/tests/test_cuda_formal_integral.py:1:1: I001 [*] Import block is un-sorted or un-formatted
tardis/spectrum/formal_integral/tests/test_cuda_formal_integral.py:9:5: F401 [*] `tardis.spectrum.formal_integral.base.interpolate_integrator_quantities` imported but unused
tardis/spectrum/formal_integral/tests/test_cuda_formal_integral.py:15:61: F401 [*] `tardis.spectrum.formal_integral.source_function.SourceFunctionSolver` imported but unused
tardis/transport/montecarlo/estimators/util.py:66:32: NPY201 `np.trapz` will be removed in NumPy 2.0. Use `numpy.trapezoid` on NumPy 2.0, or ignore this warning on earlier versions.
tardis/transport/montecarlo/packets/packet_trackers.py:199:1: W293 [*] Blank line contains whitespace
tardis/transport/montecarlo/packets/packet_trackers.py:202:1: W293 [*] Blank line contains whitespace
tardis/transport/montecarlo/packets/packet_trackers.py:213:1: W293 [*] Blank line contains whitespace
tardis/transport/montecarlo/packets/packet_trackers.py:216:1: W293 [*] Blank line contains whitespace
tardis/transport/montecarlo/packets/packet_trackers.py:227:1: W293 [*] Blank line contains whitespace
tardis/transport/montecarlo/packets/packet_trackers.py:231:1: W293 [*] Blank line contains whitespace
tardis/transport/montecarlo/packets/packet_trackers.py:276:1: W293 Blank line contains whitespace
tardis/transport/montecarlo/packets/radiative_packet.py:1:1: I001 [*] Import block is un-sorted or un-formatted
tardis/transport/montecarlo/packets/tests/test_packet.py:1:1: I001 [*] Import block is un-sorted or un-formatted
tardis/transport/montecarlo/packets/tests/test_packet.py:22:1: E402 Module level import not at top of file
tardis/transport/montecarlo/single_packet_loop.py:1:1: I001 [*] Import block is un-sorted or un-formatted
tardis/transport/montecarlo/tests/test_numba_interface.py:1:1: I001 [*] Import block is un-sorted or un-formatted
tardis/visualization/plot_util.py:3:1: I001 [*] Import block is un-sorted or un-formatted
tardis/visualization/plot_util.py:56:5: RET505 [*] Unnecessary `else` after `return` statement
tardis/visualization/sdec/util.py:117:9: RET506 [*] Unnecessary `else` after `raise` statement
tardis/visualization/sdec/util.py:275:9: RET506 [*] Unnecessary `else` after `raise` statement
tardis/visualization/sdec/util.py:312:9: UP034 [*] Avoid extraneous parentheses
tardis/visualization/tests/test_plot_util.py:1:1: I001 [*] Import block is un-sorted or un-formatted
tardis/visualization/tools/rpacket_plot.py:457:10: FA100 Add `from __future__ import annotations` to simplify `typing.Tuple`
tardis/visualization/tools/rpacket_plot.py:457:40: FA100 Add `from __future__ import annotations` to simplify `typing.List`
tardis/visualization/tools/rpacket_plot.py:535:10: FA100 Add `from __future__ import annotations` to simplify `typing.Tuple`
tardis/visualization/tools/rpacket_plot.py:570:1: W293 [*] Blank line contains whitespace
tardis/visualization/tools/rpacket_plot.py:596:10: FA100 Add `from __future__ import annotations` to simplify `typing.Tuple`
tardis/visualization/tools/rpacket_plot.py:662:10: FA100 Add `from __future__ import annotations` to simplify `typing.List`
tardis/visualization/tools/rpacket_plot.py:706:16: FA100 Add `from __future__ import annotations` to simplify `typing.Optional`
tardis/visualization/tools/rpacket_plot.py:781:64: FA100 Add `from __future__ import annotations` to simplify `typing.List`
tardis/visualization/tools/rpacket_plot.py:781:69: FA100 Add `from __future__ import annotations` to simplify `typing.Dict`
tardis/visualization/widgets/custom_abundance.py:737:16: RUF021 [*] Parenthesize `a and b` expressions when chaining `and` and `or` together, to make the precedence clear
tardis/visualization/widgets/custom_abundance.py:1127:9: RET505 [*] Unnecessary `else` after `return` statement
tardis/visualization/widgets/custom_abundance.py:1418:9: RET506 [*] Unnecessary `else` after `raise` statement
tardis/visualization/widgets/custom_abundance.py:1721:9: F811 Redefinition of unused `input_d_time_0_eventhandler` from line 1710
tardis/visualization/widgets/custom_abundance.py:1751:13: F821 Undefined name `display`
tardis/visualization/widgets/custom_abundance.py:1753:13: F821 Undefined name `display`
tardis/visualization/widgets/custom_abundance.py:1755:13: F821 Undefined name `display`
tardis/visualization/widgets/line_info.py:3:1: I001 [*] Import block is un-sorted or un-formatted
tardis/visualization/widgets/line_info.py:450:1: W293 [*] Blank line contains whitespace
tardis/visualization/widgets/line_info.py:454:1: W293 [*] Blank line contains whitespace
tardis/visualization/widgets/line_info.py:458:1: W293 [*] Blank line contains whitespace
tardis/visualization/widgets/line_info.py:462:1: W293 [*] Blank line contains whitespace
tardis/visualization/widgets/line_info.py:467:1: W293 [*] Blank line contains whitespace
tardis/visualization/widgets/line_info.py:476:1: W293 [*] Blank line contains whitespace
tardis/visualization/widgets/line_info.py:531:1: W293 [*] Blank line contains whitespace
tardis/visualization/widgets/line_info.py:715:1: W293 [*] Blank line contains whitespace
Found 132 errors.
[*] 69 fixable with the `--fix` option (22 hidden fixes can be enabled with the `--unsafe-fixes` option).
|
Contributor
|
RPacketPlotter looks good, but the legend doesn't match the actual interaction point colours |
andrewfullard
requested changes
Sep 5, 2025
Contributor
andrewfullard
left a comment
andrewfullard
left a comment
There was a problem hiding this comment.
Need the docs build to complete to view the rpacket plotter and check the legend
Contributor
|
Now an exciting new docs build error. Have fun... |
jaladh-singhal
previously approved these changes
Sep 5, 2025
Member
There was a problem hiding this comment.
Looks good to me!
One main thing is visualization modules expected escattering interaction to be 1 but now that's boundary interaction and escattering has become 4. I hope this doesn't break anything? The notebook execution is failing in docs build! other tests seem to pass but something to be careful about before merging this PR.
|
|
||
| line_mask = (expected_df["last_interaction_type"] > -1) & ( | ||
| line_mask = (expected_df["last_interaction_type"] > InteractionType.NO_INTERACTION) & ( | ||
| expected_df["last_line_interaction_in_id"] > -1 |
Member
There was a problem hiding this comment.
perhaps this -1 can also use InteractionType.NO_INTERACTION
andrewfullard
previously approved these changes
Sep 9, 2025
wkerzendorf
commented
Sep 9, 2025
andrewfullard
approved these changes
Sep 15, 2025
atharva-2001
approved these changes
Sep 15, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Enhance the organization and clarity of the Monte Carlo transport module by restructuring packet classes and improving interaction type handling. Use the
InteractionTypeenum for better consistency and readability across the codebase. Update tests and documentation to reflect these changes and ensure functionality remains intact.Potentially fixes #1766
Potentially fixes #1396