chore: update to newest traccc+detray+friends #4068
Conversation
WalkthroughUpdated, the project has been. Multiple dependency versions in CMakeLists.txt are incremented. In addition, header includes and class definitions in the Detray modules have been refined. The execution flow in the Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant DP as DetrayPropagator
participant PC as Propagator
U->>DP: Call execute()
DP->>DP: Create context (dCtx) and config (dCfg)
DP->>PC: Instantiate Propagator with dCfg
PC-->>DP: Return state/result
DP->>U: Return propagation result
Suggested labels
Suggested reviewers
Poem
Tip 🌐 Web search-backed reviews and chat
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
🔇 Additional comments (5)
✨ 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
Infrastructure
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Examples/Algorithms/Traccc/include/ActsExamples/Traccc/DetrayPropagator.hpp (1)
114-124: Refactor duplicate configuration setup, we should!Similar code in sterile and non-sterile paths, I see. Extract common configuration setup to a helper function, we must.
template <typename stepper_t, typename detray_store_t> class DetrayPropagator : public PropagatorInterface { + private: + template<typename Propagator> + typename Propagator::state createPropagationState( + const detray::free_track_parameters<DetrayAlgebraType>& track) const { + using Context = typename Propagator::state::context_type; + Context dCtx{}; + using Config = detray::propagation::config; + Config dCfg{}; + + return typename Propagator::state(track, m_cfg.detrayStore->detector, dCtx); + }Also applies to: 169-179
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
CMakeLists.txt(1 hunks)Examples/Algorithms/Traccc/include/ActsExamples/Traccc/DetrayPropagator.hpp(3 hunks)Examples/Python/src/Detray.cpp(2 hunks)Plugins/Detray/include/Acts/Plugins/Detray/DetrayConversionUtils.hpp(2 hunks)Plugins/Detray/include/Acts/Plugins/Detray/DetrayConverter.hpp(2 hunks)cmake/ActsExternSources.cmake(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (13)
- GitHub Check: merge-sentinel
- GitHub Check: CI Bridge / build_gnn_tensorrt
- GitHub Check: CI Bridge / build_linux_ubuntu
- GitHub Check: CI Bridge / build_exatrkx
- GitHub Check: CI Bridge / build_exatrkx_cpu
- GitHub Check: CI Bridge / clang_tidy
- GitHub Check: unused_files
- GitHub Check: macos
- GitHub Check: missing_includes
- GitHub Check: linux_ubuntu_extra (ubuntu2204, 20, clang++)
- GitHub Check: linux_ubuntu_extra (ubuntu2204, 20, g++)
- GitHub Check: linux_ubuntu
- GitHub Check: build_debug
🔇 Additional comments (6)
Plugins/Detray/include/Acts/Plugins/Detray/DetrayConverter.hpp (1)
72-72: Wise choice, using concepts instead of type traits, it is!From implementation details to clear concepts, moved we have. Clearer intent and better error messages, this change brings. Approve this modification, I do.
Examples/Python/src/Detray.cpp (1)
10-10: Approve the specialization of detector class, I do!From generic template to specific DetrayHostDetector, the change moves. Clarity and type safety, this brings to our code.
Also applies to: 36-37
cmake/ActsExternSources.cmake (1)
9-9: Verify the integrity of new dependency versions, we must!Updated SHA256 hashes for multiple dependencies, I see. Ensure their correctness, we should.
Also applies to: 16-16, 23-23, 30-30, 37-37
Plugins/Detray/include/Acts/Plugins/Detray/DetrayConversionUtils.hpp (1)
18-20: Improved type safety through explicit metadata handling, I sense!Using array for metadata, more precise our types become. A wise choice, this is.
Also applies to: 29-31
Examples/Algorithms/Traccc/include/ActsExamples/Traccc/DetrayPropagator.hpp (1)
21-21: Updated includes, clarity they bring!Proper organization of headers, maintain we must.
Also applies to: 24-24
CMakeLists.txt (1)
272-276: Updated dependency versions, they are!
Your changes update the versions for_acts_detray,_acts_traccc,_acts_covfie,_acts_vecmem, and_acts_algebrapluginsto the latest values. Ensure you have verified that the updated SHA256 hashes in the external sources file match these versions, and that all dependent modules remain compatible, you must.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Examples/Algorithms/Traccc/include/ActsExamples/Traccc/DetrayPropagator.hpp (1)
114-124: Consider configuration customization, you must!Empty configuration object created, I see. Opportunity for customization missed, it is. Properties like step size limits or navigation tolerances, configure you should.
DetrayConfig dCfg{}; +// Configure propagation properties +dCfg.navigation_tolerance = 1e-5; +dCfg.step_size_limit = 100.; // Example value in mm
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Examples/Algorithms/Traccc/include/ActsExamples/Traccc/DetrayPropagator.hpp(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (18)
- 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: CI Bridge / build_exatrkx
- GitHub Check: CI Bridge / build_exatrkx_cpu
- GitHub Check: merge-sentinel
- GitHub Check: unused_files
- GitHub Check: missing_includes
- GitHub Check: CI Bridge / clang_tidy
- GitHub Check: build_debug
- GitHub Check: CI Bridge / build_gnn_tensorrt
- GitHub Check: linux_ubuntu_extra (ubuntu2204, 20, clang++)
- GitHub Check: macos
- GitHub Check: linux_ubuntu_extra (ubuntu2204, 20, g++)
- GitHub Check: linux_ubuntu
🔇 Additional comments (1)
Examples/Algorithms/Traccc/include/ActsExamples/Traccc/DetrayPropagator.hpp (1)
21-21: Approve these changes, I do!Wise additions these includes are. Support the new configuration structure, they do.
Also applies to: 24-24
Examples/Algorithms/Traccc/include/ActsExamples/Traccc/DetrayPropagator.hpp
Outdated
Show resolved
Hide resolved
|
Hey @stephenswat @niermann999 - there seems to be building problems coming form both |
acts-project/acts#4068 exposes a mistake in our sanity checks, revealing that there are some missing typename keywords. This commit fixes those errors by adding the necessary keywords.
acts-project/acts#4068 exposes a mistake in our sanity checks, revealing that there are some missing typename keywords. This commit fixes those errors by adding the necessary keywords.
|
|
There's a compilation error on clang 14: Is there a clang14 build over on detray? @asalzburger @niermann999 |
I think what we check is AppleClang 15 |
|
@niermann999 it's not that obvious what this maps to. We should probably actually add a non-Apple clang build to detray CI at some point. |
The new detray version should have a fix for that though, right? |
I hope so, but I think Paul right, if ACTS checks it, detray should, too. Otherwise, this story will continue to repeat |
This PR updates to the latest
tracccversions and adapts the client code.It also adapts the Covfie pytests to the new array return type.
--- END COMMIT MESSAGE ---
Thanks @stephenswat for the
traccctalk.Any further description goes here, @-mentions are ok here!
feat,fix,refactor,docs,choreandbuildtypes.Summary by CodeRabbit
declareCovfieFieldfunction to support different scalar types and added a new array class for element access.