-
Notifications
You must be signed in to change notification settings - Fork 100
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
Read edm4hep #1371
base: master
Are you sure you want to change the base?
Read edm4hep #1371
Conversation
9dd2fbe
to
0969e1f
Compare
Test Results 14 files 14 suites 5h 37m 26s ⏱️ For more details on these failures, see this check. Results for commit 60603ac. ♻️ This comment has been updated with latest results. |
db8bfbe
to
fa9431d
Compare
That GeneratorInfo is not yet written to EDM4hep files? |
c7a9ec0
to
818d1f6
Compare
Not consistently, i.e. if at all currently only with files that are produced via |
DDG4/edm4hep/EDM4hepFileReader.cpp
Outdated
class EDM4hepFileReader : public Geant4EventReader { | ||
protected: | ||
/// Reference to reader object | ||
podio::ROOTReader m_reader {}; |
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.
podio::ROOTReader m_reader {}; | |
podio::Reader m_reader; |
Would immediately enable support for more backends (most importantly RNTuple, to a lesser extent SIO), without having to do that later.
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.
What is the file extension convention for sio files?
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.
"sio"
(as per this check)
m_directAccess = true; | ||
} | ||
|
||
void EDM4hepFileReader::registerRunParameters() { |
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.
This will inadvertently mix "file level" metadata and run parameters for the output, right?
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.
Indeed it would do that.
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.
Is there a way to keep them separated? E.g. by attaching the metadata frame to some entity as an extension and then writing it out again as a whole in the output if it is present?
3bd7b4b
to
3beea52
Compare
3beea52
to
60603ac
Compare
BEGINRELEASENOTES
ENDRELEASENOTES
[x] Also ingest GeneratorInfo(to be done later Add GeneratorInfo to EDM4hepReader #1373)