SiStripClusters2ApproxClusters: run peakFilter only if cluster is usable

[mmusich]

resolves #42131
resolves #42162

PR description:

The goal of this PR is to solve the issues #42131 and #42132.
Those issues are caused because there is a condition (SiStripNoises) which is read off its domain from this call:

cmssw/RecoTracker/PixelLowPtUtilities/interface/SlidingPeakFinder.h

Line 72 in fe01c0e

cut_ = std::min<float>(seedCutMIPs * mip, seedCutSN * noiseObj_->getNoise(firstStrip + 1, noises_));

when it looks for the noise of strip n. 768 for Detid 369120277.
Having established at #42162 (comment) that for this detId (and similar modules), the strip range is from 0 to 767 (also independently, see mmusich@c1e1ca0), thus excluding possible issues with conditions, and following the line of thought that (see #42162 (comment)) a very similar code was already used in

cmssw/RecoTracker/PixelLowPtUtilities/src/StripSubClusterShapeTrajectoryFilter.cc

Line 84 in c2e865a

struct PeakFinderTest {

which I think it has been used since ever for the Strip seeded iterations, it begs the question on why this was not spotted before in StripSubClusterShapeTrajectoryFilter.
I propose in this PR to follow (almost) the same prescription used in that file before running the filter, namely using the output of ClusterShapeHitFilter::getSizes

cmssw/RecoTracker/PixelLowPtUtilities/src/StripSubClusterShapeTrajectoryFilter.cc

Lines 258 to 259 in c2e865a

	if (!usable)
	return true;

and the check on the measured (and predicted) cluster widths

cmssw/RecoTracker/PixelLowPtUtilities/src/StripSubClusterShapeTrajectoryFilter.cc

Lines 265 to 267 in c2e865a

	if (std::abs(hitPredPos) < 1.5f && hitStrips <= 2) {
	return true;
	}

Since ClusterShapeHitFilter::getSizes is not called with the measured trajectory direction, but rather with an approximated direction (the line joining the BeamSpot to the cluster) I enlarge the window to encompass 2 cluster pitches rather than 1.5.

PR validation:

In CMSSW_13_3_DBG_X_2023-08-01-2300 run successfully runTheMatrix.py -l 140.58 --ibeos

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:

Not a backport, but to be backported to CMSSW_13_2_X.

[mmusich]

type bug-fix

[mmusich]

Cc: @Ksavva1021

[mmusich]

test parameters:

• workflow = 140.58

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-42486/36492

• This PR adds an extra 12KB to repository

• There are other open Pull requests which might conflict with changes you have proposed:

  • File RecoLocalTracker/SiStripClusterizer/plugins/SiStripClusters2ApproxClusters.cc modified in PR(s): Add SiStrip ApproximateClusterCollection as a simple format for RAW #42022

[cmsbuild]

A new Pull Request was created by @mmusich (Marco Musich) for master.

It involves the following packages:

• RecoLocalTracker/SiStripClusterizer (reconstruction)

@cmsbuild, @mandrenguyen, @clacaputo can you please review it and eventually sign? Thanks.
@mtosi, @echabert, @VourMa, @GiacomoSguazzoni, @yduhm, @robervalwalsh, @JanFSchulte, @rovere, @gbenelli, @missirol, @threus, @felicepantaleo, @gpetruc, @mmusich, @VinInn, @alesaggio, @jlidrych 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]

@cmsbuild, please test for CMSSW_13_3_DBG_X

[mmusich]

concerning the HeaderConsistency failure reported by the cms-bot:

I believe it's not coming from this PR (see log)

[cmsbuild]

-1

Failed Tests: HeaderConsistency RelVals
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-db4e21/34130/summary.html
COMMIT: 2bd7e5a
CMSSW: CMSSW_13_3_DBG_X_2023-08-03-2300/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/42486/34130/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:

• @aloeliger Add bit wise unity initialization for default LUT arrays #42390
• @hatakeyamak Updates to PFClusterMatchedToPhotonsSelector for PF ECAL cluster calibration #42436
• @mmusich Fix approximated SiStrip clusters workflow by saving SiStrip FED error information  #42475
• @fabiocos MTD simulation: update trackId for BTL PSimHits #42455
• @bsunanda Run3-sim146 Modify the utility to dump sensitive detector for DDD and DD4hep option #42472
• @mmusich Tracker Alignment monitoring: add unbalance histograms for pointing angle in Z events #42459
• @bsunanda Run3-hcx352 Try to avoid wrong list of SD names of HCAL while using DD4hep #42463
• @bsunanda Run3-gex168 Modify all Run3 scenarios after fixing the HCAL SD issue #42476
• @bsunanda Phase2-gex168 Modify all Phase2 scenarios after fixing the HCAL SD issue #42477
• @CMS-LUMI-POG Pileup 2023 august #42474
• @bundocka L1T: add puppi jet requirement for filling zpt in L1Ntuples #42466
• @iarspider AVXVec: replace 2*insertf128 with 1*_mm256_set_m128d #42446
• @bsunanda Phase2-hgx344 Resolve the issue of non-matching DetIDs from DDD and DD4hep for HGCal versions in D98 and D99 #42478
• @bsunanda Phase2-hgx343 Try to retrieve missing hits in the partial wafers of HGCal for DD4Hep description #42442
• @fwyzard Extend convertToRaw to support different input collection names #42457

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

RelVals

• 23234.0

----- Begin Fatal Exception 07-Aug-2023 21:16:49 CEST-----------------------
An exception of category 'Invalid DetId' occurred while
   [0] Calling method for module HGCalBackendLayer1Producer/'l1tHGCalBackEndLayer1Producer'
   [1] Prefetching for EventSetup module HGCalTriggerGeometryESProducer/'l1tHGCalTriggerGeometryESProducer'
   [2] Calling method for EventSetup module HGCalGeometryESProducer/'HFNoseGeometryESProducer'
Exception Message:
Cannot initialize HGCSiliconDetId from 6c8bd800
----- End Fatal Exception -------------------------------------------------

[mmusich]

-1
Failed Tests: HeaderConsistency RelVals

so analyzing the output of the bot tests:

• as already reported in SiStripClusters2ApproxClusters: run peakFilter only if cluster is usable #42486 (comment) the HeaderConsistency failure looks spurious
• the failure in 23234.0 is expected, see also issue [DBG] Exception of category Invalid DetId in RelVal wf 23234.0 #41088
• on the bright side 140.58 did succeed (in DBG_X build), as per PR intentions.

I am going to relaunch the tests with the regular build to get rid of the failures.

[mmusich]

@cmsbuild, please test

[cmsbuild]

-1

Failed Tests: RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-db4e21/34149/summary.html
COMMIT: 2bd7e5a
CMSSW: CMSSW_13_3_X_2023-08-07-2300/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/42486/34149/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals-INPUT

The relvals timed out after 4 hours.

Comparison Summary

Summary:

• You potentially removed 3 lines from the logs
• Reco comparison results: 2 differences found in the comparisons
• DQMHistoTests: Total files compared: 49
• DQMHistoTests: Total histograms compared: 3212500
• DQMHistoTests: Total failures: 0
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 3212478
• DQMHistoTests: Total skipped: 22
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
• Checked 211 log files, 161 edm output root files, 49 DQM output files
• TriggerResults: no differences found

[mmusich]

-1
Failed Tests: RelVals-INPUT

let's try again.

[mmusich]

@cmsbuild, please test

[cmsbuild]

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-db4e21/34160/summary.html
COMMIT: 2bd7e5a
CMSSW: CMSSW_13_3_X_2023-08-08-1100/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/42486/34160/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

• You potentially removed 16 lines from the logs
• Reco comparison results: 4 differences found in the comparisons
• DQMHistoTests: Total files compared: 49
• DQMHistoTests: Total histograms compared: 3212500
• DQMHistoTests: Total failures: 3
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 3212475
• DQMHistoTests: Total skipped: 22
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
• Checked 211 log files, 161 edm output root files, 49 DQM output files
• TriggerResults: no differences found

[mandrenguyen]

+1

[cmsbuild]

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

[perrotta]

+1