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

GenJetFlavourInfos: fix compiler warning, use prunedGenParticles in associations to correctly match pat jet flavour info #20281

Merged
merged 5 commits into from Sep 1, 2017

Conversation

Andrej-CMS
Copy link
Contributor

please see #19048
addressed comment by @davidlange6

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @Andrej-CMS for master.

It involves the following packages:

PhysicsTools/PatAlgos

@perrotta, @cmsbuild, @monttj, @slava77 can you please review it and eventually sign? Thanks.
@TaiSakuma, @gouskos, @imarches, @ahinzmann, @acaudron, @mmarionncern, @rappoccio, @jdolen, @nhanvtran, @gpetruc, @gkasieczka, @schoef, @ferencek, @mverzett, @mariadalfonso, @pvmulder, @seemasharmafnal, @JyothsnaKomaragiri this is something you requested to watch as well.
@davidlange6, @slava77 you are the release manager for this.

cms-bot commands are listed here

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/PR-20281/339

Code check has found code style and quality issues which could be resolved by applying a patch in https://cmssdt.cern.ch/SDT/code-checks/PR-20281/339/git-diff.patch
e.g. curl https://cmssdt.cern.ch/SDT/code-checks/PR-20281/339/git-diff.patch | patch -p1

In future, you can run scram build code-checks to apply code checks

@perrotta
Copy link
Contributor

@Andrej-CMS : maybe you can profit and apply also the two small fixes suggested by the code-checker

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

Pull request #20281 was updated. @perrotta, @cmsbuild, @monttj, @slava77 can you please check and sign again.

@perrotta
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 28, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/22517/console Started: 2017/08/28 12:34

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/PR-20281/340

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

-1

Tested at: be715ba

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
60ea8c8
You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-20281/22588/git-log-recent-commits
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-20281/22588/git-merge-result

You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-20281/22588/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 worklfows:
1306.0 step4

runTheMatrix-results/1306.0_SingleMuPt1_UP15+SingleMuPt1_UP15+DIGIUP15+RECOUP15+HARVESTUP15/step4_SingleMuPt1_UP15+SingleMuPt1_UP15+DIGIUP15+RECOUP15+HARVESTUP15.log

1330.0 step4
runTheMatrix-results/1330.0_ZMM_13+ZMM_13+DIGIUP15+RECOUP15+HARVESTUP15/step4_ZMM_13+ZMM_13+DIGIUP15+RECOUP15+HARVESTUP15.log

10042.0 step5
runTheMatrix-results/10042.0_ZMM_13+ZMM_13TeV_TuneCUETP8M1_2017_GenSimFull+DigiFull_2017+RecoFull_2017+ALCAFull_2017+HARVESTFull_2017/step5_ZMM_13+ZMM_13TeV_TuneCUETP8M1_2017_GenSimFull+DigiFull_2017+RecoFull_2017+ALCAFull_2017+HARVESTFull_2017.log

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
60ea8c8
You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-20281/22588/git-log-recent-commits
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-20281/22588/git-merge-result

@cmsbuild
Copy link
Contributor

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

@slava77
Copy link
Contributor

slava77 commented Aug 30, 2017

@cmsbuild please test
hopefully failures in harvesting are transient
@dmitrijus please take a note.

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 30, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/22595/console Started: 2017/08/30 16:07

@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-20281/22595/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 18 differences found in the comparisons
  • DQMHistoTests: Total files compared: 26
  • DQMHistoTests: Total histograms compared: 2653934
  • DQMHistoTests: Total failures: 210
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2653535
  • DQMHistoTests: Total skipped: 189
  • DQMHistoTests: Total Missing objects: 0
  • Checked 107 log files, 14 edm output root files, 26 DQM output files

@slava77
Copy link
Contributor

slava77 commented Aug 30, 2017

+1

for #20281 be715ba

  • several fixes
    • technical fix for the compiler warning in GenJetFlavourInfoPreserver.cc;
    • functional fix to select prunedGenParticles as a source of the jet flavor ID, to be in agreement with the pat jets choice
  • jenkins tests pass and comparisons with baseline show differences only in the recently added slimmedGenJetsFlavourInfos parton flavor distributions

@Andrej-CMS please update the PR title to something more descriptive:
e.g. "GenJetFlavourInfos: fix compiler warning, use prunedGenParticles in associations to correctly match pat jet flavour info"

@Andrej-CMS Andrej-CMS changed the title GenJetFlavourInfos changes, addressed comments GenJetFlavourInfos: fix compiler warning, use prunedGenParticles in associations to correctly match pat jet flavour info Aug 31, 2017
@davidlange6
Copy link
Contributor

merge

@cmsbuild cmsbuild merged commit 20b60f7 into cms-sw:master Sep 1, 2017
@ferencek
Copy link
Contributor

ferencek commented Sep 4, 2017

@@ -326,6 +326,7 @@ def miniAOD_customizeMC(process):
#GenJetFlavourInfos
process.load("PhysicsTools.JetMCAlgos.HadronAndPartonSelector_cfi")
task.add(process.selectedHadronsAndPartons)
Copy link
Contributor

Choose a reason for hiding this comment

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

Apologies for not noticing this earlier but with the updates introduced in this PR, is anything consuming selectedHadronsAndPartons now? If not, this line can be removed.

@arizzi
Copy link
Contributor

arizzi commented Oct 6, 2017

@Andrej-CMS any answer to the comment from @ferencek

This should be fixed before 940 is out

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

8 participants