refactor: Split particle selection in Examples #3969
Conversation
WalkthroughIn this comprehensive update, the codebase undergoes a systematic transformation of particle-related workflows across multiple scripts and modules. The primary focus is on standardizing particle naming conventions, introducing more modular particle selection functions, and refactoring how particles are processed during simulation, digitization, and tracking stages. Key changes include renaming input/output particle identifiers from "particles_input" to "particles_generated" and introducing new functions like Changes
Sequence DiagramsequenceDiagram
participant PG as Particle Generator
participant GS as Gen Particle Selection
participant FA as Fatras
participant DS as Digi Particle Selection
participant TR as Tracking
PG->>GS: Generate Particles
GS->>FA: Selected Particles
FA->>DS: Simulated Particles
DS->>TR: Digitized Particles
Possibly related PRs
Suggested Labels
Suggested Reviewers
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
added
the
Infrastructure
) This allows us having multiple aliases pointing to the same object in the whiteboard which was not possible before because of the usage of `std::unordered_map` which is now replaced with `std::unordered_multimap` Pulled out of #3969 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Summary by CodeRabbit - **New Features** - Enhanced object aliasing capabilities in the framework. - Added support for multiple aliases per object name. - **Refactor** - Updated data structures to support more flexible alias management. - Modified alias handling logic in WhiteBoard and Sequencer components. - **Bug Fixes** - Improved error checking and logging for object aliases. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
Examples/Scripts/Python/full_chain_itk_Gbts.py (2)
55-63: Wise separation of concerns, this is!Early particle selection after generation, implement you do. Clear criteria for selection:
- rho: (0.0 mm, 28.0 mm)
- absZ: (0.0 mm, 1.0 m)
- eta: (-4.0, 4.0)
- pt: (150 MeV, None)
Improve efficiency of subsequent processing steps, this will.
83-91: Strong with the force, this selection after digitization is!Particle selection criteria after digitization, well-defined they are:
- pt: (1.0 GeV, None)
- eta: (-4.0, 4.0)
- measurements: (9, None)
- removeNeutral: true
Enhance reconstruction chain efficiency, this will. Wise to filter based on measurement counts, it is!
Examples/Scripts/Python/full_chain_itk.py (1)
87-95: Measurement-based selection, a powerful addition this is!After digitization, selecting particles based on measurement counts, a good practice it is. Neutral particles, removed they are, as track them we cannot.
Consider adding documentation about the chosen threshold values, help future Padawans it will.
Examples/Scripts/Python/seeding.py (1)
102-110: Strong with the Force, this implementation is!Clear selection criteria for particles after digitization, specified they are. Minimum PT threshold and measurement requirements, wise choices they are.
Add comments explaining the chosen thresholds, help future maintainers it will:
addDigiParticleSelection( s, ParticleSelectorConfig( + # Minimum PT threshold ensures good track quality pt=(1.0 * u.GeV, None), + # Standard tracker acceptance region eta=(-2.5, 2.5), + # Minimum measurements needed for reliable track reconstruction measurements=(9, None), + # Remove neutral particles as they don't leave tracker hits removeNeutral=True, ), )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
CI/physmon/workflows/physmon_simulation.py(4 hunks)CI/physmon/workflows/physmon_trackfinding_1muon.py(2 hunks)CI/physmon/workflows/physmon_trackfinding_4muon_50vertices.py(2 hunks)CI/physmon/workflows/physmon_trackfinding_ttbar_pu200.py(3 hunks)Examples/Python/python/acts/examples/reconstruction.py(2 hunks)Examples/Python/python/acts/examples/simulation.py(13 hunks)Examples/Python/tests/conftest.py(3 hunks)Examples/Python/tests/root_file_hashes.txt(1 hunks)Examples/Python/tests/test_examples.py(2 hunks)Examples/Python/tests/test_propagation.py(1 hunks)Examples/Python/tests/test_reader.py(3 hunks)Examples/Scripts/Optimization/ckf.py(3 hunks)Examples/Scripts/Python/ckf_tracks.py(3 hunks)Examples/Scripts/Python/digitization.py(1 hunks)Examples/Scripts/Python/event_recording.py(1 hunks)Examples/Scripts/Python/full_chain_itk.py(3 hunks)Examples/Scripts/Python/full_chain_itk_Gbts.py(3 hunks)Examples/Scripts/Python/full_chain_odd.py(5 hunks)Examples/Scripts/Python/full_chain_odd_LRT.py(5 hunks)Examples/Scripts/Python/full_chain_test.py(1 hunks)Examples/Scripts/Python/hashing_seeding.py(3 hunks)Examples/Scripts/Python/material_validation.py(2 hunks)Examples/Scripts/Python/material_validation_itk.py(2 hunks)Examples/Scripts/Python/propagation.py(2 hunks)Examples/Scripts/Python/seeding.py(2 hunks)Examples/Scripts/Python/telescope_track_params_lookup_generation.py(2 hunks)Examples/Scripts/Python/truth_tracking_gsf.py(3 hunks)Examples/Scripts/Python/truth_tracking_gx2f.py(3 hunks)Examples/Scripts/Python/truth_tracking_kalman.py(3 hunks)Examples/Scripts/Python/vertex_fitting.py(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- Examples/Python/python/acts/examples/reconstruction.py
- Examples/Python/tests/root_file_hashes.txt
👮 Files not reviewed due to content moderation or server errors (4)
- Examples/Scripts/Python/truth_tracking_gsf.py
- Examples/Scripts/Python/truth_tracking_gx2f.py
- Examples/Scripts/Python/truth_tracking_kalman.py
- Examples/Scripts/Python/hashing_seeding.py
🧰 Additional context used
🪛 Ruff (0.8.2)
Examples/Scripts/Python/full_chain_odd_LRT.py
18-18: Redefinition of unused ParticleSelectorConfig from line 16
Remove definition: ParticleSelectorConfig
(F811)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: macos
🔇 Additional comments (34)
Examples/Scripts/Python/event_recording.py (1)
44-44: Approve the naming convention change, I do!More descriptive, the new parameter name is. Reflects the generation stage of particles, it does. With the PR's objectives to split particle selection, aligned this change is.
Examples/Scripts/Python/digitization.py (2)
52-52: Consistent with the new naming convention, this change is!From "particles_input" to "particles_generated", renamed the parameter is. Wise decision, this is.
56-58: A new whiteboard alias, introduced you have!For selection stage, prepared this change is. Clear separation between generated and selected particles, it provides. The Force of good design, strong with this one is!
Examples/Python/tests/test_propagation.py (1)
65-66: In harmony with the Force, these changes are!Updated consistently, all particle-related parameters are. From input to generated, the transition is complete:
- inputParticles → "particles_generated"
- outputTrackParameters → "params_particles_generated"
- inputTrackParameters → "params_particles_generated"
A clear path through the stages of particle processing, this creates.
Also applies to: 74-74
Examples/Scripts/Python/telescope_track_params_lookup_generation.py (2)
50-50: Consistent with the new ways, these changes are!To "particles_generated", renamed the parameters are. Throughout the codebase, harmony this brings.
Also applies to: 68-68
50-53: Removed preSelectParticles parameter, noticed I have!A significant architectural change, this represents. To separate stages of particle selection, moved this functionality has been. Verify the new selection mechanism's implementation, we must.
Examples/Scripts/Python/material_validation.py (1)
45-46: Consistent with the force, these changes are!Renamed parameters from "particles_input" to "particles_generated", align with the broader refactoring effort, they do. Maintain consistency across the codebase, this will.
Also applies to: 55-55
Examples/Scripts/Python/material_validation_itk.py (1)
52-53: Consistent naming conventions, maintain we must!Renamed parameters from "particles_input" to "particles_generated", follow the path of consistency, they do. Harmony in the codebase, this brings.
Also applies to: 62-62
Examples/Scripts/Python/propagation.py (1)
36-37: In balance with other changes, these modifications are!Renamed parameters from "particles_input" to "particles_generated", maintain consistency across the propagation code, they do. Clear path for particle tracking, this creates.
Also applies to: 53-53
CI/physmon/workflows/physmon_simulation.py (1)
60-60: Consistent naming, this change brings! Hmmmm.From "particles_input" to "particles_generated", renamed these parameters are. Better clarity about particle state in pipeline, this provides.
Also applies to: 69-69, 80-80, 95-95
Examples/Scripts/Python/full_chain_itk.py (1)
60-68: Wise separation of concerns, this is! Hmmmm.Early particle selection after generation, implemented well it is. Clear geometric and kinematic cuts, specified they are.
Examples/Scripts/Python/vertex_fitting.py (1)
36-36: Consistency in naming, achieved it is!Throughout the vertex fitting workflow, aligned with new naming convention, these changes are. Harmony in the codebase, this brings.
Also applies to: 71-71, 77-77
CI/physmon/workflows/physmon_trackfinding_1muon.py (1)
86-94: Different measurement threshold, notice I do!Different from other files, this configuration is:
- 9 measurements required, instead of 7
- Secondary particles, removed they are not
- Rest of configuration, consistent it remains
Verify these differences intentional they are, you must.
Consider standardizing selection criteria across all tracking workflows, unless specific requirements justify the differences.
Examples/Scripts/Python/ckf_tracks.py (3)
32-33: Approve the import changes, I do!Clear separation of concerns, these imports bring. A more modular approach to particle selection, they enable.
75-75: Consistent naming convention, this change follows!From "particles_input" to "particles_generated", the transition makes. More descriptive of the actual stage in the particle processing pipeline, it is.
94-101: A well-structured particle selection configuration, this is!Wise choices in selection criteria, you have made:
- Minimum pt of 0.5 GeV
- At least 9 measurements required
- Neutral particles, removed they are
Clear and maintainable, the code becomes.
CI/physmon/workflows/physmon_trackfinding_4muon_50vertices.py (1)
83-90: Stricter particle selection criteria, I observe!Higher pt threshold of 0.9 GeV, you have chosen. For a complex scenario with 4 muons and 50 vertices, appropriate this is. Clean track reconstruction, it shall enable.
CI/physmon/workflows/physmon_trackfinding_ttbar_pu200.py (2)
66-72: A wise geometric selection at generator level, you implement!Restrict particles to:
- rho < 24 mm
- |z| < 1.0 m
Early selection at generator level, efficient processing it ensures. For high pileup scenario of ttbar with PU200, crucial this is.
89-96: Complementary kinematic selection at digitization level, you add!Work together in harmony, the two selection stages do:
- First, geometric boundaries they respect
- Then, kinematic quality they ensure
A balanced approach to particle selection, achieved it is.
Examples/Scripts/Optimization/ckf.py (1)
183-190: Consistent with base implementation, these selection criteria are!Match the criteria in ckf_tracks.py, they do:
- Same pt threshold of 0.5 GeV
- Same measurement requirement of 9
- Same neutral particle removal
Consistency across optimization and standard scripts, maintain we must. Debugging and maintenance, easier it makes.
Examples/Python/tests/test_reader.py (2)
56-57: Approve the renaming of particle output, I do!More descriptive the new naming is, better reflecting the generation stage of particles. Consistent with the PR's objective of splitting particle selection, this change is.
Also applies to: 62-62
320-322: Wise changes in EDM4hepReader configuration, these are!Clear separation between generation and simulation stages, the new naming provides. With the PR's objective of splitting particle selection, this aligns perfectly.
Also applies to: 332-332
Examples/Python/tests/conftest.py (2)
214-215: Consistent naming in parameter extraction, observe I do!Well aligned with the new particle selection stages, these changes are. Clarity and consistency, they bring to the codebase.
228-228: Harmonious with the new naming convention, this change is!Propagation algorithm now speaks the same language as other components, it does.
Examples/Scripts/Python/full_chain_odd.py (3)
19-24: Strong with the Force, these new selection functions are!Clear separation of particle selection stages, these additions provide:
- Generation stage:
addGenParticleSelection- Simulation stage:
addSimParticleSelection- Digitization stage:
addDigiParticleSelection
Line range hint
198-208: Well structured, the EDM4hep particle selection is!Clear separation between generated and simulated particles, I observe. Wise selection criteria for simulation stage, you have chosen:
- Rho and Z constraints for tracking volume
- Reasonable eta range
- Minimum pT threshold
- Neutral particle removal
Also applies to: 211-219
311-319: Masterful digitization selection criteria, these are!Appropriate for the digitization stage, these requirements are:
- Higher pT threshold (1 GeV)
- Reasonable eta range
- Minimum measurement count
- Neutral particle removal
Examples/Scripts/Python/full_chain_odd_LRT.py (1)
19-24: Mirror the standard chain's wisdom, these changes do!Consistent implementation of three-stage particle selection between standard and LRT variants, I sense. Strong consistency in the Force, this brings.
Examples/Python/python/acts/examples/simulation.py (4)
54-79: Clean and well-structured implementation, hmmmm!A helper function well-designed, it is. All particle selection parameters, it handles with care.
383-410: Wise changes in particle selection after generation, I sense!Clear and descriptive naming convention, you have chosen. The force of modular design, strong with this one it is.
657-685: Duplicate code, I sense. Refactor we must!Similar structure between particle selection functions, there is. A single function with a string argument, more elegant it would be.
Consider this refactor pattern:
def addParticleSelection(s: acts.examples.Sequencer, config: ParticleSelectorConfig, kind: str = "simulated", # or "generated" or "digitized" logLevel: Optional[acts.logging.Level] = None) -> None: customLogLevel = acts.examples.defaultLogging(s, logLevel) selector = acts.examples.ParticleSelector( **acts.examples.defaultKWArgs(**_getParticleSelectionKWargs(config)), level=customLogLevel(), inputParticles=f"particles_{kind}", outputParticles=f"tmp_particles_{kind}_selected", ) s.addAlgorithm(selector) s.addWhiteboardAlias("particles_selected", selector.config.outputParticles) s.addWhiteboardAlias( f"particles_{kind}_selected", selector.config.outputParticles ) # Then provide convenience functions: def addGenParticleSelection(*args, **kwargs): return addParticleSelection(*args, kind="generated", **kwargs) def addSimParticleSelection(*args, **kwargs): return addParticleSelection(*args, kind="simulated", **kwargs) def addDigiParticleSelection(*args, **kwargs): return addParticleSelection(*args, kind="digitized", **kwargs)
778-807: Particle selection after digitization, well implemented it is!Consistent with other selection functions while handling digitization specifics, this implementation is. The force of good design practices, strong with this one it is.
Examples/Scripts/Python/full_chain_test.py (1)
404-404: Aligned with new naming convention, this change is!From "particles_input" to "particles_generated", the path leads. Consistency in the force, I sense.
Examples/Python/tests/test_examples.py (1)
407-408: Test coverage, maintained and enhanced it is!New particle selection functionality, properly tested it is. The path of thorough testing, you follow.
Also applies to: 440-448
|
blocked by
Summary by CodeRabbit
Release Notes: Particle Processing and Naming Convention Update
Overview
This release introduces significant improvements to particle processing workflows, focusing on more precise particle selection and standardized naming conventions.
Key Changes
Particle Selection
addGenParticleSelection: Selects particles during generationaddSimParticleSelection: Selects particles during simulationaddDigiParticleSelection: Selects particles during digitizationNaming Conventions
particles_inputtoparticles_generatedParticle Filtering
Impact