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

Edm4hep format as default when both edm4hep and lcio enabled #1329

Merged
merged 1 commit into from
Oct 15, 2024

Conversation

BrieucF
Copy link
Contributor

@BrieucF BrieucF commented Oct 11, 2024

BEGINRELEASENOTES

  • ddsim: Set edm4hep the default output format in case DD4hep is compiled with both DDD4HEP_USE_LCIO DD4HEP_USE_EDM4HEP ON
  • ddsim: Change the name of the default output file from dummyOutput to ddsimOutput

ENDRELEASENOTES

Copy link
Member

@andresailer andresailer left a comment

Choose a reason for hiding this comment

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

Fine for me. Just want someone from ILD to OK as well.

@andresailer andresailer linked an issue Oct 11, 2024 that may be closed by this pull request
@tmadlener
Copy link
Contributor

Will this actually yield EDM4hep files and not the DD4hep format without the edm4hep.root suffix?

@gaede anything speaking against this? I think all of the worklows, tutorials, etc. get an LCIO output in any case, because we specify an output file name with .slcio in any case.

@BrieucF
Copy link
Contributor Author

BrieucF commented Oct 11, 2024

Will this actually yield EDM4hep files and not the DD4hep format without the edm4hep.root suffix?

@gaede anything speaking against this? I think all of the worklows, tutorials, etc. get an LCIO output in any case, because we specify an output file name with .slcio in any case.

This PR does not change the behavior in case both the lcio and edm4hep flags are OFF (the dd4hep format is the default for rootfiles). If edm4hep is ON, rootfiles will be edm4hep (without the need of edm4hep.root in the name), but this was introduced in #1144.

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.

If edm4hep is ON, rootfiles will be edm4hep

Thanks for the clarification. I have either missed that entirely or forgotten again in the meatime.

All good from my side then.

@andresailer andresailer enabled auto-merge (rebase) October 14, 2024 11:52
Copy link

Test Results

   10 files     10 suites   4h 35m 15s ⏱️
  366 tests   165 ✅ 0 💤 201 ❌
1 482 runs  1 279 ✅ 0 💤 203 ❌

For more details on these failures, see this check.

Results for commit 985a4bb.

@BrieucF
Copy link
Contributor Author

BrieucF commented Oct 15, 2024

I could not relate any of the failing test to this PR...

@andresailer andresailer disabled auto-merge October 15, 2024 08:02
@andresailer andresailer merged commit 378e821 into AIDASoft:master Oct 15, 2024
9 of 14 checks passed
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.

EDM4hep as default output format and run on all events by default
3 participants