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

feat: switch to DDXv2 as baseline #31618

Merged
merged 5 commits into from Oct 12, 2020
Merged

Conversation

andrzejnovak
Copy link
Contributor

@andrzejnovak andrzejnovak commented Sep 29, 2020

PR description:

Follow-up on the proposed BTV changes in
https://indico.cern.ch/event/951991/contributions/3999401/attachments/2107928/3545219/BTV_XPOG_meeting_23-09-2020.pdf

  • switch to DDXv2 as primary fatjet tagger for BTV.
  • Nano changes
    • Deprecate DDX (v1) in Nano after 106
    • Include DDXv2 starting 106
    • Add "hadronFlavour for subjets
    • Handle DeepCSV defaults correctly (individual probabilties default to -1 creating weird composite scores)
    • Protect Deepjet composite scores against 0 denominator
    • Deprecate CMVA for jets/fatjets/subjets

@smoortga @XavierAtCERN

PR validation:

if this PR is a backport please specify the original PR and why you need to backport that PR:

Before submitting your pull requests, make sure you followed this checklist:

@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-31618/18684

  • This PR adds an extra 16KB to repository

  • There are other open Pull requests which might conflict with changes you have proposed:

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @andrzejnovak (Andrzej Novak) for master.

It involves the following packages:

PhysicsTools/PatAlgos

@perrotta, @jpata, @cmsbuild, @santocch, @slava77 can you please review it and eventually sign? Thanks.
@jdamgov, @rappoccio, @gouskos, @jdolen, @ahinzmann, @smoortga, @riga, @schoef, @emilbols, @mariadalfonso, @JyothsnaKomaragiri, @nhanvtran, @gkasieczka, @clelange, @hatakeyamak, @ferencek, @gpetruc, @andrzejnovak, @peruzzim, @seemasharmafnal, @mmarionncern 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

@slava77
Copy link
Contributor

slava77 commented Sep 29, 2020

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 29, 2020

The tests are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

+1
Tested at: 5bbb39e
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-ce1e59/9653/summary.html
CMSSW: CMSSW_11_2_X_2020-09-29-1100
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-ce1e59/9653/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 3476 differences found in the comparisons
  • DQMHistoTests: Total files compared: 35
  • DQMHistoTests: Total histograms compared: 2542225
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2542202
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 34 files compared)
  • Checked 149 log files, 22 edm output root files, 35 DQM output files

@mariadalfonso
Copy link
Contributor

mariadalfonso commented Sep 30, 2020

@andrzejnovak

to complete this migration also the nano files should be updated

_btagDiscriminators += ['pfDeepDoubleBvLJetTags:probHbb', \
'pfDeepDoubleCvLJetTags:probHcc', \
'pfDeepDoubleCvBJetTags:probHcc', \
'pfMassIndependentDeepDoubleBvLJetTags:probHbb', 'pfMassIndependentDeepDoubleCvLJetTags:probHcc', 'pfMassIndependentDeepDoubleCvBJetTags:probHcc']

btagDDBvL_noMD = Var("bDiscriminator('pfDeepDoubleBvLJetTags:probHbb')",float,doc="DeepDoubleX discriminator (no mass-decorrelation) for H(Z)->bb vs QCD",precision=10),
btagDDCvL_noMD = Var("bDiscriminator('pfDeepDoubleCvLJetTags:probHcc')",float,doc="DeepDoubleX discriminator (no mass-decorrelation) for H(Z)->cc vs QCD",precision=10),
btagDDCvB_noMD = Var("bDiscriminator('pfDeepDoubleCvBJetTags:probHcc')",float,doc="DeepDoubleX discriminator (no mass-decorrelation) for H(Z)->cc vs H(Z)->bb",precision=10),
btagDDBvL = Var("bDiscriminator('pfMassIndependentDeepDoubleBvLJetTags:probHbb')",float,doc="DeepDoubleX (mass-decorrelated) discriminator for H(Z)->bb vs QCD",precision=10),
btagDDCvL = Var("bDiscriminator('pfMassIndependentDeepDoubleCvLJetTags:probHcc')",float,doc="DeepDoubleX (mass-decorrelated) discriminator for H(Z)->cc vs QCD",precision=10),
btagDDCvB = Var("bDiscriminator('pfMassIndependentDeepDoubleCvBJetTags:probHcc')",float,doc="DeepDoubleX (mass-decorrelated) discriminator for H(Z)->cc vs H(Z)->bb",precision=10),

Note we need to keep the OLD taggers for the EOY miniAOD

(run2_nanoAOD_94X2016 | run2_nanoAOD_94XMiniAODv1 | run2_nanoAOD_94XMiniAODv2 | run2_nanoAOD_102Xv1).toModify(

but switch to the new one for the 'run2_nanoAOD_106Xv1' and master

run2_nanoAOD_106Xv1.toModify(

@andrzejnovak
Copy link
Contributor Author

@mariadalfonso This is expected along with few other btag changes. Should it be done in the same PR?

@mariadalfonso
Copy link
Contributor

@mariadalfonso This is expected along with few other btag changes. Should it be done in the same PR?

I think will make easier the review/backport if done simultaneously

@andrzejnovak
Copy link
Contributor Author

I thought, in general, the process of proposing changes to nano was to use the dedicated repo which gets merged to cmssw/master periodically in bulk, is that not the case anymore?

@mariadalfonso
Copy link
Contributor

I thought, in general, the process of proposing changes to nano was to use the dedicated repo which gets merged to cmssw/master periodically in bulk, is that not the case anymore?

@andrzejnovak
the cms-nano has been deprecated at the end of summer and the PRs go directly to cms-sw (master first and then 10_6). Now the nano format is well established and objects are developed that require change in both mini and nano
All the v8 nano features already followed this new path cms-nanoAOD#533

@andrzejnovak
Copy link
Contributor Author

andrzejnovak commented Oct 8, 2020

@mariadalfonso it seems to work as expected locally
image

The baseline is blue right? Is it possible the scale between the two is off due to the -2 bin not being in the range of the DQM hist? It looks to me a bit like the 0-1 range difference is just an overall factor of ~2 without differences to the shape, which would be strange result in the context of the change introduced here.

@mariadalfonso
Copy link
Contributor

+xpog
changes inline with the description

@slava77
Copy link
Contributor

slava77 commented Oct 9, 2020

+1

for #31618 49d2379

@mariadalfonso
Copy link
Contributor

@andrzejnovak
consider back porting this PR to 10_6_X

@andrzejnovak
Copy link
Contributor Author

Is this waiting for the backport to be opened?

@slava77
Copy link
Contributor

slava77 commented Oct 12, 2020

@silviodonato @qliphy
please check this PR and consider for merging without the analysis signature
Thank you.

@silviodonato
Copy link
Contributor

+1

@silviodonato
Copy link
Contributor

merge

@cmsbuild cmsbuild merged commit 540da10 into cms-sw:master Oct 12, 2020
@silviodonato
Copy link
Contributor

@andrzejnovak please look at #31768

andrzejnovak added a commit to andrzejnovak/cmssw that referenced this pull request Oct 14, 2020
cmsbuild added a commit that referenced this pull request Oct 14, 2020
fix: adjust jme_nano cff for changes in #31618
@mariadalfonso
Copy link
Contributor

@andrzejnovak
please go ahead with the backport of this and #31786

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

andrzejnovak added a commit to andrzejnovak/cmssw that referenced this pull request Oct 19, 2020
cmsbuild added a commit that referenced this pull request Nov 4, 2020
[10_6_X] DDXv2 and BTV changes in Nano #31618, #31786
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