append dummy index to references and fix docs #3315
Merged
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
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a grid crash issue that occurs during hydrogen-only simulations in the Monte Carlo part of the code. The crash happens when a packet activates the macroatom into the highest level of the heaviest element and can't find the end of the macroatom block.
- Changes fillna value from 0 to -99 for source level index mapping consistency
- Adds a dummy index entry to macro block references to prevent out-of-bounds access
- Ensures proper handling of edge cases in macroatom Monte Carlo simulations
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Contributor
|
*beep* *bop* Details6134 [ ] syntax-error
5 I001 [*] unsorted-imports
2 E701 [ ] multiple-statements-on-one-line-colon
2 F401 [ ] unused-import
2 ISC003 [*] explicit-string-concatenation
2 RET505 [*] superfluous-else-return
1 W292 [*] missing-newline-at-end-of-file
1 B018 [ ] useless-expression
1 F403 [ ] undefined-local-with-import-star
1 F811 [ ] redefined-while-unused
1 F821 [ ] undefined-name
1 PGH004 [ ] blanket-noqa
Found 6153 errors.
[*] 11 fixable with the `--fix` option.
Complete output(might be large): Detailstardis/conftest.py:1:1: I001 [*] Import block is un-sorted or un-formatted
tardis/conftest.py:13:1: F403 `from tardis.tests.fixtures.atom_data import *` used; unable to detect undefined names
tardis/conftest.py:18:36: F401 [*] `tardis.tests.test_util.monkeysession` imported but unused
tardis/conftest.py:21:12: F401 `tardisbase` imported but unused; consider using `importlib.util.find_spec` to test for availability
tardis/conftest.py:42:49: PGH004 Use specific rule codes when using `noqa`
tardis/conftest.py:44:9: F821 Undefined name `pytest_report_header`
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:89:5: RET505 [*] Unnecessary `else` after `return` statement
tardis/spectrum/formal_integral/formal_integral_cuda.py:132:5: RET505 [*] Unnecessary `else` after `return` statement
tardis/visualization/tools/sdec_plot.py:8:1: I001 [*] Import block is un-sorted or un-formatted
tardis/visualization/tools/tests/test_liv_plot.py:1:1: I001 [*] Import block is un-sorted or un-formatted
tardis/visualization/tools/tests/test_liv_plot.py:298:29: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
tardis/visualization/tools/tests/test_sdec_plot.py:2:1: I001 [*] Import block is un-sorted or un-formatted
tardis/visualization/tools/tests/test_sdec_plot.py:276:29: ISC003 [*] Explicitly concatenated string should be implicitly concatenated
tardis/visualization/tools/tests/test_sdec_plot.py:332:9: F811 Redefinition of unused `test_generate_plot_mpl` from line 243
Found 15 errors.
[*] 10 fixable with the `--fix` option.
|
Contributor
|
*beep* *bop* Hi, human. The Click here to see your results. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3315 +/- ##
=======================================
Coverage 68.51% 68.52%
=======================================
Files 175 175
Lines 13385 13387 +2
=======================================
+ Hits 9171 9173 +2
Misses 4214 4214 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
Contributor
|
*beep* *bop* Significantly changed benchmarks: DetailsAll benchmarks: DetailsBenchmarks that have stayed the same:
| Change | Before [47738f14] <master> | After [a1adc9de] | Ratio | Benchmark (Parameter) |
|----------|------------------------------|---------------------|---------|-------------------------------------------------------------------------------------------------------------------------------------|
| | 1.79±1μs | 2.88±2μs | ~1.60 | transport_montecarlo_estimators_radfield_estimator_calcs.BenchmarkMontecarloMontecarloNumbaPacket.time_update_line_estimators |
| | 541±200ns | 481±200ns | ~0.89 | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_compton_opacity_calculation |
| | 591±100ns | 510±200ns | ~0.86 | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_pair_creation_opacity_calculation |
| | 663±0.4ns | 694±0.7ns | 1.05 | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_thomson_scatter |
| | 41.5±20μs | 43.0±20μs | 1.04 | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_line_emission |
| | 7.46±2μs | 7.72±2μs | 1.03 | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket_volley |
| | 1.07±0μs | 1.09±0μs | 1.02 | spectrum_formal_integral.BenchmarkTransportMontecarloFormalIntegral.time_intensity_black_body |
| | 35.8±0.02μs | 36.4±0.03μs | 1.02 | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_generate_rpacket_tracker_list |
| | 35.7±0.02s | 36.1±0.2s | 1.01 | run_tardis.BenchmarkRunTardis.time_run_tardis |
| | 47.4±0.04ms | 47.7±0ms | 1.01 | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_rpacket_trackers_to_dataframe |
| | 3.03±0.6ms | 3.07±0.3ms | 1.01 | transport_montecarlo_single_packet_loop.BenchmarkTransportMontecarloSinglePacketLoop.time_single_packet_loop |
| | 5.92±1μs | 5.97±1μs | 1.01 | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket |
| | 1.04±0m | 1.04±0m | 1.00 | run_tardis.BenchmarkRunTardis.time_run_tardis_rpacket_tracking |
| | 58.9±0.01s | 59.1±0.05s | 1.00 | spectrum_formal_integral.BenchmarkTransportMontecarloFormalIntegral.time_FormalIntegrator_functions |
| | 1.61±0ms | 1.61±0.01ms | 1.00 | transport_montecarlo_main_loop.BenchmarkTransportMontecarloMontecarloMainLoop.time_montecarlo_main_loop |
| | 1.17±0μs | 1.15±0μs | 0.99 | transport_geometry_calculate_distances.BenchmarkTransportGeometryCalculateDistances.time_calculate_distance_boundary |
| | 2.62±0.6μs | 2.58±0.5μs | 0.99 | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_bad_vpacket |
| | 2.82±0.6μs | 2.80±0.6μs | 0.99 | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket_within_shell |
| | 44.0±30μs | 43.0±30μs | 0.98 | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_line_scatter |
| | 23.7±6μs | 23.2±6μs | 0.98 | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_generate_rpacket_last_interaction_tracker_list |
| | 2.58±0ms | 2.50±0.01ms | 0.97 | opacities_opacity_state.BenchmarkOpacitiesOpacityState.time_opacity_state_initialize('scatter') |
| | 601±200ns | 571±200ns | 0.95 | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_photoabsorption_opacity_calculation |
| | 3.58±0ms | 3.40±0.01ms | 0.95 | opacities_opacity_state.BenchmarkOpacitiesOpacityState.time_opacity_state_initialize('macroatom') |
| | 1.66±0.3μs | 1.52±0.5μs | 0.92 | transport_geometry_calculate_distances.BenchmarkTransportGeometryCalculateDistances.time_calculate_distance_line |
If you want to see the graph of the results, you can check it here |
jvshields
changed the title
andrewfullard
previously approved these changes
Sep 22, 2025
DeerWhale
approved these changes
Sep 22, 2025
andrewfullard
approved these changes
Sep 29, 2025
jvshields
added
git-lfs-pull
atharva-2001
added a commit
to atharva-2001/tardis
that referenced
this pull request
Oct 10, 2025
commit e122bda Author: Atharva Arya <[email protected]> Date: Tue Oct 7 19:26:09 2025 +0530 Post Release Fix (tardis-sn#3314) remove path of missing zenodo file commit aba1415 Author: Connor McClellan <[email protected]> Date: Mon Oct 6 16:39:58 2025 -0400 Remove duplicate OpacityStateNumba class (tardis-sn#3322) * remove duplicate definition of OpacityStateNumba class in opacity_state.py * add new branch for OpacityStateNumba fix * update .orcid.csv to include connor-mcclellan's ORCID * add newline at the end of .orcid.csv file * adding connor-mcclellan to mailmap commit ecf6483 Author: Joshua Shields <[email protected]> Date: Mon Oct 6 09:54:31 2025 -0400 append dummy index to references and fix docs (tardis-sn#3315) * append dummy index to references * remove fill for source since they should should never be empty * remove whitespace change * change solve in montecarlo tutorial * add hydrogen only test * add tolerances * rtol 1e-12
connor-mcclellan
added a commit
that referenced
this pull request
Dec 23, 2025
* Refactor imports in base.py and test_interaction.py; add LineInteractionType class in interaction.py * Refactor opacity state initialization to use numba implementation; update related tests * Refactor OpacityStateNumba class to use Numba types directly; update initialization and documentation * Refactor packet and tracker classes for Monte Carlo simulation; implement RPacket and VPacket with associated methods * Refactor Monte Carlo transport module to reorganize packet structure - 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. * Refactor packet source and testing structure - Updated import paths in `packet_source.py` to reflect new module structure. * Update tardis/opacities/opacity_state_numba.py Co-authored-by: Copilot <[email protected]> * Refactor base packet source and black body packet source classes for Monte Carlo simulation * Refactor import paths for gamma ray packet source and related classes * Add high energy gamma ray packet source and associated tests - 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. * Refactor packet source tests: update fixture imports and remove obsolete test files * Update energy parameter description in RPacket class * empty commit to get CI going * rename weighted.py * Refactor import paths for BlackBodySimpleSource and BlackBodyWeightedSource to use consistent module structure * Implement code changes to enhance functionality and improve performance * Fix import path for BlackBodyWeightedSource in test_weighted_packet_source.py * Implement feature X to enhance user experience and optimize performance * Enhance Monte Carlo transport tutorial with detailed instructions and code examples for independent execution * Refactor interaction type handling to use InteractionType enum for clarity and consistency * Update tardis/transport/montecarlo/packets/tests/test_packet.py Co-authored-by: Copilot <[email protected]> * Refactored Monte Carlo transport solver to initialize last interaction tracker directly, improving performance and clarity. - Enhanced the RPacketLastInteractionTracker class to include detailed tracking for various interaction types (line, electron scattering, continuum processes, and boundary crossings). - Implemented methods for tracking interactions before and after their occurrence, along with counters for each interaction type. - Updated the single packet loop to utilize the new tracking methods, ensuring comprehensive data collection during simulations. - Created a new subpackage for packet tracking, organizing related classes and functions for better maintainability. * Refactor RPacketLastInteractionTracker to correctly set line interaction IDs from r_packet attributes for improved clarity and accuracy. * Refactor InteractionType enum for clarity and update documentation * Refactor interaction type handling across multiple modules to use InteractionType enum for consistency and clarity * Implement code changes to enhance functionality and improve performance * fix rpacket plot notebook * Refactor Monte Carlo packet tracking system - Moved RPacketTracker and related functions from `packet_trackers.py` to a new file `r_packet_tracker.py` for better organization. - Updated import paths across the codebase to reflect the new structure. - Removed the old `packet_trackers.py` file after migrating its contents. - Enhanced `RPacketLastInteractionTracker` to initialize with packet properties and updated boundary interaction tracking. - Improved data handling in `rpacket_trackers_to_dataframe` to utilize categorical types for interaction types. - Updated tests to align with the new import paths and structures. * Refactor RPacket tracking system for improved clarity and functionality * Enhance RPacket Tracking and Finalization - Added RPacketTracker and its associated methods for initializing and finalizing tracking of RPackets. - Updated RPacketLastInteractionTracker to remove redundant tracking logic and added a finalize_track method. - Refactored single_packet_loop to utilize the new tracking methods, ensuring proper initialization and finalization of packet states. - Improved type hints and documentation for clarity and maintainability. * Refactor RPacket tracking to use track_boundary_event method - Updated the single_packet_loop function to replace calls to initialize_tracker and track_boundary_crossing with track_boundary_event for better clarity and consistency in tracking boundary interactions. - Modified the final boundary interaction tracking to explicitly record the packet's exit from the simulation domain as a boundary event. - Adjusted the test setup for RPacketTracker to initialize with a specified length, ensuring proper tracking functionality in tests. * Refactor tracking system: Introduce TrackerFull and TrackerLastInteraction classes - Added TrackerFull class for comprehensive tracking of RPacket interactions, including core and interaction-specific arrays. - Implemented TrackerLastInteraction class to focus on the last interaction details of RPackets. - Updated array utility functions to support dynamic array extension. - Modified existing tracking logic to utilize the new TrackerFull class, replacing RPacketTracker. - Enhanced interaction tracking methods for line, electron scattering, and continuum processes. - Updated tests to reflect changes in tracking classes and ensure functionality. - Removed deprecated attributes from RPacket related to interaction tracking. * Refactor RPacket Tracking System - Renamed `rpacket_tracker` to `tracker` for clarity in `montecarlo_main_loop.py`. - Updated `MonteCarloTransportState` to use `tracker_full_df` and `tracker_last_interaction_df` instead of `rpacket_tracker`. - Modified tests in `test_rpacket_tracker.py` to accommodate changes in tracker structure and naming. - Enhanced `TrackerFull` class to improve clarity and consistency in variable naming (e.g., `r` to `radius`, `interaction_before_nu` to `before_nu`). - Updated array initialization in `TrackerFull` to use `np.full` for better default handling. - Adjusted finalization methods to reflect new naming conventions and ensure proper array sizes. - Improved interaction tracking methods in `TrackerLastInteraction` for consistency with `TrackerFull`. - Added event ID tracking in `trackers_full_list_to_arrays` for better data management. - Updated DataFrame creation methods to include new tracking data and ensure proper data types. - Fixed boundary event tracking in `single_packet_loop.py` to ensure correct interaction handling. - Updated tests in `test_montecarlo.py` to reflect changes in tracking properties and ensure accuracy. * Refactor TrackerFull and related utilities - Simplified the TrackerFull class by consolidating interaction tracking arrays into core arrays, removing unnecessary complexity. - Introduced a new utility module (tracker_full_util.py) to handle conversion of TrackerFull instances to arrays and DataFrames, improving code organization. - Updated methods to track interactions, ensuring they now utilize the new array structure. - Enhanced the finalize method in TrackerLastInteraction for API compatibility, although it remains a no-op. - Adjusted tests to accommodate the new utility functions for generating TrackerFull instances. * Refactor tracking utilities and enhance DataFrame generation for last interactions - Moved `tracker_last_interaction_to_df` function to a new utility module for better organization. - Introduced `trackers_last_interaction_to_df` to create DataFrames from last interaction trackers, ensuring consistency with full tracker DataFrames. - Updated `tracker_full_df2tracker_last_interaction_df` to handle packets with no physics interactions, returning a DataFrame with appropriate placeholders. - Removed unused functions and imports from `tracker_last_interaction.py` to streamline the codebase. - Improved categorical handling for interaction types and statuses in the new utility functions. * remmove duplicate docstring * minor modifications to get the tracker working * sdec * liv packer plotter works for real packets * enable tracking, fix liv plot and line id plotter * viz tests * tracker tests * line info tests * rpacket tracker tests * last interaction tracker changes * remove extra files * single packet loop placeholders * use fixture * clean comments and docstrings * liv plot fixes * fix notebooks * typo fix * rename simulation simple to simulation simple tracked * remove comments * try fixing benchmarks * Squashed commit of the following: commit e122bda Author: Atharva Arya <[email protected]> Date: Tue Oct 7 19:26:09 2025 +0530 Post Release Fix (#3314) remove path of missing zenodo file commit aba1415 Author: Connor McClellan <[email protected]> Date: Mon Oct 6 16:39:58 2025 -0400 Remove duplicate OpacityStateNumba class (#3322) * remove duplicate definition of OpacityStateNumba class in opacity_state.py * add new branch for OpacityStateNumba fix * update .orcid.csv to include connor-mcclellan's ORCID * add newline at the end of .orcid.csv file * adding connor-mcclellan to mailmap commit ecf6483 Author: Joshua Shields <[email protected]> Date: Mon Oct 6 09:54:31 2025 -0400 append dummy index to references and fix docs (#3315) * append dummy index to references * remove fill for source since they should should never be empty * remove whitespace change * change solve in montecarlo tutorial * add hydrogen only test * add tolerances * rtol 1e-12 * Refactor Monte Carlo transport tracking and introduce TrackerFullSolver - Updated the MonteCarloTransportSolver to use a unified tracker list for both full and last interaction tracking. - Removed redundant comments and improved variable naming for clarity. - Introduced TrackerFullSolver class to manage tracker states and processing. - Enhanced TrackerFull to accept an extend factor during initialization. - Updated utility functions to accommodate changes in tracker initialization and processing. - Cleaned up imports in the trackers package for better organization. * empty commit * empty commit * empty commit * change remaining imports for tracker last interaction after rename * empty commit during git rebase * empty commit during git rebase * remove output cells from run_montecarlo_transport.ipynb * last interaction packet tracker fixes * extend factor * run montecarlo transport * liv plot does not have tracking enabled * packets mode liv plot * how to rpacket tracking * change boundary interaction test to include shell ids using structured array to match regression data format * change extract_and_process_packet_data to use last interaction data for emitted packets * adjust boundary interactions test to account for index offset in non-boundary events * remove virtual packets from SDEC tests and hardcode (for now) the correct regression data paths * update plotter fixture to adjust parameter indexing after virtual packets removal * Add debug SDEC luminosity notebook for testing - these should be removed before the PR is merged * add better description of data used in sdec debug notebook * fix column order in mismatch comparison * sdec skip virtual packet mode tests(undoing hardcoding paths) * remove confusing unused variable * Add notebook for comparison of master packet df with feature branch * update compare packet df notebook * fix regression data path for SDEC tests, change to relative tolerance instead of almost_equal * add deprecated packets_mode argument back in, use pytest skips until a full refactor (including regression data) is done * fix shell id access in plot util - was causing widgets to fail * remove debug notebooks and data * fix SDEC test code duplication, remove duplicate tests for last interaction df properties, add new test for converting full df to last interaction df * add tardis_example config to run montecarlo transport notebook, clean up cells --------- Co-authored-by: Copilot <[email protected]> Co-authored-by: Atharva Arya <[email protected]> Co-authored-by: connor-mcclellan <[email protected]> Co-authored-by: Connor McClellan <[email protected]> Co-authored-by: Atharva Arya <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 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.
Should fix the grid issue, which traces back to running a hydrogen only simulation, which crashes in the montecarlo part of the code when a packet activates the macroatom into the highest level of the heaviest element and can't find the end of the macroatom block.
Fixes another doc that was incorrectly accessing the new macroatom.
Adds a test that runs the montecarlo main loop with hydrogen only, which explicitly tests the failure case that warranted this pr.
Regression data here. tardis-sn/tardis-regression-data#76
UPDATE: Regression data was merged and the tests pass on moria with that regression data so this should be good to go.