-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Fix NanoAODRNTupleOutputModule #49551
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
Fix NanoAODRNTupleOutputModule #49551
Conversation
|
cms-bot internal usage |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-49551/47067
|
|
A new Pull Request was created by @ariostas for master. It involves the following packages:
@battibass, @cmsbuild, @ftorrresd can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
@makortel fyi |
|
@cmsbuild, please test |
|
+1 Size: This PR adds an extra 24KB to repository DAS Queries: The DAS query tests failed, see the summary page for details. Comparison SummarySummary:
|
|
enable nano |
|
please test |
|
+1 Size: This PR adds an extra 16KB to repository DAS Queries: The DAS query tests failed, see the summary page for details. Comparison SummarySummary:
Nano size comparison Summary:
NANO Comparison SummarySummary:
|
|
@cms-sw/xpog-l2 It would be great to have this PR in 16_0_0_pre4 (scheduled to be built next week) |
|
+xpog |
|
This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @sextonkennedy, @ftenchini, @mandrenguyen (and backports should be raised in the release meeting by the corresponding L2) |
@makortel, done, as agreed on Thursday. |
|
Also adding here a couple notes, as they were not mentioned in the PR description:
|
|
Thanks! |
|
+1 |
|
@ariostas , we now have conflicts in cmssw master and RNTUPLE_X branch [a]. Can you please provide a PR for RNTUPLE_X branch to resolve this conflict ? [a] https://cmssdt.cern.ch/jenkins/job/auto-forward-port-branch/63402/console |
|
@smuzaffar yeah, I'll do that. I'm a bit confused why there are conflicts since I just cherry-picked the commits, but I'll look into it. Thanks! |
|
Oh sorry, I missed #49239 in here. @Dr15Jones do you want to open a PR to port it to master, or should I do it? |
|
please go ahead and do the port to master. |
This is a port of #48071 and #48818 (which where on the RNTUPLE_X branch) to master. It fixes the NanoAODRNTupleOutputModule so that it is compatible with the new RNTuple API (most of which is stable now). It is successfully able to output RNTuple NanoAODs with a nicer organization than before, but the implementation is not finalized, as delayed flags are not implemented yet and field projections will probably be needed for backwards compatibility.