Introduce DeepDoubleX V2 #30016

andrzejnovak · 2020-05-28T14:36:19Z

This PR introduces V2 of DeepDoubleX. Required changes w.r.t. V1 of DeepDoubleX producer include adding a new group of variables, add more variables to existing groups and reordering some of them.

Input x-check in training FW and CMSSW - ✅.

Evaluation x-check - ✅

cmsbuild · 2020-05-28T14:36:47Z

The code-checks are being triggered in jenkins.

cmsbuild · 2020-05-28T14:43:23Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-30016/15708

This PR adds an extra 248KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File RecoBTag/ONNXRuntime/plugins/DeepDoubleXONNXJetTagsProducer.cc modified in PR(s): [RFC] Test clang-tidy --checks modernize-loop-convert #29935

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

code-format:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-30016/15708/code-format.patch
e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-30016/15708/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

andrzejnovak · 2020-05-28T18:27:08Z

@slava77 bump

perrotta

Not a real review, just pointing out a few thing that jumped on my eyes while scrolling the differences

RecoBTag/FeatureTools/interface/ChargedCandidateConverter.h

RecoBTag/FeatureTools/interface/NeutralCandidateConverter.h

RecoBTag/FeatureTools/plugins/DeepDoubleXTagInfoProducer.cc

RecoBTag/ONNXRuntime/plugins/DeepDoubleXONNXJetTagsProducer.cc

perrotta · 2020-06-08T08:23:35Z

RecoBTag/ONNXRuntime/plugins/DeepDoubleXONNXJetTagsProducer.cc

          ++idx;
        }
      }

      // run prediction
      outputs = globalCache()->run(input_names_, data_, output_names_, batch_size)[0];
+
+      // DEBUG: Dump inputs to file


Has this debug to remain in the producer?

I can gate it by a debug_ bool, but I would prefer to keep it here as it helps a lot with catching problems when syncing with the outside framework.

RecoBTag/ONNXRuntime/plugins/DeepDoubleXONNXJetTagsProducer.cc

perrotta · 2020-06-08T08:34:44Z

This is a draft PR for introducing V2 of DeepDoubleX. Required changes w.r.t. V1 of DeepDoubleX producer include adding a new group of variables, add more variables to existing groups and reordering some of them.

I would like to ask for feedback on how backcomp to V1 should be treated? Should there be a new producer? It seems like a possibly simpler choice than making all the differences configurable.

(model file will be factored out before this PR is ready)

What is the purpose of back compatibility with V1? Do you plan to use both V1 and V2 at the same time, or just allow finishing some analysis which already started with V1? That decision has to be taken in the the POG: please summarize here the result of the discussion in the POG.

If the two version must be kept available at the same time, at the first glance it does not seem to me too much complicated to factorize out the differences and make them configurable inside the same producer: code duplication doesn't seem neededd here, and normally better to be avoided.

andrzejnovak · 2020-06-08T11:45:11Z

@perrotta Thanks for getting back to me. This is supposed to supersede V1, I am asking from maintenance and backcomp point of view. From POG POV V2 could replace V1.

RecoBTag/ONNXRuntime/plugins/DeepDoubleXONNXJetTagsProducer.cc

cmsbuild · 2020-06-09T10:04:28Z

The code-checks are being triggered in jenkins.

cmsbuild · 2020-06-09T10:09:10Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-30016/15948

This PR adds an extra 252KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File DataFormats/BTauReco/src/classes_def.xml modified in PR(s): ONNX-based Higgs to bb Interaction Network Tagger #30072
- File RecoBTag/ONNXRuntime/plugins/DeepDoubleXONNXJetTagsProducer.cc modified in PR(s): [RFC] Test clang-tidy --checks modernize-loop-convert #29935

cmsbuild · 2020-06-09T10:09:37Z

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

It involves the following packages:

DataFormats/BTauReco
RecoBTag/Combined
RecoBTag/FeatureTools
RecoBTag/ONNXRuntime

@perrotta, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks.
@emilbols, @smoortga, @riga, @rovere, @JyothsnaKomaragiri, @mverzett, @hqucms, @ferencek, @andrzejnovak this is something you requested to watch as well.
@silviodonato, @dpiparo you are the release manager for this.

cms-bot commands are listed here

DataFormats/BTauReco/src/classes_def.xml

cmsbuild · 2020-06-09T14:10:25Z

The code-checks are being triggered in jenkins.

cmsbuild · 2020-06-30T17:06:34Z

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

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 36
DQMHistoTests: Total histograms compared: 2778915
DQMHistoTests: Total failures: 1
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2778864
DQMHistoTests: Total skipped: 50
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 35 files compared)
Checked 152 log files, 16 edm output root files, 36 DQM output files

andrzejnovak · 2020-07-01T19:22:00Z

@slava77 @perrotta Is anything missing?

slava77 · 2020-07-01T23:50:32Z

@slava77 @perrotta Is anything missing?

#30016 (comment)

andrzejnovak · 2020-07-02T07:33:00Z

@perrotta igprof results

Perf:
http://test-anovak.web.cern.ch/test-anovak/cgi-bin/igprof-navigator/new/igreport_perf_ticks
http://test-anovak.web.cern.ch/test-anovak/cgi-bin/igprof-navigator/old/igreport_perf_ticks

Mem:
http://test-anovak.web.cern.ch/test-anovak/cgi-bin/igprof-navigator/new/igreport_mem_live
http://test-anovak.web.cern.ch/test-anovak/cgi-bin/igprof-navigator/old/igreport_mem_live

@slava77 not this?

slava77 · 2020-07-02T12:05:23Z

@slava77 not this?

sorry, I meant that Andrea is not available this week.

perrotta · 2020-07-02T13:00:47Z

@andrzejnovak thank you for providing your performance results in #30016 (comment) and #30016 (comment). With which workflow were they obtained?

Looking at them, I see that both V1 and V2 behaves rather similarly for what timing and memory are concerned: timing seems well manageable, and also for the memory usage some 9 Mb MEM_LIVE footprint for each version seems affordable.

@slava77 please comment if you disagree; but given the measured performances I believe that both V1 and V2 could be run simultaneously, so that full studies can be performed on V2 before finally switching to it as default suggested version for analyses. This would require reverting the effect of the commit in which you removed V2 from the list of versions saved in miniAOD output.
It must be understood that such a contemporary presence of two versions/tunes of the same algo must be intended as temporary, and as soon as all studies and scale factors have been concluded and computed only the recommended version will remin available in the release.

slava77 · 2020-07-02T14:59:39Z

related to #30016 (comment)
and #30016 (comment) (taking just DeepDouble module names)

0.000017  pfDeepDoubleXTagInfos
0.000022  pfMassIndependentDeepDoubleBvLJetTags
0.000030  pfMassIndependentDeepDoubleBvLV2JetTags
0.000022  pfMassIndependentDeepDoubleCvBJetTags
0.000044  pfMassIndependentDeepDoubleCvBV2JetTags
0.000022  pfMassIndependentDeepDoubleCvLJetTags
0.000031  pfMassIndependentDeepDoubleCvLV2JetTags

I see in my recent (IB, not this PR) that in 136.888, using JetHT inputs, the times per event are typically around 1 ms/ev per JetTags modules, which is about a factor of 40 larger than the values above.
Even with a very fast machine possibly doing work a factor of 3-4 faster, I conclude that the timing posted above is more likely to be for events with no jets passing preselections.
Anyways, an addition of 3 ms from 3 extra modules seems acceptable, if I extrapolate from my reference.

andrzejnovak · 2020-07-02T15:11:16Z

Hmm, fair point. The sample processed certainly had some passing jets, but it's not majority. If the times get averaged it could be biased. Though both tests were on the same sample.

@perrotta The igprof results are from running the test_deep_doublex file.

I think actually think it's fine as is - not running v2 by default in mini. The values can be calculated when producing the validation tuples/nano. I think that will make for a cleaner switch even if it wouldn't be too expensive to keep both temporarily.

slava77 · 2020-07-02T15:45:19Z

@perrotta The igprof results are from running the test_deep_doublex file.

'file:72164088-CB67-E811-9D0D-008CFA197AC4.root', ?
If it's a local copy of what's a row above from GluGluHToCC_M125_13TeV_powheg_pythia8,
then at least from the name it sounds like there are to first order no AK8 jets with pt>120 GeV (do I recall the cut value correctly?) in these events.

andrzejnovak · 2020-07-03T14:33:58Z

@slava77 There are certainly some, but not many. I've rerun both on a dedicated higher pt H tagger training samples, which improves the stats a bit, though it is still far off having a passing jet in every event.

http://test-anovak.web.cern.ch/test-anovak/cgi-bin/igprof-navigator/DDX/new/igreport_perf_ticks
http://test-anovak.web.cern.ch/test-anovak/cgi-bin/igprof-navigator/DDX/old/igreport_perf_ticks

http://test-anovak.web.cern.ch/test-anovak/cgi-bin/igprof-navigator/DDX/new/igreport_mem_live
http://test-anovak.web.cern.ch/test-anovak/cgi-bin/igprof-navigator/DDX/old/igreport_mem_live

perrotta · 2020-07-04T16:10:54Z

+1

DeepDoubleX version 2 added as a configurable possibility in the code, but not switched on in the settings of the default workflows
The following external must also be merged to allow running V2: DDXV2 cms-data/RecoBTag-Combined#31
The timing and memory performance of V2 are comparable with the ones of current V1, therefore no issues are expected when the switch will be decided and implemented by BTV (final checks and corresponding scale factors still to be computed)
Since the developers agree on having only by now V1 in the release (see Introduce DeepDoubleX V2 #30016 (comment)), no changes are expected in reco outputs: jenkins tests pass and actually show no differences

silviodonato · 2020-07-07T18:43:28Z

merge

santocch · 2020-07-10T07:36:20Z

+1

cmsbuild · 2020-07-10T07:36:45Z

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.

cmsbuild added this to the CMSSW_11_2_X milestone May 28, 2020

cmsbuild added code-checks-pending comparison-pending orp-pending pending-signatures reconstruction-pending tests-pending labels May 28, 2020

cmsbuild added code-checks-rejected and removed code-checks-pending labels May 28, 2020

andrzejnovak changed the title ~~[WIP] Introduce DeepDoubleX V2 - feedback requested~~ Introduce DeepDoubleX V2 - feedback requested Jun 2, 2020

cmsbuild mentioned this pull request Jun 2, 2020

ONNX-based Higgs to bb Interaction Network Tagger #30072

Merged

perrotta reviewed Jun 8, 2020

View reviewed changes

slava77 reviewed Jun 8, 2020

View reviewed changes

RecoBTag/ONNXRuntime/plugins/DeepDoubleXONNXJetTagsProducer.cc Outdated Show resolved Hide resolved

cmsbuild added code-checks-pending and removed code-checks-rejected labels Jun 9, 2020

cmsbuild added code-checks-approved and removed code-checks-pending labels Jun 9, 2020

perrotta reviewed Jun 9, 2020

View reviewed changes

DataFormats/BTauReco/src/classes_def.xml Outdated Show resolved Hide resolved

cmsbuild added analysis-pending code-checks-pending and removed code-checks-approved labels Jun 9, 2020

cmsbuild added comparison-available and removed comparison-pending labels Jun 30, 2020

cmsbuild added reconstruction-approved and removed reconstruction-pending labels Jul 4, 2020

andrzejnovak mentioned this pull request Jul 5, 2020

[10_6_X] feat: Introduce DeepDoubleX V2 #30542

Merged

cmsbuild added orp-approved and removed orp-pending labels Jul 7, 2020

cmsbuild merged commit baa5e2f into cms-sw:master Jul 7, 2020

This was referenced Jul 8, 2020

Phase2-hgx252 Make the new algorithm for dd4hep #30593

Merged

Clean BuildFiles in reco plugins #30535

Merged

Use ONNXRuntime for ParticleNet inference #30599

Merged

cmsbuild added analysis-approved fully-signed and removed analysis-pending pending-signatures labels Jul 10, 2020

kpedro88 mentioned this pull request Aug 25, 2022

Fixes for DeepDoubleX #39184

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce DeepDoubleX V2 #30016

Introduce DeepDoubleX V2 #30016

andrzejnovak commented May 28, 2020 •

edited

cmsbuild commented May 28, 2020

cmsbuild commented May 28, 2020

andrzejnovak commented May 28, 2020 •

edited

perrotta left a comment

perrotta Jun 8, 2020

andrzejnovak Jun 9, 2020

perrotta commented Jun 8, 2020

andrzejnovak commented Jun 8, 2020

cmsbuild commented Jun 9, 2020

cmsbuild commented Jun 9, 2020

cmsbuild commented Jun 9, 2020

cmsbuild commented Jun 9, 2020

cmsbuild commented Jun 30, 2020

andrzejnovak commented Jul 1, 2020

slava77 commented Jul 1, 2020

andrzejnovak commented Jul 2, 2020

slava77 commented Jul 2, 2020

perrotta commented Jul 2, 2020

slava77 commented Jul 2, 2020

andrzejnovak commented Jul 2, 2020

slava77 commented Jul 2, 2020

andrzejnovak commented Jul 3, 2020

perrotta commented Jul 4, 2020

silviodonato commented Jul 7, 2020

santocch commented Jul 10, 2020

cmsbuild commented Jul 10, 2020

Introduce DeepDoubleX V2 #30016

Introduce DeepDoubleX V2 #30016

Conversation

andrzejnovak commented May 28, 2020 • edited

cmsbuild commented May 28, 2020

cmsbuild commented May 28, 2020

andrzejnovak commented May 28, 2020 • edited

perrotta left a comment

Choose a reason for hiding this comment

perrotta Jun 8, 2020

Choose a reason for hiding this comment

andrzejnovak Jun 9, 2020

Choose a reason for hiding this comment

perrotta commented Jun 8, 2020

andrzejnovak commented Jun 8, 2020

cmsbuild commented Jun 9, 2020

cmsbuild commented Jun 9, 2020

cmsbuild commented Jun 9, 2020

cmsbuild commented Jun 9, 2020

cmsbuild commented Jun 30, 2020

andrzejnovak commented Jul 1, 2020

slava77 commented Jul 1, 2020

andrzejnovak commented Jul 2, 2020

slava77 commented Jul 2, 2020

perrotta commented Jul 2, 2020

slava77 commented Jul 2, 2020

andrzejnovak commented Jul 2, 2020

slava77 commented Jul 2, 2020

andrzejnovak commented Jul 3, 2020

perrotta commented Jul 4, 2020

silviodonato commented Jul 7, 2020

santocch commented Jul 10, 2020

cmsbuild commented Jul 10, 2020

andrzejnovak commented May 28, 2020 •

edited

andrzejnovak commented May 28, 2020 •

edited