refactor(Examples): Evt gen. exchanges data via HepMC3 types #4171
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughMuch has changed, yes. The HepMC3 example build option, removed it is, along with its conditional logic in CMake. Updated, timing utilities are—with ScopedTimer refined and a new AveragingScopedTimer born. Event generators now return HepMC3::GenEvent via direct references instead of pointer indirection. In IO and Python bindings, function signatures and linking are shifted to modern practices, while tests and documentation adjust accordingly. Even Barcode’s output gains a friend function. Harmonize the project now, these changes do. Changes
Sequence Diagram(s)sequenceDiagram
participant AC as AlgorithmContext
participant EG as EventGenerator
participant GE as HepMC3::GenEvent
participant Log as Logger
AC->>EG: Call read(ctx)
EG->>GE: Create new GenEvent, set units
EG->>EG: Execute handleVertex & convertHepMC3ToInternalEdm
EG->>Log: Log event details
EG-->>AC: Return shared_ptr to GenEvent
sequenceDiagram
participant AC as AlgorithmContext
participant HW as HepMC3AsciiWriter
participant GE as HepMC3::GenEvent
participant FS as FileSystem
AC->>HW: Call writeT(ctx, event)
HW->>GE: Process event write (perEvent check)
alt Per Event Write
HW->>FS: Create unique filename and write event
else Single File Write
HW->>FS: Lock mutex and write to shared outputPath
end
HW->>HW: Finalize writer resources
HW-->>AC: Return ProcessCode status
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🪧 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
Component - Core
paulgessinger
changed the title
paulgessinger
force-pushed
the
refactor/evgen-hepmc3
branch
2 times, most recently
from
d818fdb to
e96a193
Compare
paulgessinger
force-pushed
the
refactor/evgen-hepmc3
branch
2 times, most recently
from
72b4ef1 to
984ebda
Compare
paulgessinger
added
🛑 blocked
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (14)
Examples/Io/HepMC3/include/ActsExamples/Io/HepMC3/HepMC3Writer.hpp (4)
15-15: Valued<filesystem>inclusion, good it is.
Needed forstd::filesystem::pathusage, yes. Ensure portability is carefully proven, you must, across various compilers.
30-35: Separate paths, unify them well.
Replacing multiple string members withstd::filesystem::path, quite modern and more robust. Keep cross-platform edge cases in your mind, you must.
46-47: Public constructor and destructor, an apt pairing they are.
Observe carefully if default destructor needed only, or custom logic you hold. Freed your resources must be, or break it shall.
58-58:finalize()method, a finishing stroke indeed!
Proper closure or summary tasks for the writer, you accommodate. Implement or log any errors in final stages, prudent it is.Examples/Algorithms/GeneratorsPythia8/ActsExamples/Generators/Pythia8ProcessGenerator.cpp (1)
150-151: Conditional write of event, well done it is.Consider error handling if writing fails, though minimal risk it might be.
Examples/Io/HepMC3/src/HepMC3Writer.cpp (2)
19-34: Check for output path you do, wise it is!
Empty path, if found, an exception you throw. Perfectly correct. Consider auto-creating the directory, you could, as a small improvement for usability.
36-36: Default destructor, yes. But finalize, it does not call.
If finalize not invoked, open file handle linger, it may. Ensure closure in destructor, a safer approach could be.Examples/Algorithms/Generators/ActsExamples/Generators/ParametricParticleGenerator.cpp (1)
114-114: Commented-out label, found I have.
Remove or keep you may, but ensure clarity, you must.Examples/Python/tests/test_hepmc3.py (4)
113-113: Unused import 'HepMC3OutputConverter'?
Remove or uncomment the usage, best to avoid unused imports, yes.- HepMC3OutputConverter,🧰 Tools
🪛 Ruff (0.8.2)
113-113:
acts.examples.hepmc3.HepMC3OutputConverterimported but unusedRemove unused import:
acts.examples.hepmc3.HepMC3OutputConverter(F401)
204-204: Unused import repeated, hmm?
Indeed, remove or employ it in the code, you must.- HepMC3OutputConverter,🧰 Tools
🪛 Ruff (0.8.2)
204-204:
acts.examples.hepmc3.HepMC3OutputConverterimported but unusedRemove unused import:
acts.examples.hepmc3.HepMC3OutputConverter(F401)
256-257: “to_dot” import not used, I see.
Commented out the usage is. Remove import if no longer needed, or restore usage.- from pyhepmc.view import to_dot🧰 Tools
🪛 Ruff (0.8.2)
256-256:
pyhepmc.view.to_dotimported but unused; consider usingimportlib.util.find_specto test for availability(F401)
261-261: Loop variable “evt” you do not use, hmm.
Rename_evtyou can, to reflect the ignoring of it.- for evt in f: + for _evt in f:🧰 Tools
🪛 Ruff (0.8.2)
261-261: Loop control variable
evtnot used within loop bodyRename unused
evtto_evt(B007)
Examples/Algorithms/Generators/ActsExamples/Generators/EventGenerator.cpp (2)
88-156: Read method grows large, reorganize or split you might.
Maintainability improved, it would be.
158-205: Particle merging logic, memory usage mind you should.
For bigger events, chunking or streaming consider.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (34)
CMakeLists.txt(1 hunks)Core/include/Acts/Utilities/ScopedTimer.hpp(1 hunks)Core/src/Utilities/CMakeLists.txt(1 hunks)Core/src/Utilities/ScopedTimer.cpp(1 hunks)Examples/Algorithms/Geant4HepMC/CMakeLists.txt(1 hunks)Examples/Algorithms/Generators/ActsExamples/Generators/EventGenerator.cpp(3 hunks)Examples/Algorithms/Generators/ActsExamples/Generators/EventGenerator.hpp(4 hunks)Examples/Algorithms/Generators/ActsExamples/Generators/ParametricParticleGenerator.cpp(3 hunks)Examples/Algorithms/Generators/ActsExamples/Generators/ParametricParticleGenerator.hpp(1 hunks)Examples/Algorithms/Generators/CMakeLists.txt(1 hunks)Examples/Algorithms/GeneratorsPythia8/ActsExamples/Generators/Pythia8ProcessGenerator.cpp(5 hunks)Examples/Algorithms/GeneratorsPythia8/ActsExamples/Generators/Pythia8ProcessGenerator.hpp(1 hunks)Examples/Algorithms/GeneratorsPythia8/CMakeLists.txt(1 hunks)Examples/Algorithms/HepMC/src/HepMCProcessExtractor.cpp(4 hunks)Examples/Framework/src/EventData/SimParticle.cpp(1 hunks)Examples/Io/HepMC3/CMakeLists.txt(1 hunks)Examples/Io/HepMC3/include/ActsExamples/Io/HepMC3/HepMC3Event.hpp(1 hunks)Examples/Io/HepMC3/include/ActsExamples/Io/HepMC3/HepMC3OutputConverter.hpp(1 hunks)Examples/Io/HepMC3/include/ActsExamples/Io/HepMC3/HepMC3Particle.hpp(1 hunks)Examples/Io/HepMC3/include/ActsExamples/Io/HepMC3/HepMC3Vertex.hpp(1 hunks)Examples/Io/HepMC3/include/ActsExamples/Io/HepMC3/HepMC3Writer.hpp(2 hunks)Examples/Io/HepMC3/src/HepMC3Event.cpp(10 hunks)Examples/Io/HepMC3/src/HepMC3OutputConverter.cpp(1 hunks)Examples/Io/HepMC3/src/HepMC3Particle.cpp(1 hunks)Examples/Io/HepMC3/src/HepMC3Vertex.cpp(5 hunks)Examples/Io/HepMC3/src/HepMC3Writer.cpp(1 hunks)Examples/Python/python/acts/examples/simulation.py(2 hunks)Examples/Python/src/Generators.cpp(1 hunks)Examples/Python/src/HepMC3.cpp(2 hunks)Examples/Python/tests/test_hepmc3.py(1 hunks)Examples/Python/tests/test_writer.py(0 hunks)Fatras/include/ActsFatras/EventData/Barcode.hpp(1 hunks)docs/getting_started.md(0 hunks)pytest.ini(1 hunks)
💤 Files with no reviewable changes (2)
- docs/getting_started.md
- Examples/Python/tests/test_writer.py
🧰 Additional context used
🧬 Code Definitions (12)
Examples/Algorithms/GeneratorsPythia8/ActsExamples/Generators/Pythia8ProcessGenerator.hpp (4)
Examples/Algorithms/GeneratorsPythia8/ActsExamples/Generators/Pythia8ProcessGenerator.cpp (4)
rng(39-52)rng(55-55)rng(121-233)rng(121-122)Examples/Algorithms/Generators/ActsExamples/Generators/EventGenerator.hpp (3)
rng(61-61)rng(72-72)rng(85-85)Examples/Algorithms/Generators/ActsExamples/Generators/ParametricParticleGenerator.hpp (1)
rng(73-73)Examples/Algorithms/Generators/ActsExamples/Generators/ParametricParticleGenerator.cpp (2)
rng(92-152)rng(92-93)
Examples/Algorithms/GeneratorsPythia8/ActsExamples/Generators/Pythia8ProcessGenerator.cpp (4)
Examples/Algorithms/GeneratorsPythia8/ActsExamples/Generators/Pythia8ProcessGenerator.hpp (1)
rng(69-69)Examples/Algorithms/Generators/ActsExamples/Generators/ParametricParticleGenerator.cpp (2)
rng(92-152)rng(92-93)Examples/Algorithms/Generators/ActsExamples/Generators/EventGenerator.hpp (3)
rng(61-61)rng(72-72)rng(85-85)Examples/Algorithms/Generators/ActsExamples/Generators/ParametricParticleGenerator.hpp (1)
rng(73-73)
Examples/Io/HepMC3/src/HepMC3Writer.cpp (3)
Examples/Io/HepMC3/include/ActsExamples/Io/HepMC3/HepMC3Writer.hpp (4)
HepMC3AsciiWriter(45-45)HepMC3AsciiWriter(47-47)m_cfg(61-61)ctx(55-56)Examples/Algorithms/Generators/ActsExamples/Generators/EventGenerator.hpp (3)
m_cfg(139-139)ctx(136-136)ctx(144-145)Examples/Io/HepMC3/include/ActsExamples/Io/HepMC3/HepMC3OutputConverter.hpp (2)
m_cfg(34-34)ctx(36-36)
Examples/Algorithms/Generators/ActsExamples/Generators/ParametricParticleGenerator.cpp (7)
Examples/Algorithms/Generators/ActsExamples/Generators/EventGenerator.hpp (4)
rng(61-61)rng(72-72)rng(85-85)m_cfg(139-139)Examples/Algorithms/Generators/ActsExamples/Generators/ParametricParticleGenerator.hpp (1)
rng(73-73)Examples/Io/HepMC3/include/ActsExamples/Io/HepMC3/HepMC3OutputConverter.hpp (1)
m_cfg(34-34)Examples/Io/HepMC3/include/ActsExamples/Io/HepMC3/HepMC3Writer.hpp (1)
m_cfg(61-61)Examples/Framework/include/ActsExamples/Framework/Sequencer.hpp (1)
m_cfg(153-153)Examples/Io/HepMC3/src/HepMC3Particle.cpp (4)
momentum(61-67)momentum(61-61)momentum(86-90)momentum(86-87)Fatras/include/ActsFatras/EventData/Particle.hpp (2)
hypot(210-210)m_mass(166-166)
Examples/Python/src/HepMC3.cpp (2)
Examples/Io/HepMC3/src/HepMC3OutputConverter.cpp (1)
HepMC3OutputConverter(22-40)Examples/Io/HepMC3/include/ActsExamples/Io/HepMC3/HepMC3OutputConverter.hpp (1)
HepMC3OutputConverter(32-32)
Examples/Algorithms/Generators/ActsExamples/Generators/EventGenerator.cpp (3)
Examples/Algorithms/Generators/ActsExamples/Generators/EventGenerator.hpp (6)
rng(61-61)rng(72-72)rng(85-85)ctx(136-136)ctx(144-145)genVertex(147-151)Examples/Io/HepMC3/include/ActsExamples/Io/HepMC3/HepMC3Writer.hpp (1)
ctx(55-56)Examples/Algorithms/Generators/ActsExamples/Generators/ParametricParticleGenerator.cpp (2)
rng(92-152)rng(92-93)
Examples/Io/HepMC3/src/HepMC3OutputConverter.cpp (3)
Examples/Io/HepMC3/include/ActsExamples/Io/HepMC3/HepMC3OutputConverter.hpp (3)
HepMC3OutputConverter(32-32)m_cfg(34-34)ctx(36-36)Examples/Io/HepMC3/include/ActsExamples/Io/HepMC3/HepMC3Writer.hpp (2)
m_cfg(61-61)ctx(55-56)Examples/Algorithms/Generators/ActsExamples/Generators/EventGenerator.hpp (3)
m_cfg(139-139)ctx(136-136)ctx(144-145)
Examples/Algorithms/Generators/ActsExamples/Generators/ParametricParticleGenerator.hpp (5)
Examples/Algorithms/Generators/ActsExamples/Generators/ParametricParticleGenerator.cpp (2)
rng(92-152)rng(92-93)Examples/Algorithms/GeneratorsPythia8/ActsExamples/Generators/Pythia8ProcessGenerator.cpp (4)
rng(39-52)rng(55-55)rng(121-233)rng(121-122)Examples/Algorithms/Generators/ActsExamples/Generators/EventGenerator.hpp (3)
rng(61-61)rng(72-72)rng(85-85)Examples/Algorithms/GeneratorsPythia8/ActsExamples/Generators/Pythia8ProcessGenerator.hpp (1)
rng(69-69)Examples/Algorithms/Generators/ActsExamples/Generators/MultiplicityGenerators.hpp (2)
rng(34-36)rng(34-34)
Core/include/Acts/Utilities/ScopedTimer.hpp (1)
Core/src/Utilities/ScopedTimer.cpp (7)
ScopedTimer(16-20)ScopedTimer(22-31)Sample(44-45)Sample(47-51)Sample(57-65)AveragingScopedTimer(33-36)AveragingScopedTimer(67-81)
Examples/Io/HepMC3/include/ActsExamples/Io/HepMC3/HepMC3OutputConverter.hpp (2)
Examples/Algorithms/Generators/ActsExamples/Generators/EventGenerator.hpp (3)
m_cfg(139-139)ctx(136-136)ctx(144-145)Examples/Io/HepMC3/include/ActsExamples/Io/HepMC3/HepMC3Writer.hpp (2)
m_cfg(61-61)ctx(55-56)
Examples/Io/HepMC3/src/HepMC3Particle.cpp (1)
Examples/Io/HepMC3/include/ActsExamples/Io/HepMC3/HepMC3Particle.hpp (14)
barcode(23-23)particle(28-28)id(33-33)productionVertex(38-39)endVertex(44-44)pdgID(49-49)pdgID(74-74)momentum(54-54)momentum(79-79)energy(59-59)energy(84-84)mass(64-64)mass(89-89)charge(69-69)
Examples/Io/HepMC3/include/ActsExamples/Io/HepMC3/HepMC3Particle.hpp (1)
Examples/Io/HepMC3/src/HepMC3Particle.cpp (10)
particle(22-31)particle(22-22)energy(69-71)energy(69-69)energy(92-97)energy(92-93)mass(73-75)mass(73-73)mass(99-101)mass(99-99)
🪛 Ruff (0.8.2)
Examples/Python/tests/test_hepmc3.py
113-113: acts.examples.hepmc3.HepMC3OutputConverter imported but unused
Remove unused import: acts.examples.hepmc3.HepMC3OutputConverter
(F401)
204-204: acts.examples.hepmc3.HepMC3OutputConverter imported but unused
Remove unused import: acts.examples.hepmc3.HepMC3OutputConverter
(F401)
256-256: pyhepmc.view.to_dot imported but unused; consider using importlib.util.find_spec to test for availability
(F401)
261-261: Loop control variable evt not used within loop body
Rename unused evt to _evt
(B007)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: CI Bridge / clang_tidy
- GitHub Check: CI Bridge / build_gnn_tensorrt
🔇 Additional comments (110)
pytest.ini (1)
13-13: A new marker, added you have.
Useful for grouping tests, it is. Concerns, I have none.Examples/Algorithms/Geant4HepMC/CMakeLists.txt (1)
21-21: Adopting target-based linking, wisely done it is.
Yes, modern practice this approach be.CMakeLists.txt (1)
516-530: Graceful handling of HepMC3 versions, I see.
Backward compatibility ensured, yes.Core/include/Acts/Utilities/ScopedTimer.hpp (2)
35-61: ScopedTimer changes, implement well you do.
High-resolution clock usage, wise it is. Constructor and destructor separation, neat indeed. Deletion of copy and move, avoids confusion does it.
63-133: AveragingScopedTimer class, introduced splendidly you have.
Averaging mechanism, robust it looks. RAII sample design, quite efficient. Logging of statistics, thorough.Examples/Io/HepMC3/include/ActsExamples/Io/HepMC3/HepMC3Writer.hpp (5)
11-11: Powerful addition, your import ofProcessCodeis.
Using the required header you are, to handle process results elegantly. Fortunate we are to see forward, hmm.
18-20: Forward declarations, arranged well they are.
Maintain them properly to reduce compile times, indeed.
26-27: Wise to inherit fromWriterT<std::shared_ptr<HepMC3::GenEvent>>, you have.
Streamlined handling of single events, this approach fosters. Delightful is the final class usage.
52-56: Signature rewriting, from a vector to a single event, has improved clarity.
Ensure usage across the pipeline, you do. Not break, your code must.
66-67:✅ Verification successful
Shared configuration and a mutex you keep.
Thread-safety, a vital precaution is. Acquire locks properly inwriteT()orfinalize(), you must.
🏁 Script executed:
#!/bin/bash # Searching for lock usage to confirm synchronization rg -A 5 "lock_guard<std::mutex>"Length of output: 17749
Synchronization Verified, It Is!
Confirmed, our search has. In
HepMC3Writer.cpp, the use ofstd::lock_guard<std::mutex> lock(m_mutex);is found to secure shared resources, as it should be. In your header at lines 66–67, the mutex is declared and then properly guarded in functions such asfinalize(). Thread-safety maintained is, and further vigilance with additional shared resources, you must have.Core/src/Utilities/CMakeLists.txt (1)
13-13: A newScopedTimer.cppin the build, good measure it ensures.
Profile code performance further, this allows. Verify though all necessary includes are set, and no separate timings needed, you must.Examples/Io/HepMC3/CMakeLists.txt (1)
9-9:✅ Verification successful
A new
HepMC3OutputConverter.cppyou add.
Strong extension of functionality, yes. Validate final linking with the library, you should.
🏁 Script executed:
#!/bin/bash # Searching references to ensure the converter is properly linked and used rg "HepMC3OutputConverter"Length of output: 1413
Linking verified, the converter is.
The new file
HepMC3OutputConverter.cppis properly referenced in the codebase—from its inclusion in the CMakeLists.txt to its implementation and test usage. Final linking appears sound; do ensure that library linking during build and runtime is validated as a final check.Examples/Framework/src/EventData/SimParticle.cpp (1)
14-14: Simplified the particle ID output format, hmm.Changed, the output format has been. Removed, the raw value from the particle ID output is. More concise representation, this creates.
Examples/Algorithms/Generators/CMakeLists.txt (1)
13-13: Direct dependency on HepMC3, now added is.Directly linked now, HepMC3 library is. More explicit and direct approach, this is. With the PR objectives' refactoring goal, aligns this does. Changed from conditional inclusion to direct dependency, the integration has.
Examples/Algorithms/GeneratorsPythia8/CMakeLists.txt (1)
17-17: Modern CMake practice, this follows.From variable-based linking
${HEPMC3_LIBRARIES}to target-based linkingHepMC3::HepMC3, changed it has. Better practice, this is. More explicit dependency specification provides it does. With modern CMake conventions, aligns it does.Examples/Python/python/acts/examples/simulation.py (2)
237-238: New parameters for event output control, added they are.New optional parameters, introduced they have been:
outputEvent: For specifying output event stringprintListing: For controlling event listing displayEnhanced configurability, these provide. Backward compatible, they remain. With direct HepMC3 types exchange mentioned in PR title, consistent this is.
337-338: Parameters to EventGenerator properly passed, they are.Correctly forwarded, the new parameters are. To EventGenerator constructor, they go. Consistent implementation, this is.
Fatras/include/ActsFatras/EventData/Barcode.hpp (1)
163-168: Useful addition for debugging and logging, hmm, yes!A friend function that overloads the output stream operator, added you have. Clear formatting, it provides. For debugging and logging purposes, most valuable this will be.
Examples/Algorithms/GeneratorsPythia8/ActsExamples/Generators/Pythia8ProcessGenerator.hpp (1)
69-69: Changed the return type to use HepMC3 directly, you have!From
std::pair<SimVertexContainer, SimParticleContainer>tostd::shared_ptr<HepMC3::GenEvent>, the return type now changes. Consistent with changes in other generator components, this is. Aligns with the refactoring goal to use HepMC3 types directly, it does. The Force of standardization, strong it is!Examples/Python/src/Generators.cpp (1)
76-81: More concise and maintainable, your code now is!Replaced multiple
def_readwritecalls with theACTS_PYTHON_STRUCTmacro, you have. Added new configuration options you did -outputEvent,printListing,checkConsistency, and more. Streamlined the Python bindings, this change has. Hmm, easier to maintain this will be!Examples/Python/src/HepMC3.cpp (4)
11-11: New include for HepMC3OutputConverter, added you have!For the new converter functionality, necessary this include is. To the path of complete HepMC3 integration, one step closer we move.
19-19: Filesystem support for Python bindings, added you have!For handling file paths in Python, this include required is.
38-39: Updated parameters for HepMC3AsciiWriter, I see!Changed from
outputDirandoutputStemtooutputPathandperEvent. More direct file path handling, this indicates. Clearer intent, it has. Yes, hmmm.
45-47: New algorithm for HepMC3OutputConverter, declared you have!Convert between particles, vertices, and HepMC3 events, this algorithm will. For the refactoring to HepMC3 types, essential this is. Good addition, yes!
Examples/Algorithms/HepMC/src/HepMCProcessExtractor.cpp (1)
226-226:❓ Verification inconclusive
Particles_in, empty not, ensure.
vertex->particles_in()[0]we ask for. If none found, break this code would. Confirm guaranteed not empty, you must.
🏁 Script executed:
#!/bin/bash # Checking for vertex->particles_in() usage to confirm no empty container is assumed. rg -A 5 "particles_in("Length of output: 102
Attention, young padawan:
The original regex stumbled, it did, due to unescaped parenthesis. Though the intent was clear—ensure thatvertex->particles_in()[0]is not called upon an empty container—I find that manual re-verification is needed. Confirm, you must, that the container is guaranteed non-empty before accessing element zero. Re-run the above script and review the code logic carefully, you should.
🏁 Script executed:
#!/bin/bash # Re-check for vertex->particles_in() usage in the target file with proper escaping rg -A 5 "particles_in\(" Examples/Algorithms/HepMC/src/HepMCProcessExtractor.cppLength of output: 1106
Assure non-empty particle container, you must!
While the loop verifies thatcurrentVertex->particles_in()is non-empty before its use, the final call usingvertex->particles_in()[0]is made without an explicit check in sight. Confirm, you must, that at this point the variablevertexis guaranteed to have a non-empty container. Consider adding an inline comment (or guard) if its non-emptiness is ensured by prior logic, for clarity and safety, it would be.Examples/Algorithms/Generators/ActsExamples/Generators/ParametricParticleGenerator.hpp (1)
73-73: Refactor to HepMC3 GenEvent, good improvement, yes.
Harmonize with the new event structure, it does. Hmmm, approve this, I do.Examples/Io/HepMC3/src/HepMC3OutputConverter.cpp (3)
25-35: Validated config, wise and thorough, it is.
Missing inputs and outputs do you guard. Approved, yes.
76-86:❓ Verification inconclusive
Dummy incoming particle, an interesting solution, hmmm.
Status 666 and mass 123, a placeholder quite large. Confirm physically correct or desired, we must. If caution you have, we can modify or explain.
🏁 Script executed:
#!/bin/bash # Search for references to 'dummyParticle' creation to confirm usage and necessity. rg -A 5 "dummyParticle"Length of output: 940
Hmm, verify these placeholder values you must!
In fileExamples/Io/HepMC3/src/HepMC3OutputConverter.cpp(lines 76–86), a dummy incoming particle is created when none exist—an interesting solution, it is. The status code 666 and generated mass 123, magic numbers they appear. Confirm if these are physically correct or intentionally placeholders, you should. If intentional, consider adding an inline explanation so future developers understand the wisdom behind these constants, hmmm.
51-65:❓ Verification inconclusive
Barcodes mapped, strong the approach is.
Unique barcodes, assured be they must. Verify collisions do not happen, yes?
🏁 Script executed:
#!/bin/bash # Scanning for any duplicates in the inputParticles calls that might lead to barcode collisions. rg -A 5 "insert({"Length of output: 111
Duplicate barcode check, verify you must.
The regex error, caused by missing escape characters for curly braces, confounds the duplicate detection. Re-run the command with proper escaping, you should. For example, try:This revised query will help you verify that duplicate insertions of particle barcodes do not occur, ensuring unique mappings remain. Manual confirmation of the absence of collisions, please perform.
🏁 Script executed:
#!/bin/bash rg -n "barcodeMap.insert\(\{" Examples/Io/HepMC3/src/HepMC3OutputConverter.cppLength of output: 271
Barcode uniqueness, verify you must!
Strong, your mapping approach is. One insertion point we see, yet if duplicate particle IDs arise in inputParticles, unexpected behavior may follow. Confirm, you must, that inputParticles contains only unique IDs—or else add defensive checks to handle collisions. Manual verification of inputParticles uniqueness, consider you should.Examples/Io/HepMC3/include/ActsExamples/Io/HepMC3/HepMC3Event.hpp (4)
69-69: Strong the reference-based approach is, yes.Passing the particle by reference, simpler memory management it brings. Overhead from shared ownership, reduced it is. Good, hmm!
75-75: Consistent the vertex parameter is, hmmm.A reference-based method for addVertex, harmonious with the new design it becomes.
83-83: Remove a particle by reference, wise design you have made.Better for direct usage and memory clarity, certain it is.
90-90: Removing a vertex with references, improved the interface is.Potential confusion from shared pointers, vanish it does.
Examples/Algorithms/GeneratorsPythia8/ActsExamples/Generators/Pythia8ProcessGenerator.cpp (7)
23-23: Include ordering changed, yes.Bringing in Pythia8/Pythia first, aligned with usage it is.
70-70: Initialization log, helpful is, hmmm.A quick trace of generator init, this gives. Good for debugging, yes.
98-98: Converter creation, correct and straightforward.Use of
std::make_unique, safe memory management ensures, hmmm.
121-122: Signature revised to return HepMC3::GenEvent, extraordinary step forward it is.Integration of event data, more direct this becomes. Check thoroughly the caller side, you must.
143-144: Units set to GEV and MM, consistent with typical usage they are.Straightforward this is. Good practice, yes.
146-148: Assertion on m_impl->m_hepMC3Converter, safe path ensures, hmmm.Fewer null pointer surprises, you shall have.
232-232: Returning the GenEvent, integration complete it becomes.Usable for further steps or logging, very helpful it is.
Examples/Io/HepMC3/include/ActsExamples/Io/HepMC3/HepMC3OutputConverter.hpp (1)
1-49: New converter introduced, promising this design is.Inputs and outputs well-defined, flexible for event data. Thorough testing ensure you must, verify you will.
Examples/Io/HepMC3/src/HepMC3Writer.cpp (3)
13-14: New include for WriterAscii, hmm.
Valuable addition it is, for handling event writing in ASCII format.
38-69: Thread-safe writing approach, commendable it is.
Lock guard usage, correct. Single-file writer, well managed. Implementation looks carefully done.
71-77: Finalize method, well structured it is!
Close the writer you do, ensuring resource cleanup. Good practice, it remains.Examples/Algorithms/Generators/ActsExamples/Generators/ParametricParticleGenerator.cpp (8)
18-18: Smart pointer header included, mm.
Memory management simpler become, with<memory>.
21-28: HepMC3 headers, add you have.
Integrate deeply with GenEvent structures, yes. All lined up, it is.
92-93: Return a shared GenEvent, you do.
Clear ownership model, this provides. Efficient indeed.
94-99: Vertex creation logic, see I do.
Set position to zero, good. Ties well into the event structure.
100-109: Beam particle, cleverly placed.
Momentum is zero, mass is zero, set to status 4. All correct for a beam placeholder.
131-140: Particle momentum and mass, carefully set they are.
Correct usage of GeV conversion, quite consistent is.
141-149: Particles, add you do to the event.
Attributes appended for each particle, thoroughly done. Good approach, yes.
151-151: Event returned, final step.
Fully consistent with the signature, it is.Core/src/Utilities/ScopedTimer.cpp (9)
16-20: Constructor of ScopedTimer, well structured it is.
Initialize the start time you do, correct this is. Thread safety with shared usage consider if needed, you must.
22-31: Destructor logging, helpful it is.
Time measurement you finalize, then recorded it becomes. Good for performance insights, yes.
33-36: AveragingScopedTimer constructor, simple and clear it is.
Initialize parent members, correct you do. No issues found, have I.
38-42: Accumulation of samples, well done it is.
Keep track of sums and squares, a robust approach it shows.
44-46: Sample constructor, straightforward it appears.
Start time captured, link to parent stored. Good design, yes.
47-51: Move constructor, moved-from sample disabling you do.
Ensures double accumulation avoided, wise it is.
53-55: Factory method for sample, clarity it provides.
Encapsulate the creation you do, maintainable this is.
57-65: Sample destructor, adding measured duration it does.
When parent valid, aggregated the data becomes. Clean design, yes.
67-81: AveragingScopedTimer destructor, final statistics recorded it does.
Mean and standard deviation calculated, good for analysis it is.Examples/Algorithms/Generators/ActsExamples/Generators/EventGenerator.cpp (9)
11-14: New includes for definitions, correct are they.
Needed for extended functionalities, yes. Merged, can be.
20-20: Include , a standard usage it is.
Sorting or other ops you may require, wise to add.
26-29: HepMC3 headers included, good alignment with library usage.
Ensure version compatibility, you must.
43-43: Using namespace for unit literals inside .cpp, clarity it provides.
Easy reading, conflict rarely arises. Approve, I do.
58-69: Validate sub-generators presence, you do.
Runtime errors prevented, robust design it is.
210-218: Final event listing, good for debugging it is.
Verbosity a user can control, well done.
220-231: Reading completes, success code returned.
Fitting end to the method, no issues found have I.
233-365: Handling of vertices and secondaries, robust indeed.
Distance checks done, merges well defined. Approve, I do.
367-611: Conversion to internal EDM, thorough it is.
Consistency checks optionally done, a well-designed approach.Examples/Algorithms/Generators/ActsExamples/Generators/EventGenerator.hpp (8)
26-29: Forward declarations of HepMC3 classes, wise it is.
Reduce compilation dependencies, yes.
101-102: Output event name optional, flexible approach this is.
Use defaults or custom value, user can.
103-103: String "hepmc3_event" as default, a good choice you made.
Clarifies purpose, it does.
109-112: Boolean printListing introduced, large debugging output it yields.
Useful for development, enable only if needed you should.
113-125: Vertex threshold merges introduced, advanced control this offers.
Adjust these for performance or physics detail, trial and error you may do.
144-146: Function to convert HepMC3 events, an extended feature it is.
Implementation in .cpp consistent, see I do.
147-151: New handleVertex method, modular approach you adopt.
Helps clarity, reusability it does.
160-161: WriteDataHandle for HepMC3 event, flexible integration you add.
Shrinking data or storing direct, feasible it becomes.Examples/Io/HepMC3/src/HepMC3Event.cpp (11)
22-31: Wise changes to actsParticleToGen function, I see!References instead of shared pointers you now use. More efficient, this is. Cleaner access to fourMomentum and pdg() with direct notation, hmm.
38-47: Simplified parameter passing, you have!From shared pointer to direct reference, the transformation is complete. Position access more straightforward becomes, yes. Good practice this is, when ownership transfer not needed.
59-83: Refactored comparison function, you did!The Force guides your code to direct object access. No longer through pointer indirection do you travel. All comparisons remain functionally equivalent, hmm, yet cleaner they become.
158-162: Function signature harmonized with the others, it is!Parameter type changed to reference for addParticle. Consistent with overall code style and philosophy this remains. Implementation details unaltered, yet cleaner the interface becomes.
164-167: The way of references, you follow consistently!The addVertex function takes reference now. Harmonized with other functions, your code is. Less overhead, more clarity, hmm, yes.
173-185: Permission to remove particles granted, with improved syntax!Direct reference instead of shared pointer, you use. Way to access particleId(), cleaner it has become. Search logic remains untouched, good this is!
187-198: For vertex removal, references you also chose!Consistent changes throughout codebase, I sense. Simpler parameter type, yet same functionality preserved.
239-240: Direct dereference in place of shared_ptr, wise choice!For particle translation, dereferencing now used instead of shared_ptr creation. Less memory overhead this creates, more efficient your code becomes.
252-253: Translation of vertices simplified too, I see!Direct object access via dereference you now use. Consistent with your pattern of fewer pointers, more direct access.
263-264: For beam translation, same pattern applied!Dereferencing instead of shared_ptr creation. Consistent refactoring, hmm, yes.
277-278: Final state collection also simplified!Direct object passing with dereference, you now employ. Reduced indirection throughout the code, I approve.
Examples/Io/HepMC3/include/ActsExamples/Io/HepMC3/HepMC3Vertex.hpp (4)
23-24: HepMC3::GenVertex reference instead of shared_ptr, a wise declaration!Simplified interface you have created. Return unique_ptr still you do, but input parameter more direct it becomes. Good practice this is.
29-30: All getter functions harmonized with reference parameters, hmmmm!References instead of shared_ptr for inEvent, id, particlesIn, particlesOut, position, and time functions. Consistent change throughout API. Clear intent and less overhead, this brings.
Also applies to: 34-35, 39-40, 44-45, 49-50, 54-55
59-60: Setter functions refactored similarly, I see!For addParticleIn, addParticleOut, removeParticleIn, removeParticleOut - all now take references instead of shared_ptr. Mutable reference for vertex parameter, const reference for particle. Ownership semantics clearer this makes.
Also applies to: 64-65, 69-70, 74-75
79-80: Position and time setters refactored too, hmm yes!Follows same pattern - reference instead of shared_ptr for the vertex. Modify in place you can, without pointer indirection.
Also applies to: 84-85
Examples/Io/HepMC3/src/HepMC3Particle.cpp (6)
17-20: Direct reference to particle for barcode extraction, good is!Simplified parameter from shared_ptr to reference. Access to id() more direct becomes. Code intent clearer now appears.
22-31: Particle creation function improved, it has!Reference instead of shared_ptr for input parameter. Member access changes from arrow (->) to dot (.) notation. More readable and direct, your code becomes.
33-35: Simple id accessor also refactored, I observe!Consistent pattern of reference over shared_ptr continues. Direct dot access instead of arrow, cleaner this is.
37-45: Production vertex access simplified too, hmmm!More direct access to vertices through references. Passed to HepMC3Vertex::processVertex through dereference when vertex exists. Pattern of reference over pointer consistently applied.
47-55: End vertex accessor follows same pattern, good!Reference instead of shared_ptr for endVertex parameter. Same conditional check maintained, with cleaner access through dot notation.
57-101: All accessors and mutators refactored consistently, see I do!For pdgID, momentum, energy, mass, charge - getters and setters - all now use references instead of shared pointers. Direct object access with dot notation replaces arrow operators. Harmonized approach throughout the codebase, pleasant this is!
Examples/Io/HepMC3/src/HepMC3Vertex.cpp (7)
22-30: Type updated from GenParticlePtr to ConstGenParticlePtr, observant you are!More precise const-correctness this brings. Barcode extraction through dereferenced pointer now occurs.
35-43: Same pattern for particle translation function applied!ConstGenParticlePtr instead of GenParticlePtr as input type. Dereferencing used when calling particle function. Consistent with other changes, this is.
49-59: Direct SimParticle reference in actsParticleToGen, good refactor!Access to fourMomentum and pdg now through direct dot notation. No more dereferencing of shared pointers needed. Cleaner, this code becomes.
69-79: Particle matcher function also refactored, I see!Reference instead of shared_ptr for particle parameter. Direct access to particleId through dot notation. Search logic remains unchanged.
83-92: Process vertex function improved significantly!Reference instead of shared_ptr for vertex parameter. All member access now through direct dot notation. Position, particles_in, particles_out - all cleanly accessed. More straightforward implementation this creates.
94-122: All accessor functions harmonized with reference parameters!For inEvent, id, particlesIn, particlesOut, position and time - reference replaces shared_ptr. Direct member access with dot notation instead of arrow. Consistent pattern throughout codebase maintained.
124-162: All mutator functions also refactored consistently!For addParticleIn, addParticleOut, removeParticleIn, removeParticleOut, position and time setters - all now use mutable reference for vertex and const reference for other parameters. Cleaner access pattern and API consistency this creates. Most pleased, I am!
Examples/Io/HepMC3/include/ActsExamples/Io/HepMC3/HepMC3Particle.hpp (4)
23-23: Approve these changes, I do. From shared pointers to references, a wise move this is.Simplified the interface has become. Better performance and clearer ownership semantics, we now have. References over shared pointers, the modern C++ way this is. Hmm, yes.
Also applies to: 28-28, 33-33
38-39: Improved vertex handling, this refactoring has.For vertex functions, direct references now you use. No more unnecessary indirection through pointers. Cleaner code, better performance, hmm, yes.
Also applies to: 44-44
49-49: Particle property getters, improved they are.For pdgID, momentum, energy, mass, and charge - consistent approach you have taken. References instead of shared pointers, a path to cleaner code this is. Memory management burden reduced, hmm. Performance improved, it has.
Also applies to: 54-54, 59-59, 64-64, 69-69
74-74: For setter functions, non-const references you now use. Wise, this is.Mutable references for setter functions, the correct approach this is. The const-correctness, respected it has been. Modern C++ practices, follow you do. The Force of clean code, strong with these changes it is.
Also applies to: 79-79, 84-84, 89-89
Examples/Algorithms/Generators/ActsExamples/Generators/EventGenerator.cpp
Show resolved
Hide resolved
paulgessinger
force-pushed
the
refactor/evgen-hepmc3
branch
from
5b60ab7 to
04ec670
Compare
Examples/Algorithms/Generators/ActsExamples/Generators/EventGenerator.cpp
Show resolved
Hide resolved
Examples/Algorithms/Generators/ActsExamples/Generators/EventGenerator.cpp
Outdated
Show resolved
Hide resolved
Examples/Algorithms/Generators/ActsExamples/Generators/EventGenerator.cpp
Show resolved
Hide resolved
Examples/Algorithms/Generators/ActsExamples/Generators/EventGenerator.cpp
Outdated
Show resolved
Hide resolved
Examples/Algorithms/Generators/ActsExamples/Generators/ParametricParticleGenerator.cpp
Outdated
Show resolved
Hide resolved
Examples/Algorithms/GeneratorsPythia8/ActsExamples/Generators/Pythia8ProcessGenerator.cpp
Outdated
Show resolved
Hide resolved
Examples/Algorithms/Generators/ActsExamples/Generators/EventGenerator.cpp
Show resolved
Hide resolved
Examples/Algorithms/Generators/ActsExamples/Generators/EventGenerator.cpp
Outdated
Show resolved
Hide resolved
paulgessinger
force-pushed
the
refactor/evgen-hepmc3
branch
3 times, most recently
from
a7f97b6 to
f1dd64b
Compare
github-actions
bot
added
Infrastructure
paulgessinger
force-pushed
the
refactor/evgen-hepmc3
branch
2 times, most recently
from
5785bb6 to
6f7a99d
Compare
paulgessinger
force-pushed
the
refactor/evgen-hepmc3
branch
from
7f60ec3 to
a65c342
Compare
Blocked by:
--- END COMMIT MESSAGE ---
Any further description goes here, @-mentions are ok here!
feat,fix,refactor,docs,choreandbuildtypes.Summary by CodeRabbit