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

NanoAOD: add flag to keep all PS weights #31276

Merged
merged 7 commits into from Sep 23, 2020

Conversation

swertz
Copy link
Contributor

@swertz swertz commented Aug 28, 2020

PR description:

Note: was originally cms-nanoAOD#542
FYI @camclean @alefisico

PR validation:

Tested in 106X: produced nanoAOD files for TTToSemiLeptonic (2016 and 2018) and for one of the parameter scan files mentioned in cms-nanoAOD#441 [1].

[1] /store/mc/RunIIAutumn18MiniAOD/SMS-TChiWZ_ZToLL_mZMin-0p1_mC1-325to1000_TuneCP2_13TeV-madgraphMLM-pythia8/MINIAODSIM/GridpackScan_102X_upgrade2018_realistic_v15-v1/70000/FFE78E85-6F7A-9441-8963-980A38403D1F.root

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-31276/17971

Code check has found code style and quality issues which could be resolved by applying following patch(s)

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-31276/17973

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @swertz (Sébastien Wertz) for master.

It involves the following packages:

PhysicsTools/NanoAOD

@gouskos, @cmsbuild, @fgolf, @mariadalfonso, @santocch, @peruzzim can you please review it and eventually sign? Thanks.
@gpetruc this is something you requested to watch as well.
@silviodonato, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

@mariadalfonso
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 28, 2020

The tests are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

+1
Tested at: 50bf2c3
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-7cb620/8983/summary.html
CMSSW: CMSSW_11_2_X_2020-08-27-2300
SCRAM_ARCH: slc7_amd64_gcc820

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-7cb620/8983/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 35
  • DQMHistoTests: Total histograms compared: 2609667
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2609644
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.324 KiB( 34 files compared)
  • DQMHistoSizes: changed ( 1325.7 ): 0.324 KiB Physics/NanoAODDQM
  • Checked 149 log files, 22 edm output root files, 35 DQM output files

@swertz
Copy link
Contributor Author

swertz commented Sep 22, 2020

Wouldn't the LHEScaleWeight or LHEPdfWeight be empty or not produced in cases where they don't exist?

Indeed, so it would be more consistent to also leave the PSWeight array empty if there are no weights.

@agrohsje
Copy link

My concern was not so much consistency, but more being stable for users. I would prefer keeping the current implementation.

@mariadalfonso
Copy link
Contributor

+xpog
changes are inline with the PR description.

If activated by the flag, the PS weights appear as listed here
#31276 (comment)

@agrohsje
Copy link

+generators

@qliphy
Copy link
Contributor

qliphy commented Sep 23, 2020

+1

@qliphy qliphy merged commit 5cfc0ce into cms-sw:master Sep 23, 2020
@mariadalfonso
Copy link
Contributor

@swertz
consider back-porting this PR in 10_6

@agrohsje
Copy link

@mariadalfonso What about 10_2 and 9_4? Can one backport?

@mariadalfonso
Copy link
Contributor

@agrohsje
Master or 10_6 can perfectly run on EOY miniAOD make with 10_2 and 94X.
for the moment we are back porting all the nano only to 10_6 for the UL reprocessing.
Is there any particular reason to backport the GEN nano-fixes to previous releases ?

Note: NanoGen is only available in 11_2

@agrohsje
Copy link

v7 is currently using 10_2:
https://cms-pdmv.cern.ch/mcm/campaigns?prepid=Run*NanoAODv7
Will we move to 10_6 also for these?

@mariadalfonso
Copy link
Contributor

v7 is currently using 10_2:
https://cms-pdmv.cern.ch/mcm/campaigns?prepid=Run*NanoAODv7
Will we move to 10_6 also for these?

We will not be making anymore V7 centrally, and we didn't get requests on fixing older releases for private renano of the V7.

@agrohsje
Copy link

Hi @mariadalfonso . The problem is that we had failing HIG requests due to the bug. So they would need to resubmit nano-aod. I assume for this it might be easiest to update v7?

@mariadalfonso
Copy link
Contributor

mariadalfonso commented Sep 24, 2020

Hi @mariadalfonso . The problem is that we had failing HIG requests due to the bug. So they would need to resubmit nano-aod. I assume for this it might be easiest to update v7?

ok, we will follow up with the PC group, so that's will be clear what are the requests for the EOY and 10_2 and how to best help the private reprocessing. There are other bug-fixes besides this one among the cms-nanoAOD#533

For the time being let's complete the 10_6

@santocch
Copy link

+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 be automatically merged.

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.

Bug for MC genweight vector of size 1
7 participants