feat: optimization of the electron stacking#627
feat: optimization of the electron stacking#627MoritzNeuberger merged 30 commits intolegend-exp:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR optimizes the electron/positron “bulk” stacking logic by replacing repeated full distance-to-surface calculations with a cached, early-exit safety-threshold check, and updates the /RMG/Output/VolumeStacker/ UI to support multiple volumes and optional Germanium-only distance checks.
Changes:
- Add
RMGOutputToolsgeometry caching (VolumeCache) and a newis_within_surface_safety()fast threshold check with bounding-sphere prechecks. - Update
RMGVolumeDistanceStackerto support registering multiple volume names and to use the new safety-threshold API (with a safety==0 fast-path). - Register/document the new VolumeStacker commands in doc-dump and
docs/rmg-commands.md.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/remage-doc-dump.cc | Registers RMGVolumeDistanceStacker so its commands appear in generated command docs. |
| src/RMGVolumeDistanceStacker.cc | Switches to multi-volume configuration and uses is_within_surface_safety() for faster stacking decisions. |
| src/RMGOutputTools.cc | Implements global cache, cache builder, optimized distance checking, and Germanium-only filtering toggle. |
| include/RMGVolumeDistanceStacker.hh | Updates stacker configuration API (multi-volume + Germanium-only toggle). |
| include/RMGOutputTools.hh | Adds new APIs and exposes cache structs/config in the public header. |
| docs/rmg-commands.md | Documents the new /RMG/Output/VolumeStacker/ command group and updated command names/options. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Very nice work Moritz, I will look in more detail later |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…ad-local and add mutex for MultiUnion safety
|
I am not sure what is going on here, but the docs building cannot handle the |
…ed. This is to asure high energy electrons in showers to still produce expected results. Inspired by Eric.
|
Taking inspiration from Eric's implementation in his muon simulation, I added an energy threshold below which electrons are stacked. This ensures that high-energy electrons in showers still contribute. The default setting stacks all electrons, though one should test to see at what threshold it no longer influences muon showers and set it to that value. EDIT: for that I added a new macro command of which I will update the docs momentarily. |
|
something to keep in mind when somebody tests this in MT mode: I guess that the performance will be much lower (than single-threaded) because of the still existing MultiUnions in the L200 geometry. The global lock really is not that great. (the better option is however to get rid of the MultiUnions...) |
|
how should we proceed with this? i admit the issue got quite complex and i personally lost track of what is going on. i would like to understand how much this is improving the situation, the thing i want to avoid is to have to maintain complex code that does not significantly help in the end |
|
My understanding is that this helps alot |
|
IMO, the PR is final. I wanted to test how useful it is for muon sims. @EricMEsch, since you implemented it in your simulations before, please have a look if anything is missing, and maybe also check if it has the speedup you expected in your simulations. |
|
I'm happy to allocate some time during the next call to summarize and review the situation, if you'd like |
|
There was a presentation on 13.4.2026 for a status update on this PR [link]. A point of interest discussed was that the differences in the 511 keV line are likely due to positron stacking. I removed the functionality, and the agreement is now again better than before (see figures at the end of the post). Plan for the PR:
A question I had was whether we should also do a "light refactor" of the staging condition logic. At the moment, it is necessary to set Before: After: |
|
I agree with changing command name but I also think it's a good idea to ask the AI to simplify! |
- Updated RMGGermaniumOutputScheme and RMGIsotopeFilterScheme to eliminate optical photon handling, delegating to RMGStagingScheme. - Introduced RMGStagingScheme to manage staging of optical photons and electrons, allowing for deferred processing based on energy thresholds and volume safety. - Removed RMGVolumeDistanceStacker and integrated its functionality into RMGStagingScheme. - Updated tests to reflect changes in output schemes and ensure proper staging behavior for electrons and optical photons.
|
shit, didnt want to merge yet EDIT: yes, I am aware that I pressed auto-merge ... I was convinced it was about rebasing to main ... |
|
Reverted the changes in main. I guess I have to make a new PR? |
This pull request introduces optimizations to the recently added stacking scheme for electron tracks in the bulk of volumes. It optimizes safety calculations using caching and bounding spheres, and allows filtering to only consider Germanium detectors for certain checks.
Updates to the User Commands
/RMG/Output/VolumeStacker/command group was adjusted. Instead of activating the filter for only a single volume, the user can add multiple volumes via the nameAddVolumeName(register multiple volumes). In addition, the safety calculation can be adjusted to only consider the distances to HPGe detectors viaDistanceCheckGermaniumOnly(limit checks to Germanium detectors).Performance Improvements: Safety Calculation
is_within_surface_safetyfunction that quickly determines if any surface is within a safety distance, with early exits and bounding sphere optimizations.volume_cacheandVolumeCachestruct) for volume geometry data.These changes were presented in an internal presentation on 26.01.2026 [link]. Since then, some more tests have been performed. The previous tests were run with the multithreading option, which turned out to distort the mean time per event, and systematic differences were introduced due to differences depending on when the simulations were run. Now the simulations are all run single-threaded and at the same time. The plot below shows the mean time per event for a Tl208 simulation in L1000 for different E_low thresholds and different configurations of safety compared to the optical photon stacking implementation and w/o optical physics.
A slight increase in the mean time per event for larger safeties is visible, but they are all still significantly smaller than with the only optical photon stacking.
EDIT: I realize that I probably should rebase the branch to the main repo. Will do that in a bit.