Skip to content

Geant4Output2EDM4hep: get run from worker context - #1663

Open
andresailer wants to merge 18 commits into
AIDASoft:masterfrom
andresailer:run2multi
Open

Geant4Output2EDM4hep: get run from worker context#1663
andresailer wants to merge 18 commits into
AIDASoft:masterfrom
andresailer:run2multi

Conversation

@andresailer

@andresailer andresailer commented Aug 21, 2026

Copy link
Copy Markdown
Member

This will need some more work, the current status writes one run per thread.

BEGINRELEASENOTES

  • Geant4OutputAction: In multithreaded mode, make sure the ingested runparameters and fileparameters end up in the output, fixes OutputAction and contexts #1660
    • Fixed locking endRun action of the outputaction
  • Tests: added comparison of run and metadata trees if they exist in the output

ENDRELEASENOTES

TODO

  • Output2ROOT
  • Output2LCIO: writes one run per thread
  • Common refactor?

@github-actions

github-actions Bot commented Aug 21, 2026

Copy link
Copy Markdown

Test Results

  9 files    9 suites   11h 44m 19s ⏱️
 49 tests  34 ✅ 0 💤 15 ❌
410 runs  387 ✅ 0 💤 23 ❌

For more details on these failures, see this check.

Results for commit 1eb7ad4.

♻️ This comment has been updated with latest results.

@tmadlener

Copy link
Copy Markdown
Contributor

the current status writes one run per thread.

I'm assuming a "trivial" solution employing std::call_once doesn't do the trick? Might be a bit trickier if there are multiple runs as well as multiple threads...

@andresailer
andresailer marked this pull request as ready for review August 26, 2026 11:31
@andresailer
andresailer force-pushed the run2multi branch 2 times, most recently from 86c6456 to 2c073a2 Compare August 26, 2026 11:33
@andresailer
andresailer requested a review from tmadlener August 26, 2026 11:33

@tmadlener tmadlener left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks deceptively simple in the end, but I think following your comments / explanations it should be correct.

Somewhat related: Can we handle multiple runs in flight in a multi-threaded context correctly in DD4hep/ddsim? I think at least from the podio side things should keep working, as long as all runs are added to a worker context at some point.

@andresailer

Copy link
Copy Markdown
Member Author

Somewhat related: Can we handle multiple runs in flight in a multi-threaded context correctly in DD4hep/ddsim? I think at least from the podio side things should keep working, as long as all runs are added to a worker context at some point.

Unless I already forgot again: run information is read when the file is opened. There is no sequential processing of files. Only one run per file, so no treatment of multiple runs in parallel.

Comment thread DDG4/edm4hep/Geant4Output2EDM4hep.cpp Outdated
// only one of the worker contexts (or the master context) has run parameters, since only one reader is created
// which registers the run parameters
parameters->extractParameters(runHeader);
m_file->writeFrame(runHeader, podio::Category::Run);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have to check. Are there always runParameters, or are there cases where we don't have runParameters, but we still want to write the runheader.

only write run frame when we have run parameters
@andresailer

Copy link
Copy Markdown
Member Author

deceptively

Indeed

Comment thread DDG4/edm4hep/Geant4Output2EDM4hep.cpp Outdated
Comment thread DDG4/include/DDG4/Geant4Action.h Outdated
Comment thread DDG4/edm4hep/Geant4Output2EDM4hep.cpp Outdated
@andresailer

Copy link
Copy Markdown
Member Author

EventReader test fails only in 107a
106: no EDM4hepFileReader, podio is too old
107a: issue with

16: Warning in <TClass::Init>: no dictionary for class podio::root_utils::CollectionWriteInfo is available

(podio 1.2.0)

109: this warning is not present (podio 1.7.0)

Still weird why it started failing so, need to investigate.

@andresailer

Copy link
Copy Markdown
Member Author

Still weird why it started failing so, need to investigate.

Because we just turned on EDM4hep for the CI

@tmadlener

Copy link
Copy Markdown
Contributor

What is the file version with which the input file has been written? (podio-dump on the input file should tell you). Since podio doesn't make forward compatibility guarantees, it's possible that the input file is too new for certain stacks.

@andresailer

Copy link
Copy Markdown
Member Author

possible that the input file is too new for certain stacks.

106

podio-dump crashes

107a

TClass::Init:0: RuntimeWarning: no dictionary for class podio::root_utils::CollectionWriteInfo is available
input file: ../DDTest/inputFiles/ZH250_ISR.edm4hep.root
            (written with podio version: 1.6.0)

datamodel model definitions stored in this file: 
 - edm4hep (0.99.99)

Frame categories in this file:
Name    Entries
------  ---------
ERROR: Cannot print category 'events' (not present in file)
$ rootls ../DDTest/inputFiles/ZH250_ISR.edm4hep.root
events  podio_metadata  runs

109a

podio-dump DDTest/inputFiles/ZH250_ISR.edm4hep.root 
input file: DDTest/inputFiles/ZH250_ISR.edm4hep.root
            (written with podio version: 1.6.0)

datamodel model definitions stored in this file:
 - edm4hep (0.99.99)

Frame categories in this file:
Name    Entries  
------  -------  
runs    1        
events  3        
################################### events: 0 ####################################
Collections:
Name (s = subset collection)  ValueType                          Size  ID          
----------------------------  ---------------------------------  ----  --------    
EventHeader                   edm4hep::EventHeader               1     d793ab91    
GeneratorEventParameters      edm4hep::GeneratorEventParameters  1     a193a48b    
MCParticles                   edm4hep::MCParticle                125   a1cba250    

Parameters:
Name  Type  Elements  
----  ----  --------  

@andresailer

Copy link
Copy Markdown
Member Author

I am going to disable the edm4hep filereader test for podio < 1.6

Comment thread DDTest/python/compare_simulation_output.py Outdated
@andresailer
andresailer force-pushed the run2multi branch 4 times, most recently from 8b461b0 to bc2c601 Compare August 28, 2026 13:37
Comment thread DDTest/python/compare_simulation_output.py
Comment thread DDG4/plugins/Geant4EventSeed.cpp
Comment thread DDG4/lcio/Geant4Output2LCIO.cpp Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OutputAction and contexts

2 participants