[102X][Backport] DeepAK8 tagger integration #24505

hqucms · 2018-09-11T14:59:02Z

Backport of #23768.
Needs external from cms-sw/cmsdist#4326.
The tagger is disabled in the MiniAOD sequence by default to preserve the event content.

- Both the nominal and the decorrelated versions are included, as well as a few meta taggers (aggregating the scores). - Currently it supports only updating a jet collection. The implementation to run on RECO exists but not tested.

Re-bind executor every time for thread-safety.

cmsbuild · 2018-09-12T23:03:15Z

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

jmduarte · 2018-09-12T23:07:52Z

@hqucms Are you saying this issue is only causing a difference when running the tagger on RECO inputs?

How should I test that this? e.g. Should I compare the tagger output running on both RECO and MINIAOD inputs before and after this PR is applied?

Thanks,
Javier

hqucms · 2018-09-12T23:08:24Z

@slava77
I found a workaround to avoid the change in d7a1c26. This should allow us to preserve the event content in the backport.

slava77 · 2018-09-12T23:22:25Z

@cmsbuild please test with cms-sw/cmsdist#4326

cmsbuild · 2018-09-12T23:23:54Z

The tests are being triggered in jenkins.
Using externals from cms-sw/cmsdist#4326
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/30386/console

hqucms · 2018-09-13T00:06:38Z

@jmduarte

If I understand correctly, the way that DeepDoubleB was set up in the MiniAOD sequence (in https://github.com/cms-sw/cmssw/blob/CMSSW_10_2_X/PhysicsTools/PatAlgos/python/slimming/applyDeepBtagging_cff.py#L49-L68), i.e., by updating slimmedJetsAK8, was because you wanted to access pat::PackedCandidate, which was fully consistent with the training input from MiniAOD files, right? However, due to changes introduced in #22914, the daughters of slimmedJetsAK8 are still reco::PFCandidate before slimmedJetsAK8 is written to MiniAOD file (because the daughters are cached in memory), and therefore DeepDoubleB was actually accessing reco::PFCandidate when running in the MiniAOD step.

This PR, specifically d7a1c26, fixes the change in #22914 such that the daughters of slimmedJetsAK8 are always pat::PackedCandidate. Then, DeepDoubleB running in MiniAOD step will use pat::PackedCandidate which I think is more consistent with the training, right? However, this is then causing some difference for the DeepDoubleB scores produced in the MiniAOD step:

for AK8 jets with pt>170, the difference seems very small (I think you also checked in the past using RECO inputs and MINIAOD inputs gave very consistent results, so probably nothing to worry here).
for AK8 jets with pt<170, the difference is much larger. This is because the jet daughters are dropped for slimmedJetsAK8 (https://github.com/cms-sw/cmssw/blob/CMSSW_10_2_X/PhysicsTools/PatAlgos/python/slimming/applySubstructure_cff.py#L306), while previously since we were getting the daughters from the cache, we still have access to all the reco::PFCandidate daughters.

However, I think only the high pt AK8 jets are of interest for applying DeepDoubleB, right? And since the training was derived with high pt jets, it is most likely not optimal/applicable for these low pt jets anyhow.

What do you think, @jmduarte?

cmsbuild · 2018-09-13T03:12:42Z

+1
Tested at: ed815a3
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-24505/30386/summary.html

cmsbuild · 2018-09-13T03:12:49Z

Comparison job queued.

cmsbuild · 2018-09-13T04:50:02Z

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

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 31
DQMHistoTests: Total histograms compared: 2987919
DQMHistoTests: Total failures: 3
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2987726
DQMHistoTests: Total skipped: 190
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 30 files compared)
Checked 129 log files, 14 edm output root files, 31 DQM output files

slava77 · 2018-09-14T16:38:45Z

+1

for #24505 ed815a3

the backport of DeepAK8 tagger integration #23768 1acac95 is correct: it is not verbatim to satisfy the no-change policy. The deepAK8 taggers can be enabled in analysis setup.
- the deepAK8 taggers are disabled (can be enabled by uncommenting some code in PhysicsTools/PatAlgos/python/slimming/applyDeepBtagging_cff.py
- the changes in DataFormats/Candidate/interface/CompositePtrCandidate.h and DataFormats/PatCandidates/interface/Jet.h are not in this backport PR to preserve the default behavior of the deepDoubleB tagger currently running in production. From the discussion so far in [102X][Backport] DeepAK8 tagger integration #24505 (comment) related to these, the change in the master branch seems appropriate.
jenkins tests pass and comparisons with the baseline show no differences as expected

@fabiocos this PR needs to be merged with cms-sw/cmsdist#4326
IIUC, the dependence on the external becomes essential at runtime only in multi-threaded mode. So, I do not expect any build/run issues in the IBs if the updates are not in sync.

hqucms · 2018-09-14T16:50:11Z

@slava77

Thank you very much for your review!

@fabiocos this PR needs to be merged with cms-sw/cmsdist#4326
IIUC, the dependence on the external becomes essential at runtime only in multi-threaded mode. So, I do not expect any build/run issues in the IBs if the updates are not in sync.

Actually cms-sw/cmsdist#4326 needs to be merged before this PR, because cms-sw/cmsdist#4326 also adds some header files to the MXNet external which are necessary for this PR to compile.

slava77 · 2018-09-14T17:05:04Z

On 9/14/18 9:50 AM, Huilin Qu wrote: @slava77 Thank you very much for your review! @fabiocos this PR needs to be merged with cms-sw/cmsdist#4326 IIUC, the dependence on the external becomes essential at runtime only in multi-threaded mode. So, I do not expect any build/run issues in the IBs if the updates are not in sync. Actually cms-sw/cmsdist#4326 needs to be merged before this PR, because cms-sw/cmsdist#4326 also adds some header files to the MXNet external which are necessary for this PR to compile.

good to know. Thanks for the correction, it may have averted a minor disaster.

fabiocos · 2018-10-01T13:29:22Z

PhysicsTools/PatAlgos/python/slimming/applyDeepBtagging_cff.py

       btagDiscriminators = [
          'pfDeepDoubleBJetTags:probQ',
          'pfDeepDoubleBJetTags:probH',
       ],
-       postfix = 'SlimmedAK8DeepDoubleB'+postfix,
+#        ] + pfDeepBoostedJetTagsAll,  # uncomment it to test DeepBoostedJet


@slava77 I understand that this is the line that would trigger the production of the new collection, for the erst the old one is renamed (DeepDoubleB -> DeppTags) but it is not a persistent collection

I'm not sure I understand for the erst .

The change of the name in the transient collection names starts at L49 and the postfix above also updates other transient collection names.

fabiocos · 2018-10-03T15:13:57Z

+1

fabiocos · 2018-10-03T15:15:09Z

merge

hqucms added 30 commits September 11, 2018 06:16

Add MXNet predictor based on the C API.

cdfb9a5

DataFormats change for DeepAK8.

678c743

Add DeepAK8 tagger.

55b6cd7

- Both the nominal and the decorrelated versions are included, as well as a few meta taggers (aggregating the scores). - Currently it supports only updating a jet collection. The implementation to run on RECO exists but not tested.

Apply code-checks.

6aa6cca

Fix dependency of meta taggers.

fc1fd12

Fix misuse of edm::Ptr and some clean-ups.

20b9aec

Override clearDaughters in pat::Jet to reset the daughter cache too.

d7a1c26

Some improvements and clean-ups.

d0e4e1a

Add a test unit for MXNetPredictor.

794b953

Fix DeepBoostedJetTagInfoProducer.

e1e9e1a

Need to sort subjets by pt.

70477ac

Thread-safety and better logging in MXNetPredictor.

957cf9d

Switch to MXNet C++ API.

a976d4e

Protect executor creatation with a mutex in MXNetCppPredictor.

830e1c3

Refactor MXNetCppPredictor.

0966b24

Re-bind executor every time for thread-safety.

Remove MXNet C API.

e8632e0

Move non-TF code in RecoBTag/TensorFlow to RecoBTag/FeatureTools

03fc196

Code style changes.

f13af0c

Fix BuildFile.

8997d92

Modify MXNet source to avoid rebinding.

b12fd79

Fix BuildFile.

d1eaf16

Improve the checks when setting up pfDeepBoostedJetTagInfos.

f4562fa

Update DeepBoostedJet meta-taggers.

3aae32e

Rename MXNet Predictor.

2fcd0fb

Style and performance improvements.

fad4dc3

Mark DeepBoostedJetTagInfo as transient.

22d3cb5

Reorganize TagInfo producers and JetTags producers.

4e5fb07

A few fixes.

0f3a195

Improve the setup of DeepBoostedJetTagInfos.

1f639cf

Disable DeepBoostedJet in applyDeepBtagging_cff.py.

32b2b2c

cmsbuild added the tests-pending label Sep 12, 2018

cmsbuild added requires-external tests-started and removed tests-pending labels Sep 12, 2018

cmsbuild added tests-approved and removed tests-started labels Sep 13, 2018

cmsbuild added comparison-available and removed comparison-pending labels Sep 13, 2018

cmsbuild added reconstruction-approved and removed reconstruction-pending labels Sep 14, 2018

fabiocos reviewed Oct 1, 2018

View reviewed changes

cmsbuild added orp-approved and removed orp-pending labels Oct 3, 2018

cmsbuild merged commit fe0f9a9 into cms-sw:CMSSW_10_2_X Oct 3, 2018

peruzzim mentioned this pull request Dec 12, 2018

List of blocked features affecting reco/miniAOD defaults in 94X and 102X #25365

Closed

hqucms deleted the deep-boosted-jets-rebase-102X branch November 15, 2019 19:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[102X][Backport] DeepAK8 tagger integration #24505

[102X][Backport] DeepAK8 tagger integration #24505

hqucms commented Sep 11, 2018

cmsbuild commented Sep 12, 2018

jmduarte commented Sep 12, 2018

hqucms commented Sep 12, 2018

slava77 commented Sep 12, 2018

cmsbuild commented Sep 12, 2018 •

edited

hqucms commented Sep 13, 2018

cmsbuild commented Sep 13, 2018

cmsbuild commented Sep 13, 2018

cmsbuild commented Sep 13, 2018

slava77 commented Sep 14, 2018

hqucms commented Sep 14, 2018

slava77 commented Sep 14, 2018 via email

fabiocos Oct 1, 2018

slava77 Oct 1, 2018

fabiocos commented Oct 3, 2018

fabiocos commented Oct 3, 2018

[102X][Backport] DeepAK8 tagger integration #24505

[102X][Backport] DeepAK8 tagger integration #24505

Conversation

hqucms commented Sep 11, 2018

cmsbuild commented Sep 12, 2018

jmduarte commented Sep 12, 2018

hqucms commented Sep 12, 2018

slava77 commented Sep 12, 2018

cmsbuild commented Sep 12, 2018 • edited

hqucms commented Sep 13, 2018

cmsbuild commented Sep 13, 2018

cmsbuild commented Sep 13, 2018

cmsbuild commented Sep 13, 2018

slava77 commented Sep 14, 2018

hqucms commented Sep 14, 2018

slava77 commented Sep 14, 2018 via email

fabiocos Oct 1, 2018

Choose a reason for hiding this comment

slava77 Oct 1, 2018

Choose a reason for hiding this comment

fabiocos commented Oct 3, 2018

fabiocos commented Oct 3, 2018

cmsbuild commented Sep 12, 2018 •

edited