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

Fix Tracks related Pixel quantities at HLT DQM (backport) #40875

Merged
merged 5 commits into from
Mar 1, 2023

Conversation

arossi83
Copy link
Contributor

PR description:

Monitor of the tracks related Pixel quantities at HLT DQM never works and it was disabled at some point, with this PR we introduce the changes needed (tracks refitting) in order to be able to monitor Pixel Tracks quantities at HLT DQM.
In this PR we also update the collection to be used by the Standard DQM Pixel Residuals Monitor in order to use the refitted tracks.

PR validation:

runTheMatrix.sh -l 140.063
Manually run the Online HLT client at P5 with old stored DQM stream file

If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:

Backport of #40873

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 24, 2023

A new Pull Request was created by @arossi83 (Alessandro Rossi) for CMSSW_13_0_X.

It involves the following packages:

  • DQM/Integration (dqm)
  • DQM/SiPixelPhase1Track (dqm)
  • DQMOffline/Trigger (dqm)

@emanueleusai, @cmsbuild, @syuvivida, @rvenditti, @micsucmed, @pmandrik can you please review it and eventually sign? Thanks.
@fioriNTU, @arossi83, @trocino, @batinkov, @hdelanno, @battibass, @sroychow, @jhgoh, @missirol, @HuguesBrun, @Fedespring, @threus, @jandrea, @idebruyn, @mmusich, @rociovilar, @mtosi, @cericeci, @francescobrivio this is something you requested to watch as well.
@perrotta, @dpiparo, @rappoccio you are the release manager for this.

cms-bot commands are listed here

@mmusich
Copy link
Contributor

mmusich commented Feb 24, 2023

test parameters:

  • workflow = 140.063

@mmusich
Copy link
Contributor

mmusich commented Feb 24, 2023

please test

@mmusich
Copy link
Contributor

mmusich commented Feb 24, 2023

urgent

  • please @cms-sw/dqm-l2 consider this for integration in 13_0_0.

@mmusich
Copy link
Contributor

mmusich commented Feb 24, 2023

@arossi83 the unit test TestDQMOnlineClient-hlt_dqm_sourceclient is failing with

----- Begin Fatal Exception 24-Feb-2023 12:25:10 CET-----------------------
An exception of category 'ProductNotFound' occurred while
   [0] Processing  Event run: 355380 lumi: 19 event: 26716348 stream: 0
   [1] Running path 'hlt4vector'
   [2] Calling method for module SiPixelClusterShapeCacheProducer/'hltSiPixelClusterShapeCache'
Exception Message:
Principal::getByToken: Found zero products matching all criteria
Looking for type: edmNew::DetSetVector<SiPixelCluster>
Looking for module label: hltSiPixelClusters
Looking for productInstanceName: 

   Additional Info:
      [a] If you wish to continue processing events after a ProductNotFound exception,
add "SkipEvent = cms.untracked.vstring('ProductNotFound')" to the "options" PSet in the configuration.

----- End Fatal Exception -------------------------------------------------

this happens because the input file for the test /store/express/Run2022B/ExpressPhysics/FEVT/Express-v1/000/355/380/00000/2b3f408f-883a-4f64-8083-9e5786589069.root is a FEVT file from StreamExpress and as such it doesn't have the necessary HLT collections.
The test has been running successfully in IB and PR tests so far only because all the (DQM module) consumers in its path are apparently well protected in terms of missing inputs, while SiPixelClusterShapeCacheProducer is not.
I think the right move would be to change the input of this unit test to use a file that instead does contain the HLT-produced collections, since otherwise the test is not testing anything, but I am not sure if there's any readily available such file. A FEVTHLTALL file from /HLTMonitor/Run2022*Express-v*/FEVTHLTALL would serve the purpose but I cannot find any on disk.
@cms-sw/hlt-l2 FYI.

@missirol
Copy link
Contributor

Interesting.. I couldn't find any files from /HLTMonitor/Run2022*Express-v*/FEVTHLTALL either.

I could only find [1]; that might help for local tests, but I guess we should not rely on this non-standard path in CMSSW. Maybe, one quick workaround could be asking to copy this in a proper folder of the cmsbot cache (then CMSSW tests would see it). Not sure how to proceed.
[1]

/eos/cms/tier0/store/backfill/1/express/Tier0_REPLAY_2022/HLTMonitor/FEVTHLTALL/Express-v710/000/356/005/00000/0f207c66-8cda-4c3d-b4c8-3b063efc72e8.root

#+ hltSiPixelPhase1TrackClustersAnalyzer
hltSiPixelClusterShapeCache
+ hltSiPixelPhase1ClustersAnalyzer
+ refittedForPixelDQM
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this be

Suggested change
+ refittedForPixelDQM
+ hltrefittedForPixelDQM

?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, It should. Just push a commit to fix it.

@cmsbuild
Copy link
Contributor

-1

Failed Tests: UnitTests RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-cc786b/30879/summary.html
COMMIT: 5f46737
CMSSW: CMSSW_13_0_X_2023-02-23-2300/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/40875/30879/install.sh to create a dev area with all the needed externals and cmssw changes.

Unit Tests

I found errors in the following unit tests:

---> test TestDQMOnlineClient-hlt_dqm_sourceclient had ERRORS

RelVals-INPUT

  • 140.54140.54_RunPA2013/step2_RunPA2013.log

Comparison Summary

There are some workflows for which there are errors in the baseline:
22034.911 step 1
23234.911 step 1
The results for the comparisons for these workflows could be incomplete
This means most likely that the IB is having errors in the relvals.The error does NOT come from this pull request

Summary:

  • You potentially added 84 lines to the logs
  • Reco comparison results: 14 differences found in the comparisons
  • DQMHistoTests: Total files compared: 50
  • DQMHistoTests: Total histograms compared: 3608349
  • DQMHistoTests: Total failures: 7042
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3601285
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 20185.41 KiB( 49 files compared)
  • DQMHistoSizes: changed ( 11634.0,... ): 3364.235 KiB HLT/Pixel
  • Checked 217 log files, 167 edm output root files, 50 DQM output files
  • TriggerResults: no differences found

@mmusich
Copy link
Contributor

mmusich commented Feb 24, 2023

so this patch:

diff --git a/DQM/Integration/python/clients/hlt_dqm_sourceclient-live_cfg.py b/DQM/Integration/python/clients/hlt_dqm_sourceclient-live_cfg.py
index 3e6b28ecb1f..cc04e270abf 100644
--- a/DQM/Integration/python/clients/hlt_dqm_sourceclient-live_cfg.py
+++ b/DQM/Integration/python/clients/hlt_dqm_sourceclient-live_cfg.py
@@ -46,8 +46,8 @@ process.ClusterShapeHitFilterESProducer = cms.ESProducer( "ClusterShapeHitFilter
 )
 #SiStrip Local Reco
 process.load("CalibTracker.SiStripCommon.TkDetMapESProducer_cfi")
-#SiPixelTemplate
-process.load("CalibTracker.SiPixelESProducers.SiPixelTemplateDBObjectESProducer_cfi")
+#Track refitters
+process.load("RecoTracker.TrackProducer.TrackRefitters_cff")
 
 #---- for P5 (online) DB access
 process.load("DQM.Integration.config.FrontierCondition_GT_cfi")
diff --git a/DQMOffline/Trigger/python/SiPixel_OfflineMonitoring_cff.py b/DQMOffline/Trigger/python/SiPixel_OfflineMonitoring_cff.py
index 37714b63571..94184f2792e 100644
--- a/DQMOffline/Trigger/python/SiPixel_OfflineMonitoring_cff.py
+++ b/DQMOffline/Trigger/python/SiPixel_OfflineMonitoring_cff.py
@@ -6,11 +6,12 @@ from RecoPixelVertexing.PixelLowPtUtilities.siPixelClusterShapeCache_cfi import
 from DQM.SiPixelMonitorTrack.RefitterForPixelDQM import *
 
 hltSiPixelClusterShapeCache = siPixelClusterShapeCache.clone(src = 'hltSiPixelClusters')
-hltrefittedForPixelDQM = refittedForPixelDQM.clone(src ='hltMergedTracks')
+hltrefittedForPixelDQM = refittedForPixelDQM.clone(src ='hltMergedTracks',
+                                                   TTRHBuilder = cms.string('WithTrackAngle')) # no templates at HLT
 
 sipixelMonitorHLTsequence = cms.Sequence(
     hltSiPixelClusterShapeCache
     + hltSiPixelPhase1ClustersAnalyzer
-    + refittedForPixelDQM
+    + hltrefittedForPixelDQM
     + hltSiPixelPhase1TrackClustersAnalyzer
 )    
diff --git a/RecoPixelVertexing/PixelLowPtUtilities/plugins/SiPixelClusterShapeCacheProducer.cc b/RecoPixelVertexing/PixelLowPtUtilities/plugins/SiPixelClusterShapeCacheProducer.cc
index dba9894b23c..6322f9cc868 100644
--- a/RecoPixelVertexing/PixelLowPtUtilities/plugins/SiPixelClusterShapeCacheProducer.cc
+++ b/RecoPixelVertexing/PixelLowPtUtilities/plugins/SiPixelClusterShapeCacheProducer.cc
@@ -61,6 +61,13 @@ void SiPixelClusterShapeCacheProducer::produce(edm::StreamID, edm::Event& iEvent
   edm::Handle<InputCollection> input;
   iEvent.getByToken(token_, input);
 
+  if (!input.isValid()) {
+    edm::LogError("siPixelClusterShapeCache") << "input pixel cluster collection is not valid!";
+    auto output = std::make_unique<SiPixelClusterShapeCache>();
+    iEvent.put(std::move(output));
+    return;
+  }
+
   const auto& geom = &iSetup.getData(geomToken_);
 
   auto output = std::make_unique<SiPixelClusterShapeCache>(input);

technically makes the test scram b runtests_TestDQMOnlineClient-hlt_dqm_sourceclient use-ibeos run fine, even though it's not actually testing anything (see #40875 (comment)).
I suppose that this would also fix the problem with 140.54, though we probably want to run some sequence that doesn't run the HLT monitoring instead, since there's no re-HLT in it.

@cmsbuild
Copy link
Contributor

Pull request #40875 was updated. @emanueleusai, @cmsbuild, @syuvivida, @rvenditti, @micsucmed, @pmandrik can you please check and sign again.

@cmsbuild
Copy link
Contributor

Pull request #40875 was updated. @micsucmed, @emanueleusai, @clacaputo, @cmsbuild, @syuvivida, @pmandrik, @mandrenguyen, @rvenditti can you please check and sign again.

@mmusich
Copy link
Contributor

mmusich commented Feb 28, 2023

@cmsbuild, please test

@cmsbuild
Copy link
Contributor

-1

Failed Tests: RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-cc786b/30940/summary.html
COMMIT: d943f6f
CMSSW: CMSSW_13_0_X_2023-02-27-2300/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/40875/30940/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals-INPUT

  • 20834.020834.0_TTbar_14TeV+2026D88/step2_TTbar_14TeV+2026D88.log
  • 20834.2120834.21_TTbar_14TeV+2026D88_ProdLike/step2_TTbar_14TeV+2026D88_ProdLike.log
  • 20834.10320834.103_TTbar_14TeV+2026D88Aging3000/step2_TTbar_14TeV+2026D88Aging3000.log
Expand to see more relval errors ...

Comparison Summary

Summary:

  • You potentially added 7 lines to the logs
  • Reco comparison results: 14 differences found in the comparisons
  • DQMHistoTests: Total files compared: 50
  • DQMHistoTests: Total histograms compared: 3647977
  • DQMHistoTests: Total failures: 7043
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3640912
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 23549.645 KiB( 49 files compared)
  • DQMHistoSizes: changed ( 140.063,... ): 3364.235 KiB HLT/Pixel
  • Checked 217 log files, 167 edm output root files, 50 DQM output files
  • TriggerResults: no differences found

@arossi83
Copy link
Contributor Author

Test failing because of #40889. Backport of #40873 with the addition of #40891 (see last commit here) (both PR already merged in master)

@mmusich
Copy link
Contributor

mmusich commented Feb 28, 2023

@emanueleusai @mandrenguyen @perrotta w.r.t last signature this PR just removed cms type specification in one configuration file. Can you consider this for inclusion in 13_0_0?

@mandrenguyen
Copy link
Contributor

+reconstruction
Disregarding relval input failure

@emanueleusai
Copy link
Member

+1

  • differences in DQM understood (see master PR)

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 1, 2023

This pull request is fully signed and it will be integrated in one of the next CMSSW_13_0_X IBs (but tests are reportedly failing) and once validation in the development release cycle CMSSW_13_1_X is complete. This pull request will now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @rappoccio (and backports should be raised in the release meeting by the corresponding L2)

@perrotta
Copy link
Contributor

perrotta commented Mar 1, 2023

+1

@perrotta
Copy link
Contributor

perrotta commented Mar 1, 2023

merge

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.

7 participants