Skip to content
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

Add a script to create an EDM4hep file making use of the complete datamodel #272

Merged
merged 23 commits into from
Jul 28, 2024

Conversation

jmcarcell
Copy link
Contributor

BEGINRELEASENOTES

  • Add a script to create an EDM4hep file. The script is in python and can be called like python createEDM4hepFile.py. I have tried to make use of every member and relation in the data model. The purpose is to have something that can create quickly an EDM4hep file and because the script is in python no compilation is needed, hopefully lowering the barrier to get to an EDM4hep file. People can adapt it to their needs or use it as an example for testing, debugging, etc.

ENDRELEASENOTES
For the question, "should this be a test?" I don't have a strong opinion but I think it would have less visibility than in /scripts

@peremato
Copy link

Great! well done

For the question, "should this be a test?" I don't have a strong opinion but I think it would have less visibility than in /scripts
A test can be created that uses the script in /scripts

Two questions (perhaps are trivial)

  • how do you choose the ROOT format (TTree or RNTuple)?
  • Why do you need cppyy.ll.static_cast?

@jmcarcell
Copy link
Contributor Author

Great! well done

For the question, "should this be a test?" I don't have a strong opinion but I think it would have less visibility than in /scripts
A test can be created that uses the script in /scripts

Two questions (perhaps are trivial)

* how do you choose the ROOT format (TTree or RNTuple)?

* Why do you need `cppyy.ll.static_cast`?

I added an option --rntuple to save to RNTuples. For the second question it seems it's a problem in cppyy maybe, see: #270 (the conversion operator is not being used from the mutable to the non-mutable types)

@jmcarcell jmcarcell changed the title Add a script to create an EDM4hep file Add a script to create an EDM4hep file making use of the complete datamodel Jun 11, 2024
Copy link
Contributor

@tmadlener tmadlener left a comment

Choose a reason for hiding this comment

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

We should add a run of this script to CI, to make sure it always works.

@@ -6,12 +6,16 @@ function(set_test_env _testname)
set_property(TEST ${_testname} APPEND PROPERTY ENVIRONMENT
LD_LIBRARY_PATH=$<TARGET_FILE_DIR:edm4hep>:$<TARGET_FILE_DIR:podio::podio>:$<$<TARGET_EXISTS:edm4hepRDF>:$<TARGET_FILE_DIR:edm4hepRDF>:>$<TARGET_FILE_DIR:ROOT::Core>:$ENV{LD_LIBRARY_PATH}
ROOT_INCLUDE_PATH=${PROJECT_SOURCE_DIR}/edm4hep:${PROJECT_SOURCE_DIR}/utils/include:$ENV{ROOT_INCLUDE_PATH}
PYTHONPATH=${PROJECT_SOURCE_DIR}/python:$ENV{PYTHONPATH}
Copy link
Contributor

Choose a reason for hiding this comment

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

We either need to add the podio that we have used for building here as well to PYTHONPATH, or we have to do it in the workflow file in the step where we build and install podio. CI is partially failing, because it picks up an older version of podio from the LCG stack:

https://github.com/key4hep/EDM4hep/actions/runs/10045234420/job/27762066625?pr=272#step:4:817

@jmcarcell jmcarcell merged commit d409c53 into main Jul 28, 2024
12 of 17 checks passed
@jmcarcell jmcarcell deleted the script branch July 28, 2024 17:18
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.

3 participants