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

Use isolated particle level photons #28371

Merged
merged 3 commits into from Nov 11, 2019

Conversation

AndrewLevin
Copy link
Contributor

PR description:

In order to measure isolated photon cross sections, we need to have isolated particle level photons.

PR validation:

I have run over some recent relval samples and checked that the number of isolated particles level photon is reasonable.

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 9, 2019

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 9, 2019

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-28371/12684

  • This PR adds an extra 12KB to repository

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-28371/12685

  • This PR adds an extra 12KB to repository

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @AndrewLevin for master.

It involves the following packages:

GeneratorInterface/RivetInterface

@SiewYan, @efeyazgan, @mkirsano, @cmsbuild, @agrohsje, @alberto-sanchez, @qliphy can you please review it and eventually sign? Thanks.
@alberto-sanchez, @agrohsje, @mkirsano this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

@qliphy
Copy link
Contributor

qliphy commented Nov 10, 2019

adding @intrepid42 @xjanssen

@qliphy
Copy link
Contributor

qliphy commented Nov 10, 2019

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 10, 2019

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/3410/console Started: 2019/11/10 12:36

@cmsbuild
Copy link
Contributor

-1

Tested at: 5546d11

You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-4f43e0/3410/summary.html

I found follow errors while testing this PR

Failed tests: RelVals

  • RelVals:

When I ran the RelVals I found an error in the following workflows:
1325.7 step2

runTheMatrix-results/1325.7_TTbar_13_94Xv2NanoAODINPUT+TTbar_13_94Xv2NanoAODINPUT+NANOEDMMC2017_94XMiniAODv2+HARVESTNANOAODMC2017_94XMiniAODv2/step2_TTbar_13_94Xv2NanoAODINPUT+TTbar_13_94Xv2NanoAODINPUT+NANOEDMMC2017_94XMiniAODv2+HARVESTNANOAODMC2017_94XMiniAODv2.log

1330.0 step5
runTheMatrix-results/1330.0_ZMM_13+ZMM_13+DIGIUP15+RECOUP15_L1TMuDQM+HARVESTUP15_L1TMuDQM+NANOUP15/step5_ZMM_13+ZMM_13+DIGIUP15+RECOUP15_L1TMuDQM+HARVESTUP15_L1TMuDQM+NANOUP15.log

10042.0 step6
runTheMatrix-results/10042.0_ZMM_13+ZMM_13TeV_TuneCUETP8M1_2017_GenSimFull+DigiFull_2017+RecoFull_2017+HARVESTFull_2017+ALCAFull_2017+NanoFull_2017/step6_ZMM_13+ZMM_13TeV_TuneCUETP8M1_2017_GenSimFull+DigiFull_2017+RecoFull_2017+HARVESTFull_2017+ALCAFull_2017+NanoFull_2017.log

10024.0 step6
runTheMatrix-results/10024.0_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2017_GenSimFull+DigiFull_2017+RecoFull_2017+HARVESTFull_2017+ALCAFull_2017+NanoFull_2017/step6_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2017_GenSimFull+DigiFull_2017+RecoFull_2017+HARVESTFull_2017+ALCAFull_2017+NanoFull_2017.log

10824.0 step6
runTheMatrix-results/10824.0_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2018_GenSimFull+DigiFull_2018+RecoFull_2018+HARVESTFull_2018+ALCAFull_2018+NanoFull_2018/step6_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2018_GenSimFull+DigiFull_2018+RecoFull_2018+HARVESTFull_2018+ALCAFull_2018+NanoFull_2018.log

25202.0 step5
runTheMatrix-results/25202.0_TTbar_13+TTbar_13+DIGIUP15_PU25+RECOUP15_PU25+HARVESTUP15_PU25+NANOUP15_PU25/step5_TTbar_13+TTbar_13+DIGIUP15_PU25+RECOUP15_PU25+HARVESTUP15_PU25+NANOUP15_PU25.log

10224.0 step5
runTheMatrix-results/10224.0_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2017PU_GenSimFull+DigiFullPU_2017PU+RecoFullPU_2017PU+HARVESTFullPU_2017PU+NanoFull_2017PU/step5_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2017PU_GenSimFull+DigiFullPU_2017PU+RecoFullPU_2017PU+HARVESTFullPU_2017PU+NanoFull_2017PU.log

@cmsbuild
Copy link
Contributor

Comparison not run due to runTheMatrix errors (RelVals and Igprof tests were also skipped)

@cmsbuild
Copy link
Contributor

@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-4f43e0/3411/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 34
  • DQMHistoTests: Total histograms compared: 2936360
  • DQMHistoTests: Total failures: 20
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2935999
  • DQMHistoTests: Total skipped: 341
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 33 files compared)
  • Checked 147 log files, 16 edm output root files, 34 DQM output files

@mseidel42
Copy link
Contributor

Hi, looks fine to me, thank you for implementing! I verified that all the prompt photons are still stored in the nanoAod "GenPart" collection if some analysis needs them.

My only worry would be that some analysis might be using the current particleLevel:photons and unknowingly switches from prompt to isolated photons.
But we could send a mail to the Rivet HN to announce this change.

@qliphy
Copy link
Contributor

qliphy commented Nov 11, 2019

+1

@peruzzim
Copy link
Contributor

+xpog

I think at the moment we're not saving particleLevel:photons in NanoAOD, so it will not affect anything in the NanoAOD content. In this sense, the configuration change is purely technical.
Can you confirm this is totally unrelated to HTXSRivetProducer, and will not affect its results?

For the more general item of changing the default configuration of the producer, it's up to GEN to follow up - but I agree it should be clearly announced to physics groups.

@fabiocos
Copy link
Contributor

+1

@AndrewLevin please consider the question #28371 (comment)

@fabiocos
Copy link
Contributor

merge

@cmsbuild cmsbuild merged commit 7ab1e3f into cms-sw:master Nov 11, 2019
@AndrewLevin
Copy link
Contributor Author

+1

@AndrewLevin please consider the question #28371 (comment)

I am pretty sure this is totally independent of the HTXSRivetProducer.

@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.

None yet

7 participants