Changing tensorflow implementation in RecoHGCal/TICL #36724

dr-stringfellow · 2022-01-17T17:26:54Z

PR description:

The tensorflow model used to regress energy and ID of TICL objects ("tracksters") has been moved from an ED-based implementation to an ES setup. All plugins (CA, CLUE3D, FastJet, ...) will now access the same cached model, instead of caching one model for each plugin as was the case in the EDProducer setup.

This is now in line with the RecoTracker tensorflow implementation. In this first step, we are pointing also to their tensorflow producer. Eventually, in a pull request that would follow soon, this should be harmonised and moved to a central, neutral place that both RecoTracker and RecoHGCal use.

PR validation:

It saves about 5% in memory on upgrade workflow
39034.202:

https://bmaier.web.cern.ch/bmaier/view.php?dir=figs/hgcal/ticltf/new

(The more plugins are run in the same job, the higher the savings.)

… setup

cmsbuild · 2022-01-17T17:36:38Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36724/27825

This PR adds an extra 80KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File RecoHGCal/TICL/plugins/PatternRecognitionbyCA.cc modified in PR(s): Pattern Recognition validation for HGCal Tracksters #36712

cmsbuild · 2022-01-17T17:37:01Z

A new Pull Request was created by @dr-stringfellow for master.

It involves the following packages:

RecoHGCal/TICL (upgrade, reconstruction)

@clacaputo, @cmsbuild, @AdrianoDee, @srimanob, @slava77, @jpata can you please review it and eventually sign? Thanks.
@felicepantaleo, @rovere, @apsallid, @sobhatta, @lecriste, @hatakeyamak, @ebrondol this is something you requested to watch as well.
@perrotta, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

rovere · 2022-01-17T18:03:37Z

Thanks a lot, @dr-stringfellow for this nice piece of work!

rovere · 2022-01-17T18:03:52Z

@cmsbuild please test

cmsbuild · 2022-01-17T21:41:42Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-95bd10/21763/summary.html
COMMIT: 2dd2348
CMSSW: CMSSW_12_3_X_2022-01-17-1100/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/36724/21763/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 1 differences found in the comparisons
DQMHistoTests: Total files compared: 43
DQMHistoTests: Total histograms compared: 3464734
DQMHistoTests: Total failures: 0
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3464712
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 42 files compared)
Checked 181 log files, 42 edm output root files, 43 DQM output files
TriggerResults: no differences found

cmsbuild · 2022-01-19T15:27:44Z

-1

Failed Tests: RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-95bd10/21806/summary.html
COMMIT: 2dd2348
CMSSW: CMSSW_12_3_X_2022-01-18-2300/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/36724/21806/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals-INPUT

4.764.76_ZMuSkim2012D+ZMuSkim2012D+HLTDSKIM2+RECODR1reHLT2+HARVESTDR1reHLT/step2_ZMuSkim2012D+ZMuSkim2012D+HLTDSKIM2+RECODR1reHLT2+HARVESTDR1reHLT.log

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 2 differences found in the comparisons
DQMHistoTests: Total files compared: 43
DQMHistoTests: Total histograms compared: 3464860
DQMHistoTests: Total failures: 5
DQMHistoTests: Total nulls: 1
DQMHistoTests: Total successes: 3464832
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.004 KiB( 42 files compared)
DQMHistoSizes: changed ( 312.0 ): 0.004 KiB MessageLogger/Warnings
Checked 181 log files, 42 edm output root files, 43 DQM output files
TriggerResults: no differences found

jpata · 2022-01-24T10:08:49Z

Note that the static checker complains about the const_cast (also an issue for the trackDNN), related to #32529. It's not something that can be fixed in this PR, but to be kept in mind.

4 warnings generated.
/data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_3_X_2022-01-18-2300/src/RecoHGCal/TICL/plugins/PatternRecognitionbyFastJet.cc:272:19: warning: const_cast was used, this may result in thread-unsafe code [threadsafety.ConstCast]
  tensorflow::run(const_cast<tensorflow::Session *>(eidSession), inputList, outputNames, &outputs);
                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
/data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_3_X_2022-01-18-2300/src/RecoHGCal/TICL/plugins/PatternRecognitionbyCA.cc:439:19: warning: const_cast was used, this may result in thread-unsafe code [threadsafety.ConstCast]
  tensorflow::run(const_cast<tensorflow::Session *>(eidSession), inputList, outputNames, &outputs);
                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
/data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_3_X_2022-01-18-2300/src/RecoHGCal/TICL/plugins/TrackstersMergeProducer.cc:662:19: warning: const_cast was used, this may result in thread-unsafe code [threadsafety.ConstCast]
  tensorflow::run(const_cast<tensorflow::Session *>(eidSession), inputList, outputNames, &outputs);
                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
/data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_3_X_2022-01-18-2300/src/RecoHGCal/TICL/plugins/PatternRecognitionbyCLUE3D.cc:403:19: warning: const_cast was used, this may result in thread-unsafe code [threadsafety.ConstCast]
  tensorflow::run(const_cast<tensorflow::Session *>(eidSession), inputList, outputNames, &outputs);
                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

I agree that in a follow-up (not necessarily by the HGCAL team), the TfGraphDefProducer stuff should be moved out of RecoTracker to a common area.

jpata · 2022-01-24T10:12:07Z

For posterity, can you comment on what settings were used to make this plot (besides the worfklow 39034.202):

jpata · 2022-01-24T15:34:08Z

enable profiling

jpata · 2022-01-25T08:18:50Z

@cmsbuild please test

dr-stringfellow · 2022-01-25T09:11:54Z

For posterity, can you comment on what settings were used to make this plot (besides the worfklow 39034.202):

The following settings were used:

cmsRun step3 -s RAW2DIGI,L1Reco,RECO,RECOSIM,PAT,VALIDATION:@phase2Validation+@miniAODValidation,DQM:@phase2+@miniAODDQM --conditions auto:phase2_realistic_T27 --datatier GEN-SIM-RECO,MINIAODSIM,DQMIO -n 30 --eventcontent FEVTDEBUGHLT,MINIAODSIM,DQM --geometry Extended2026D87 --era Phase2C11I13T27M9 --procModifiers fastJetTICL,clue3D --nThreads 1 --customise Validation/Performance/TimeMemoryInfo.py

cmsbuild · 2022-01-25T15:54:39Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-95bd10/21972/summary.html
COMMIT: 2dd2348
CMSSW: CMSSW_12_3_X_2022-01-24-2300/slc7_amd64_gcc10
Additional Tests: PROFILING
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/36724/21972/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 1 differences found in the comparisons
DQMHistoTests: Total files compared: 43
DQMHistoTests: Total histograms compared: 3449324
DQMHistoTests: Total failures: 0
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3449302
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 42 files compared)
Checked 181 log files, 42 edm output root files, 43 DQM output files
TriggerResults: no differences found

jpata · 2022-01-26T09:51:57Z

https://cmssdt.cern.ch/SDT/cgi-bin/igprof-navigator/CMSSW_12_3_X_2022-01-24-2300/slc7_amd64_gcc10/profiling/23434.21/igprofMEM_step3.19

https://cmssdt.cern.ch/SDT/cgi-bin/igprof-navigator/CMSSW_12_3_X_2022-01-24-2300/slc7_amd64_gcc10/profiling/23434.21/PR-95bd10/21972/igprofMEM_step3.19

we also see the improvement in the bot igprof job.

jpata · 2022-01-26T10:37:33Z

+reconstruction

technical, improves the memory handling of the TICL TF models by loading them from the EventSetup
no reco changes expected/observed

AdrianoDee · 2022-01-26T10:41:24Z

+upgrade
Technical, improving memory for upgrade wfs including TICL.

cmsbuild · 2022-01-26T10:41:41Z

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 now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

perrotta · 2022-01-26T14:39:23Z

RecoHGCal/TICL/plugins/PatternRecognitionbyCA.cc

@@ -446,7 +436,7 @@ void PatternRecognitionbyCA<TILES>::energyRegressionAndID(const std::vector<reco
  }

  // run the inference (7)
-  tensorflow::run(eidSession_, inputList, outputNames, &outputs);
+  tensorflow::run(const_cast<tensorflow::Session *>(eidSession), inputList, outputNames, &outputs);


eidSession is already a const tensorflow::Session *: why do you need the const_cast?

this is removing the const (OK since tensorflow::run is known to be thread-safe), to fit the the tensorflow interface.
related to #35541

perrotta · 2022-01-26T14:39:35Z

RecoHGCal/TICL/plugins/PatternRecognitionbyCLUE3D.cc

@@ -410,7 +400,7 @@ void PatternRecognitionbyCLUE3D<TILES>::energyRegressionAndID(const std::vector<
  }

  // run the inference (7)
-  tensorflow::run(eidSession_, inputList, outputNames, &outputs);
+  tensorflow::run(const_cast<tensorflow::Session *>(eidSession), inputList, outputNames, &outputs);


eidSession is already a const tensorflow::Session *: why do you need the const_cast?

perrotta · 2022-01-26T14:39:46Z

RecoHGCal/TICL/plugins/PatternRecognitionbyFastJet.cc

@@ -278,7 +269,7 @@ void PatternRecognitionbyFastJet<TILES>::energyRegressionAndID(const std::vector
  }

  // run the inference (7)
-  tensorflow::run(eidSession_, inputList, outputNames, &outputs);
+  tensorflow::run(const_cast<tensorflow::Session *>(eidSession), inputList, outputNames, &outputs);


eidSession is already a const tensorflow::Session *: why do you need the const_cast?

perrotta · 2022-01-26T14:39:58Z

RecoHGCal/TICL/plugins/TrackstersMergeProducer.cc

@@ -655,7 +659,7 @@ void TrackstersMergeProducer::energyRegressionAndID(const std::vector<reco::Calo
  }

  // run the inference (7)
-  tensorflow::run(eidSession_, inputList, outputNames, &outputs);
+  tensorflow::run(const_cast<tensorflow::Session *>(eidSession), inputList, outputNames, &outputs);


eidSession is already a const tensorflow::Session *: why do you need the const_cast?

perrotta · 2022-01-27T17:02:43Z

+1

dr-stringfellow added 12 commits January 17, 2022 06:53

Linking RecoTracker and defining tfGraphDefProducer for TICL

05bec75

Linking RecoTracker and defining tfGraphDefProducer for TICL

64a321d

Stripping cache from non-plugin-specific files and adding tfSession

e89d832

Stripping cache from non-plugin-specific files and adding tfSession

c2f38b7

Stripping cache from non-plugin-specific files and adding tfSession

3d003a8

Stripping cache from non-plugin-specific files and adding tfSession

8fd51d6

Updating Cellular Automaton plugin

c53e01b

Updating CLUE3D plugin to run with ES tensorflow setup

7ba045b

Updating FastJet pattern recognition plugin to run with ES tensorflow…

5cbdb64

… setup

Reordering arguments in TrackstersMergeProducer

eeb82d2

Updating FastJet pattern recognition plugin to run with ES tensorflow…

522c58e

… setup

Obeying to code-checks and code-format

2dd2348

cmsbuild added this to the CMSSW_12_3_X milestone Jan 17, 2022

cmsbuild added code-checks-pending orp-pending pending-signatures reconstruction-pending tests-pending upgrade-pending labels Jan 17, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels Jan 17, 2022

cmsbuild added tests-started and removed tests-pending labels Jan 17, 2022

cmsbuild added tests-approved and removed tests-started labels Jan 17, 2022

cmsbuild added the tests-started label Jan 19, 2022

cmsbuild added tests-rejected and removed tests-started labels Jan 19, 2022

cmsbuild added tests-started and removed tests-rejected labels Jan 25, 2022

cmsbuild added tests-approved and removed tests-started labels Jan 25, 2022

cmsbuild added reconstruction-approved and removed reconstruction-pending labels Jan 26, 2022

cmsbuild added fully-signed upgrade-approved and removed pending-signatures upgrade-pending labels Jan 26, 2022

perrotta reviewed Jan 26, 2022

View reviewed changes

cmsbuild added orp-approved and removed orp-pending labels Jan 27, 2022

cmsbuild merged commit ac0a8b7 into cms-sw:master Jan 27, 2022

cmsbuild mentioned this pull request Jan 31, 2022

[Aarch64] Build with -march=armv8-a cms-sw/cmsdist#7593

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Changing tensorflow implementation in RecoHGCal/TICL #36724

Changing tensorflow implementation in RecoHGCal/TICL #36724

dr-stringfellow commented Jan 17, 2022

cmsbuild commented Jan 17, 2022

cmsbuild commented Jan 17, 2022

rovere commented Jan 17, 2022

rovere commented Jan 17, 2022

cmsbuild commented Jan 17, 2022

cmsbuild commented Jan 19, 2022

jpata commented Jan 24, 2022 •

edited

Loading

jpata commented Jan 24, 2022

jpata commented Jan 24, 2022

jpata commented Jan 25, 2022

dr-stringfellow commented Jan 25, 2022 •

edited

Loading

cmsbuild commented Jan 25, 2022

jpata commented Jan 26, 2022

jpata commented Jan 26, 2022

AdrianoDee commented Jan 26, 2022

cmsbuild commented Jan 26, 2022

perrotta Jan 26, 2022

jpata Jan 27, 2022 •

edited

Loading

perrotta Jan 26, 2022

perrotta Jan 26, 2022

perrotta Jan 26, 2022

perrotta commented Jan 27, 2022

Changing tensorflow implementation in RecoHGCal/TICL #36724

Changing tensorflow implementation in RecoHGCal/TICL #36724

Conversation

dr-stringfellow commented Jan 17, 2022

PR description:

PR validation:

cmsbuild commented Jan 17, 2022

cmsbuild commented Jan 17, 2022

rovere commented Jan 17, 2022

rovere commented Jan 17, 2022

cmsbuild commented Jan 17, 2022

Comparison Summary

cmsbuild commented Jan 19, 2022

RelVals-INPUT

Comparison Summary

jpata commented Jan 24, 2022 • edited Loading

jpata commented Jan 24, 2022

jpata commented Jan 24, 2022

jpata commented Jan 25, 2022

dr-stringfellow commented Jan 25, 2022 • edited Loading

cmsbuild commented Jan 25, 2022

Comparison Summary

jpata commented Jan 26, 2022

jpata commented Jan 26, 2022

AdrianoDee commented Jan 26, 2022

cmsbuild commented Jan 26, 2022

perrotta Jan 26, 2022

Choose a reason for hiding this comment

jpata Jan 27, 2022 • edited Loading

Choose a reason for hiding this comment

perrotta Jan 26, 2022

Choose a reason for hiding this comment

perrotta Jan 26, 2022

Choose a reason for hiding this comment

perrotta Jan 26, 2022

Choose a reason for hiding this comment

perrotta commented Jan 27, 2022

jpata commented Jan 24, 2022 •

edited

Loading

dr-stringfellow commented Jan 25, 2022 •

edited

Loading

jpata Jan 27, 2022 •

edited

Loading