Implement Celeritas-DD4hep integration plugin #1756
Draft
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
rahmans1
added
enhancement
Test summary 5 735 files 9 221 suites 7m 2s ⏱️ Results for commit aeadd13. ♻️ This comment has been updated with latest results. |
sethrj
reviewed
Apr 21, 2025
rahmans1
force-pushed
the
feature-dd4hep-integration-plugin
branch
from
b3e3fd5 to
ab67f02
Compare
rahmans1
force-pushed
the
feature-dd4hep-integration-plugin
branch
from
ccc0fca to
988047d
Compare
rahmans1
force-pushed
the
feature-dd4hep-integration-plugin
branch
from
6369a78 to
2d9531e
Compare
Contributor
Author
|
@sethrj This implementation is pretty preliminary based on streamlining the multiple test beds I was working on. I decided to use the OpenDataDetector from acts-project for validation as it should be a simpler approximation to something like the EIC ePIC geometry. Build and run instructions are inside the steeringFile.py. I can see celeritas logs but haven't validated the output yet. |
wdconinc
reviewed
May 29, 2025
wdconinc
reviewed
May 29, 2025
Contributor
Author
|
Running with gdb to investigate segfaults gives: |
Contributor
Author
|
Building Geant4 Building DD4hep Building celeritas Running with gdb Complete backtrace |
Contributor
Author
|
sethrj
reviewed
Jun 26, 2025
Contributor
Author
|
With changes to the hit processor (#1835), we can see some hits in the ODD ecal if we ignore saving MC truth particle info in edm4hep output.
Now we need to consider the following:
|
6 tasks
sethrj
reviewed
Oct 15, 2025
Member
sethrj
left a comment
sethrj
left a comment
There was a problem hiding this comment.
This is really great! Excellent work, and I have hope that we can continue to make the integration even tighter.
| inline DDcelerRunAction(Geant4Context* ctxt, std::string const& nam); | ||
|
|
||
| // Default destructor | ||
| ~DDcelerRunAction(); |
Member
There was a problem hiding this comment.
I think virtual is automatic if the base class is; @esseivaju has taught me using override (if it's not the first virtual base class), whichI think is the best choice here.
| workflow_call: | ||
| push: | ||
| branches: | ||
| - feature-dd4hep-integration-plugin |
Member
There was a problem hiding this comment.
Note to self: will update this so that we run at the end of each PR
| if (!tfile || tfile->IsZombie()) | ||
| { | ||
| std::cerr << "Error: Cannot open file " << file.Data() << std::endl; | ||
| return; |
Member
There was a problem hiding this comment.
This is now being done as part of the GHA workflow via a root invocation; is that or a direct build preferable? (I would tend toward direct build but I'm not a ROOT guy.)
example/ddceler/steeringFile.py
Outdated
| @@ -0,0 +1,59 @@ | |||
| """ | |||
Member
There was a problem hiding this comment.
Please add the Celeritas copyright header to this file and other relevant ones.
Comment on lines
+54
to
+56
| CELER_VALIDATE(!overlayed_obj->magnetic_components.empty(), | ||
| << "No magnetic field components found in DD4hep field " | ||
| "description."); |
Member
There was a problem hiding this comment.
It's OK if there's no magnetic field; we can just use a constant of zero.
Comment on lines
+101
to
+103
| double min_step = 1e-6 * celer_mm; | ||
| double delta_chord = 0.025 * celer_mm; | ||
| double delta_intersection = 1e-5 * celer_mm; |
Member
There was a problem hiding this comment.
Does DD4HEP define these defaults somewhere? It would be good to annotate where they should be copied from
Comment on lines
+165
to
+166
| // Save diagnostic file to a unique name | ||
| opts.output_file = "trackingmanager-offload.out.json"; |
Member
There was a problem hiding this comment.
This should be a user input parameter, or be based on the dd4hep run name
| } // namespace sim | ||
| } // namespace dd4hep | ||
|
|
||
| DECLARE_GEANT4ACTION(DDcelerTMI) |
Member
There was a problem hiding this comment.
A little annotation about what this does would be good (it doesn't start with DDG4_ or anything to indicate it's coming from that library).
src/ddceler/DDcelerTMI.hh
Outdated
Comment on lines
33
to
34
| int m_maxNumTracks; | ||
| int m_initCapacity; |
Member
There was a problem hiding this comment.
These should be default initialized. Maybe use -1, and then VALIDATE that they're greater than zero when you first set up, to ensure that the user has set them to appropriate values?
There was a problem hiding this comment.
Could also use unsigned here since both num tracks and capacity are inherently positive (and then init at 0 since that's what the capacity presumably is at startup).
This commit means that you must (please!) install pre-commit on your development machine and run `pre-commit install --install-hooks`. For more information, see https://celeritas-project.github.io/celeritas/user/development/style.html#formatting Autogenerated: https://pre-commit.ci
…ponents files - Set CMAKE_LIBRARY_OUTPUT_DIRECTORY to output plugin libraries to build/lib - Configure RPATH to prioritize build directory libraries over installed ones - Use BUILD_RPATH to point to build/lib during development - Use INSTALL_RPATH for installation directory - Add --disable-new-dtags linker flag to use RPATH instead of RUNPATH for correct library search priority - Add custom target to copy .components files to build/lib for DD4hep plugin discovery This ensures the plugins link against the newly built Celeritas libraries instead of any pre-existing installation (e.g., /opt/local). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
When VecGeom is enabled, add accel_final library with --no-as-needed linker flag to ensure proper symbol resolution for VecGeom geometry. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
sethrj
reviewed
Nov 6, 2025
src/ddceler/DDcelerTMI.hh
Outdated
Comment on lines
57
to
63
| DDcelerTMI::DDcelerTMI(Geant4Context* ctxt, std::string const& nam) | ||
| : Geant4PhysicsList(ctxt, nam) | ||
| { | ||
| declareProperty("MaxNumTracks", m_maxNumTracks); | ||
| declareProperty("InitCapacity", m_initCapacity); | ||
| declareProperty("IgnoreProcesses", m_ignoreProcesses); | ||
| } |
Member
There was a problem hiding this comment.
This should go in the .cc file (there is, after all, nowhere to inline it to!). Once we transition to an inp::FrameworkInput, we can specify even more of these (and maybe semi-automate it). Since should be set before the run initialization, we could do this setup in the TMI or RunAction.
|
|
||
| // Register Celeritas tracking manager | ||
| auto& tmi = TMI::Instance(); | ||
| physics->RegisterPhysics(new celeritas::TrackingManagerConstructor(&tmi)); |
Member
There was a problem hiding this comment.
Since this can throw, you may want to write (after including accel/ExceptionConverter.hh):
Suggested change
| physics->RegisterPhysics(new celeritas::TrackingManagerConstructor(&tmi)); | |
| CELER_TRY_HANDLE( | |
| physics->RegisterPhysics(new celeritas::TrackingManagerConstructor(&tmi)), | |
| ExceptionConverter{"ddceler.construct-physics"}); |
and same below with your call to SetOptions(makeOptions())
…ywhere" This reverts commit 9120784.
Removed push trigger for feature-dd4hep-integration-plugin branch.
This reverts commit 1b437a6.
Updated the workflow to run Celeritas and Geant4 simulations in parallel, capturing logs for both and checking exit codes.
Removed sourcing of /etc/profile in the CI workflow.
Added error handling and improved logging for dd4hep integration tests.
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- Rename RUNNER to runner (lowercase for module-level variable) - Rename setupPhysics to setup_physics (snake_case for functions) - Reformat docstring to be more concise - Split long comment line to stay within 79 character limit - Add function docstring - Move DDG4 imports inside function scope 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- Update DDcelerTMI constructor parameter from nam to name - Update DDcelerRunAction constructor parameter from nam to name - Remove 'nam' from codespell-ignore.txt 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
…m/rahmans1/celeritas into feature-dd4hep-integration-plugin
Default-initialize m_maxNumTracks and m_initCapacity to 0 and add validation in makeOptions() to ensure they are set to positive values before use. This addresses PR feedback to ensure users explicitly configure these parameters. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
rahmans1
commented
Dec 4, 2025
| DDG4_DEFINE_ACTION_CONSTRUCTORS(DDcelerTMI); | ||
|
|
||
| int m_maxNumTracks{0}; | ||
| int m_initCapacity{0}; |
Contributor
Author
There was a problem hiding this comment.
@wdconinc if i try to set it to unsigned, i get the following error
PhysicsList +++ Verbosity: 1
BasicGrammar ERROR FAILED to look up non existent registry: 46C292BB247E0FFD [unsigned int]
DDSim.Helper.Physics ERROR Exception in UserFunction: <cppyy.gbl.std.runtime_error object at 0x57907f1714d0>
Traceback (most recent call last):
File "/opt/local/lib/python3.12/site-packages/DDSim/Helper/Physics.py", line 221, in setupPhysics
func(kernel)
File "/home/srahman1/celeritas-dd4hep/celeritas-orange/example/ddceler/steeringFile.py", line 51, in setup_physics
celer_phys = PhysicsList(kernel, str("DDcelerTMI"))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/opt/local/lib/python3.12/site-packages/DDG4.py", line 242, in PhysicsList
return Interface.createPhysicsList(kernel, str(nam))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
cppyy.gbl.std.runtime_error: static dd4hep::sim::PhysicsListHandle dd4hep::sim::Geant4ActionCreation::createPhysicsList(dd4hep::sim::KernelHandle& kernel, const string& name_type) =>
runtime_error: BasicGrammar: FAILED to look up non existent registry: 46C292BB247E0FFD [unsigned int]
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "/opt/local/bin/ddsim", line 25, in <module>
sys.exit(RUNNER.run())
^^^^^^^^^^^^
File "/opt/local/lib/python3.12/site-packages/DDSim/DD4hepSimulation.py", line 519, in run
_phys = self.physics.setupPhysics(kernel, name=self.physicsList)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/opt/local/lib/python3.12/site-packages/DDSim/Helper/Physics.py", line 224, in setupPhysics
raise RuntimeError("Exception in UserFunction: %r" % e)
RuntimeError: Exception in UserFunction: <cppyy.gbl.std.runtime_error object at 0x57907f1714d0>
There was a problem hiding this comment.
Is this caused by the unsigned int change? Doesn't look like that to me.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
See issue #1731