DeepTau - Do not call TF inference with empty grid #44455

valsdav · 2024-03-18T21:45:20Z

PR description:

Fix DeepTau issue #44333

[2] Calling method for module DeepTauId/'hltHpsPFTauDeepTauProducerForVBFIsoTau'
Exception Message:
error while running session: INVALID_ARGUMENT: Incompatible shapes: [0,1,1,64] vs. [154]

The number of valid_grid_cells here is 0 for some events and this is creating a TF::Tensor with shape [0, 1, 1, N].

When this input is passed to a TF model executed on a CPU without AVX512F AVX512_VNNI, the model is executed and returns an empty output without complaining. When AVX512F AVX512_VNNI instructions are present, the TF executor complains.

This PR avoids calling the TF inference if empty inputs are detected.

The grid should never be empty. The issue will be followed up with Tau experts (enter here issue number..)

PR validation:

Works on the issue reproducer from #44333 (comment)

lxplus901 for AVX512
Pick event:

edmCopyPickMerge outputFile=pickevents.root eventsToProcess=367131:196942831 inputFiles=/store/data/Run2023C/DisplacedJet/RAW/v1/000/367/131/00000/9f3f571f-6dc9-4bda-a68b-5d1b9a5fc3ac.root

Run:

cmsDriver.py step2 --conditions auto:run3_hlt_relval --data --datatier FEVTDEBUGHLT --era Run3_2023 --eventcontent FEVTDEBUGHLT --filein file:pickevents.root --fileout file:step2.root --nStreams 4 --nThreads 8 --number -1 --process reHLT --python_filename step_2_cfg.py --step L1REPACK:Full,HLT:@relval2024 --accelerators cpu

cmsbuild · 2024-03-18T21:45:47Z

cms-bot internal usage

cmsbuild · 2024-03-18T21:54:29Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44455/39530

This PR adds an extra 28KB to repository

cmsbuild · 2024-03-18T21:54:47Z

A new Pull Request was created by @valsdav for master.

It involves the following packages:

RecoTauTag/HLTProducers (hlt)
RecoTauTag/RecoTau (reconstruction)

@Martin-Grunewald, @mandrenguyen, @mmusich, @cmsbuild, @jfernan2 can you please review it and eventually sign? Thanks.
@silviodonato, @azotz, @missirol, @mbluj this is something you requested to watch as well.
@sextonkennedy, @antoniovilela, @rappoccio you are the release manager for this.

cms-bot commands are listed here

valsdav · 2024-03-18T21:55:54Z

assign ml
type tau

cmsbuild · 2024-03-18T21:56:11Z

New categories assigned: ml

@valsdav,@wpmccormack you have been requested to review this Pull request/Issue and eventually sign? Thanks

valsdav · 2024-03-18T22:12:56Z

please test

cmsbuild · 2024-03-19T01:40:54Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-e9b7b8/38241/summary.html
COMMIT: 6bb9279
CMSSW: CMSSW_14_1_X_2024-03-18-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/44455/38241/install.sh to create a dev area with all the needed externals and cmssw changes.

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:

You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-e9b7b8/38241/git-recent-commits.json
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-e9b7b8/38241/git-merge-result

Comparison Summary

Summary:

You potentially removed 105 lines from the logs
Reco comparison results: 59 differences found in the comparisons
DQMHistoTests: Total files compared: 48
DQMHistoTests: Total histograms compared: 3297369
DQMHistoTests: Total failures: 1355
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3295994
DQMHistoTests: Total skipped: 20
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
Checked 202 log files, 165 edm output root files, 48 DQM output files
TriggerResults: no differences found

jfernan2 · 2024-03-19T08:10:06Z

+1

mmusich · 2024-03-19T08:12:10Z

RecoTauTag/HLTProducers/src/L2TauTagNNProducer.cc

@@ -732,7 +732,10 @@ void L2TauNNProducer::fillPatatracks(tensorflow::Tensor& cellGridMatrix,

 std::vector<float> L2TauNNProducer::getTauScore(const tensorflow::Tensor& cellGridMatrix) {
  std::vector<tensorflow::Tensor> pred_tensor;
-  tensorflow::run(L2cacheData_->session, {{inputTensorName_, cellGridMatrix}}, {outputTensorName_}, &pred_tensor);
+  // Check for empty input
+  if (cellGridMatrix.NumElements() != 0) {


does cellGridMatrix.NumElements() == 0 guarantee that nTau is also 0, such that accessing pred_tensor[0] doesn't result in undefined behaviour?

It is guaranteed because nTaus = cellGridMatrix.shape().dim_size(0), which is the same check, but I agree that it can be improved by wrapping all the snippet around a if (nTaus > 0).
If you wish I can rework it quickly

If you wish I can rework it quickly

perhaps leave a comment, if it's guaranteed let's not add more if branches.
Thank you.

@valsdav will you apply the suggestion above?

yes, I'm about to push a commit with improvements 👍

cmsbuild · 2024-03-19T17:29:48Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-e9b7b8/38263/summary.html
COMMIT: ba3ec51
CMSSW: CMSSW_14_1_X_2024-03-19-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/44455/38263/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially removed 9 lines from the logs
Reco comparison results: 52 differences found in the comparisons
DQMHistoTests: Total files compared: 48
DQMHistoTests: Total histograms compared: 3297369
DQMHistoTests: Total failures: 1352
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3295997
DQMHistoTests: Total skipped: 20
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
Checked 202 log files, 165 edm output root files, 48 DQM output files
TriggerResults: no differences found

jfernan2 · 2024-03-19T17:39:22Z

+1

mmusich · 2024-03-19T20:49:20Z

+hlt

valsdav · 2024-03-19T23:09:59Z

+ml

cmsbuild · 2024-03-19T23:10:18Z

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. @rappoccio, @antoniovilela, @sextonkennedy (and backports should be raised in the release meeting by the corresponding L2)

antoniovilela · 2024-03-20T08:45:14Z

+1

DeepTau - Do not call TF inference with empty grid

6bb9279

cmsbuild added this to the CMSSW_14_1_X milestone Mar 18, 2024

cmsbuild added reconstruction-pending hlt-pending pending-signatures tests-pending orp-pending code-checks-pending labels Mar 18, 2024

valsdav mentioned this pull request Mar 18, 2024

[backport] DeepTau - Do not call TF inference with empty grid #44456

Merged

cmsbuild added code-checks-approved and removed code-checks-pending labels Mar 18, 2024

cmsbuild added the ml-pending label Mar 18, 2024

cmsbuild added tests-started and removed tests-pending labels Mar 18, 2024

cmsbuild added tests-approved and removed tests-started labels Mar 19, 2024

mmusich mentioned this pull request Mar 19, 2024

DeepTauId failures in RelVals (Incompatible shapes) #44333

Closed

cmsbuild added reconstruction-approved and removed reconstruction-pending labels Mar 19, 2024

mmusich reviewed Mar 19, 2024

View reviewed changes

Skipping input preparation for empty grid

ba3ec51

cmsbuild removed the reconstruction-approved label Mar 19, 2024

cmsbuild added tests-approved and removed tests-started labels Mar 19, 2024

cmsbuild added reconstruction-approved and removed reconstruction-pending labels Mar 19, 2024

cmsbuild added hlt-approved and removed hlt-pending labels Mar 19, 2024

cmsbuild added fully-signed ml-approved and removed pending-signatures ml-pending labels Mar 19, 2024

cmsbuild added orp-approved and removed orp-pending labels Mar 20, 2024

cmsbuild merged commit 754f2b8 into cms-sw:master Mar 20, 2024
11 checks passed

mmusich mentioned this pull request Mar 20, 2024

Exception on HLT_DoubleMediumDeepTauPFTauHPS35_eta2p1 Phase-2 workflow #42862

Closed

valsdav mentioned this pull request Mar 21, 2024

Empty grid for DeepTauId #44501

Open

This was referenced Mar 22, 2024

[CPP20][ALCA] Replace some enums with constexpr ints #44509

Merged

[CPP20][GEOMETRY] Replace some enums with constexpr ints #44514

Merged

This was referenced Jun 4, 2024

HLT farm crash in run 381543 #45136

Closed

L2TauTagNNProducerAlpaka: do not call TF inference with empty grid #45147

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DeepTau - Do not call TF inference with empty grid #44455

DeepTau - Do not call TF inference with empty grid #44455

valsdav commented Mar 18, 2024

cmsbuild commented Mar 18, 2024 •

edited

Loading

cmsbuild commented Mar 18, 2024

cmsbuild commented Mar 18, 2024

valsdav commented Mar 18, 2024

cmsbuild commented Mar 18, 2024

valsdav commented Mar 18, 2024

cmsbuild commented Mar 19, 2024

jfernan2 commented Mar 19, 2024

mmusich Mar 19, 2024

valsdav Mar 19, 2024

mmusich Mar 19, 2024

mmusich Mar 19, 2024

valsdav Mar 19, 2024

cmsbuild commented Mar 19, 2024

jfernan2 commented Mar 19, 2024

mmusich commented Mar 19, 2024

valsdav commented Mar 19, 2024

cmsbuild commented Mar 19, 2024

antoniovilela commented Mar 20, 2024

DeepTau - Do not call TF inference with empty grid #44455

DeepTau - Do not call TF inference with empty grid #44455

Conversation

valsdav commented Mar 18, 2024

PR description:

PR validation:

cmsbuild commented Mar 18, 2024 • edited Loading

cmsbuild commented Mar 18, 2024

cmsbuild commented Mar 18, 2024

valsdav commented Mar 18, 2024

cmsbuild commented Mar 18, 2024

valsdav commented Mar 18, 2024

cmsbuild commented Mar 19, 2024

Comparison Summary

jfernan2 commented Mar 19, 2024

mmusich Mar 19, 2024

Choose a reason for hiding this comment

valsdav Mar 19, 2024

Choose a reason for hiding this comment

mmusich Mar 19, 2024

Choose a reason for hiding this comment

mmusich Mar 19, 2024

Choose a reason for hiding this comment

valsdav Mar 19, 2024

Choose a reason for hiding this comment

cmsbuild commented Mar 19, 2024

Comparison Summary

jfernan2 commented Mar 19, 2024

mmusich commented Mar 19, 2024

valsdav commented Mar 19, 2024

cmsbuild commented Mar 19, 2024

antoniovilela commented Mar 20, 2024

cmsbuild commented Mar 18, 2024 •

edited

Loading