-
Notifications
You must be signed in to change notification settings - Fork 180
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
feat: detray material collection #4065
feat: detray material collection #4065
Conversation
WalkthroughIn the realm of particle physics code, two significant modifications have emerged. The Changes
Sequence DiagramsequenceDiagram
participant Propagator
participant MaterialTracer
participant DetrayStore
Propagator->>DetrayStore: Initialize memory resource
Propagator->>MaterialTracer: Create tracer state
MaterialTracer-->>Propagator: Track material properties
Propagator->>Propagator: Record material information (sX0, sL0)
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
Examples/Algorithms/Traccc/include/ActsExamples/Traccc/DetrayPropagator.hpp (3)
109-111
: Extended actor chain, a subtle approach this is.
Performance overhead in mind, keep you must. Measure changes carefully as the code matures, yes.
118-121
: MaterialTracer state initialization, wise it seems.
Null memory resource, handle gracefully you should, if ever needed. Offer assistance, I can.
146-151
: Material retrieval, a good first step it is.
Further data logged, consider you might. Broad coverage in tests, ensure.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Examples/Algorithms/Traccc/include/ActsExamples/Traccc/DetrayPropagator.hpp
(3 hunks)Plugins/Detray/src/DetrayMaterialConverter.cpp
(0 hunks)
💤 Files with no reviewable changes (1)
- Plugins/Detray/src/DetrayMaterialConverter.cpp
⏰ Context from checks skipped due to timeout of 90000ms (18)
- GitHub Check: CI Bridge / lcg_106a: [alma9, clang16]
- GitHub Check: CI Bridge / lcg_106a: [alma9, gcc14]
- GitHub Check: CI Bridge / lcg_106a: [alma9, gcc13]
- GitHub Check: CI Bridge / lcg_105: [alma9, clang16]
- GitHub Check: CI Bridge / lcg_105: [alma9, gcc13]
- GitHub Check: CI Bridge / linux_ubuntu_2204_clang
- GitHub Check: CI Bridge / linux_ubuntu_2204
- GitHub Check: CI Bridge / build_linux_ubuntu
- GitHub Check: missing_includes
- GitHub Check: macos
- GitHub Check: linux_ubuntu_extra (ubuntu2204, 20, clang++)
- GitHub Check: linux_ubuntu_extra (ubuntu2204, 20, g++)
- GitHub Check: linux_ubuntu
- GitHub Check: build_debug
- GitHub Check: CI Bridge / build_gnn_tensorrt
- GitHub Check: CI Bridge / build_exatrkx
- GitHub Check: CI Bridge / clang_tidy
- GitHub Check: CI Bridge / build_exatrkx_cpu
🔇 Additional comments (3)
Examples/Algorithms/Traccc/include/ActsExamples/Traccc/DetrayPropagator.hpp (3)
23-23
: New include, well-chosen it is.
Include for material validation utilities, strongly aligned with the tracer approach, hmm.
105-107
: Freshly defined MaterialTracer, impressed I am.
Ensure your double precision and vecmem usage meet performance demands, you should.
123-123
: Propagation with actor states, verify concurrency paths, you must.
If multi-threaded scenarios arise, mind potential thread-safety of your actors, you should.
|
This PR adds the possibility to record material from detray.
Currently, the material_tracer does not return enough information to fill the individual steps sense fully, this can come at a later stage.
--- END COMMIT MESSAGE ---
@beomki-yeo - the material maps for the ITk look actually quite good, it seems!
Any further description goes here, @-mentions are ok here!
feat
,fix
,refactor
,docs
,chore
andbuild
types.Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Refactor