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

Fix haddnano.py to handle zero-events nanoAOD files properly #43533

Merged
merged 1 commit into from
Jan 17, 2024

Conversation

namapane
Copy link
Contributor

@namapane namapane commented Dec 9, 2023

PR description:

Fix a mis-handled case in haddnano.py, where nanoAOD files with no events were wrongly recognized as files with missing branches. As a result, the hadded file had branches screwed up.

PR validation:

Tested using a file with no events, (/store/data/Run2022F/MuonEG/NANOAOD/22Sep2023-v1/50000/4d76213a-ef14-411a-9558-559a6df3f978.root) in all possible conditions:

  • several valid files + zero-event file as the first in the list
  • several valid files + zero-event file in the middle of the list
  • several valid files + zero-event file as the last in the list
  • zero-event file as the only in the list

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 9, 2023

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-43533/38115

  • This PR adds an extra 16KB to repository

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 9, 2023

A new Pull Request was created by @namapane (Nicola Amapane) for master.

It involves the following packages:

  • PhysicsTools/NanoAOD (xpog)

@cmsbuild, @simonepigazzini, @vlimant can you please review it and eventually sign? Thanks.
@AnnikaStein, @gpetruc this is something you requested to watch as well.
@sextonkennedy, @rappoccio, @antoniovilela you are the release manager for this.

cms-bot commands are listed here

@vlimant
Copy link
Contributor

vlimant commented Jan 10, 2024

@clelange @lenzip : is this OK to sign-off ?
NB : I'd really love a signature delegation for that package if we're going to have frequent updates

@namapane
Copy link
Contributor Author

@clelange @lenzip : is this OK to sign-off ? NB : I'd really love a signature delegation for that package if we're going to have frequent updates

@vlimant, pls note that this is a fix in PhysicsTools/NanoAOD. The fixed file has been added 3 years ago and there has been only a minor update since [blame]. I would not call this "frequent updates"...

@vlimant
Copy link
Contributor

vlimant commented Jan 10, 2024

probably belongs in NANOAODTools then

@lenzip
Copy link
Contributor

lenzip commented Jan 10, 2024

I am unsure where it belongs. It was put her because it is used to merge nanoAODs.
In any case it is fine to sign off.

@vlimant
Copy link
Contributor

vlimant commented Jan 10, 2024

enable nano

@vlimant
Copy link
Contributor

vlimant commented Jan 10, 2024

please test

@namapane
Copy link
Contributor Author

I am unsure where it belongs. It was put her because it is used to merge nanoAODs. In any case it is fine to sign off.

Indeed it's nothing specific to NanoAODTools, it is intended to properly merge nanoAOD files, regardless of the way they are produced or analyzed. So I think it really belongs to NanoAOD.

@cmsbuild
Copy link
Contributor

-1

Failed Tests: RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-9aff76/36775/summary.html
COMMIT: 5c57af9
CMSSW: CMSSW_14_0_X_2024-01-09-2300/el8_amd64_gcc12
Additional Tests: NANO
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/43533/36775/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals-INPUT

The relvals timed out after 4 hours.

Comparison Summary

Summary:

NANO Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 15
  • DQMHistoTests: Total histograms compared: 16405
  • DQMHistoTests: Total failures: 0
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 16405
  • DQMHistoTests: Total skipped: 0
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 14 files compared)
  • Checked 38 log files, 18 edm output root files, 15 DQM output files

Nano size comparison Summary:

Sample kb/ev ref kb/ev diff kb/ev ev/s/thd ref ev/s/thd diff rate mem/thd ref mem/thd
2500.0 2.542 2.542 0.000 ( +0.0% ) 5.28 5.29 -0.1% 2.128 2.217
2500.001 2.688 2.688 0.000 ( +0.0% ) 4.73 4.73 -0.1% 2.538 2.650
2500.002 2.625 2.625 0.000 ( +0.0% ) 4.87 4.96 -1.7% 2.526 2.627
2500.01 1.312 1.312 0.000 ( +0.0% ) 9.60 9.74 -1.4% 2.227 2.332
2500.011 1.729 1.729 0.000 ( +0.0% ) 5.20 5.33 -2.4% 2.410 2.530
2500.012 1.575 1.575 0.000 ( +0.0% ) 7.50 7.63 -1.7% 2.365 2.445
2500.1 2.190 2.190 0.000 ( +0.0% ) 5.37 5.32 +0.9% 2.077 2.078
2500.2 2.304 2.304 0.000 ( +0.0% ) 6.14 6.11 +0.5% 1.990 1.998
2500.21 1.180 1.180 0.000 ( +0.0% ) 4.40 4.37 +0.5% 2.269 2.285
2500.211 1.542 1.542 0.000 ( +0.0% ) 3.84 3.84 +0.2% 2.251 2.371
2500.3 2.059 2.059 0.000 ( +0.0% ) 12.92 12.91 +0.1% 1.975 1.976
2500.31 1.255 1.255 0.000 ( +0.0% ) 20.44 20.67 -1.1% 2.356 2.374
2500.311 1.642 1.642 0.000 ( +0.0% ) 13.44 13.80 -2.6% 2.446 2.454
2500.312 7.025 7.025 0.000 ( +0.0% ) 1.39 1.32 +5.3% 1.711 1.712
2500.313 1.471 1.471 0.000 ( +0.0% ) 6.81 6.73 +1.2% 1.040 1.040
2500.4 2.059 2.059 0.000 ( +0.0% ) 12.79 12.91 -1.0% 1.979 1.982
2500.5 19.575 19.575 0.000 ( +0.0% ) 1.12 1.19 -5.8% 1.124 1.355

@namapane
Copy link
Contributor Author

Error is a timeout that is unrelated to the PR. Maybe the test could be retriggered?

@vlimant
Copy link
Contributor

vlimant commented Jan 16, 2024

please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-9aff76/36857/summary.html
COMMIT: 5c57af9
CMSSW: CMSSW_14_0_X_2024-01-16-1100/el8_amd64_gcc12
Additional Tests: NANO
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/43533/36857/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • You potentially added 88 lines to the logs
  • Reco comparison results: 1 differences found in the comparisons
  • DQMHistoTests: Total files compared: 48
  • DQMHistoTests: Total histograms compared: 3247526
  • DQMHistoTests: Total failures: 0
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3247504
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
  • Checked 200 log files, 161 edm output root files, 48 DQM output files
  • TriggerResults: no differences found

NANO Comparison Summary

Summary:

  • You potentially removed 5 lines from the logs
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 15
  • DQMHistoTests: Total histograms compared: 16489
  • DQMHistoTests: Total failures: 0
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 16489
  • DQMHistoTests: Total skipped: 0
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 14 files compared)
  • Checked 38 log files, 18 edm output root files, 15 DQM output files

Nano size comparison Summary:

Sample kb/ev ref kb/ev diff kb/ev ev/s/thd ref ev/s/thd diff rate mem/thd ref mem/thd
2500.0 2.544 2.544 0.000 ( +0.0% ) 5.24 5.24 +0.0% 2.224 2.173
2500.001 2.691 2.691 0.000 ( +0.0% ) 4.72 4.72 +0.0% 2.662 2.588
2500.002 2.629 2.629 0.000 ( +0.0% ) 4.88 4.88 +0.0% 2.647 2.580
2500.01 1.314 1.314 0.000 ( +0.0% ) 9.81 9.55 +2.7% 2.347 2.255
2500.011 1.730 1.730 0.000 ( +0.0% ) 5.20 5.15 +1.0% 2.536 2.414
2500.012 1.576 1.576 0.000 ( +0.0% ) 7.54 7.42 +1.6% 2.444 2.304
2500.1 2.192 2.192 0.000 ( +0.0% ) 5.31 5.31 -0.0% 2.058 2.000
2500.2 2.307 2.307 0.000 ( +0.0% ) 6.02 5.99 +0.4% 1.975 1.890
2500.21 1.182 1.182 0.000 ( +0.0% ) 4.32 4.35 -0.7% 2.271 2.204
2500.211 1.545 1.545 0.000 ( +0.0% ) 3.78 3.67 +3.0% 2.362 2.227
2500.3 2.062 2.062 0.000 ( +0.0% ) 12.72 12.42 +2.4% 1.964 1.892
2500.31 1.257 1.257 0.000 ( +0.0% ) 20.09 19.31 +4.1% 2.353 2.251
2500.311 1.645 1.645 0.000 ( +0.0% ) 13.47 12.70 +6.1% 2.440 2.312
2500.312 7.025 7.025 0.000 ( +0.0% ) 1.37 1.33 +3.0% 1.688 1.688
2500.313 1.471 1.471 0.000 ( +0.0% ) 6.85 6.50 +5.3% 1.043 1.046
2500.4 2.062 2.062 0.000 ( +0.0% ) 12.77 12.42 +2.8% 1.965 1.889
2500.5 19.575 19.575 0.000 ( +0.0% ) 1.18 1.13 +4.4% 1.354 1.101

@vlimant
Copy link
Contributor

vlimant commented Jan 17, 2024

+1

@cmsbuild
Copy link
Contributor

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. @rappoccio, @sextonkennedy, @antoniovilela (and backports should be raised in the release meeting by the corresponding L2)

@rappoccio
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 87aa866 into cms-sw:master Jan 17, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants