Fix lostTracks producer not setting lostInnerHits

[elusian]

PR description:

The lostTracks producer has a bug that leaves the lostInnerHits of the PackedCandidate unset (and thus at its default value of validHitInFirstPixelBarrelLayer). Other than in lostInnerHits the bug propagates in the number of valid pixel hits and the number of pixel and tracker layers with measurements in the HitPattern of the bestTrack of the candidate.

This PR fixes the bug by adding the call to setLostInnerHits as the code from the packedPFCandidates producer does. This fix has been checked running the workflow 136.8311 and the issue was solved (see attached figures in the PR on master).

PR validation:

This PR has been validated with runTheMatrix.py -l limited -i all --ibeos and all checks are ok.
The final output is

35 34 33 25 12 2 1 1 1 tests passed, 0 0 0 0 0 0 0 0 0 failed

In particular the DQM output of 136.8311 shows the differences which are expected: inside Tracking/PackedCandidate/lostTracks, in diffLostInnerHits, diffHitPatternNumberOfLostPixelHits, diffHitPatternHasValidHitInFirstPixelBarrel, diffHitPatternNumberOfValidPixelHits, diffHitPatternPixelLayersWithMeasurement, diffHitPatternTrackerLayersWithMeasurement, see the attached plots in the PR on master.

if this PR is a backport please specify the original PR and why you need to backport that PR:

This PR is a backport of #32814, needed for BParking reprocessing.

[cmsbuild]

A new Pull Request was created by @elusian for CMSSW_10_6_X.

It involves the following packages:

PhysicsTools/PatAlgos

@perrotta, @jpata, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks.
@rappoccio, @gouskos, @jdolen, @JyothsnaKomaragiri, @ahinzmann, @schoef, @emilbols, @swozniewski, @jdamgov, @mbluj, @nhanvtran, @gkasieczka, @clelange, @hatakeyamak, @ferencek, @gpetruc, @andrzejnovak, @mariadalfonso, @seemasharmafnal, @mmarionncern this is something you requested to watch as well.
@silviodonato, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

• Backported from Fix lostTracks producer not setting lostInnerHits #32814

[perrotta]

backport of #32814

[perrotta]

please test

[slava77]

my interpretation of the no-change policy is that this PR should provide a configurable way to apply the bugfix and we can include it in a relevant production (re)processing with a configuration switch.
From the description it sounds like the switch would enable the feature with the bParking modifier.

[mtosi]

ciao, sorry, but just for understanding
even if it is a bugfix, one needs to add a switch in order to enable the bugfix ?

so, @elusian should add a flag in the config in order to run his code, while being still back-compatible ?
thanks

[cmsbuild]

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-74d077/13741/summary.html
COMMIT: fe13e88
CMSSW: CMSSW_10_6_X_2021-03-24-1100/slc7_amd64_gcc700
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/33258/13741/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: 88 differences found in the comparisons
• DQMHistoTests: Total files compared: 35
• DQMHistoTests: Total histograms compared: 3215541
• DQMHistoTests: Total failures: 106
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 3215101
• DQMHistoTests: Total skipped: 334
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 34 files compared)
• Checked 143 log files, 29 edm output root files, 35 DQM output files
• TriggerResults: no differences found

[slava77]

ciao, sorry, but just for understanding
even if it is a bugfix, one needs to add a switch in order to enable the bugfix ?

if the bug is not significant enough to invalidate the current production, then it essentially becomes a feature.

In the past we left exceptions to the no-change requirement for variables that were known to be not used or if they were fairly localized (e.g. one variable not used anywhere else downstream).

Here the way how the lostInnerHits is used and saved is too convoluted. So, it seems more appropriate to have this fix on demand.

[cmsbuild]

Pull request #33258 was updated. @perrotta, @jpata, @cmsbuild, @slava77 can you please check and sign again.

[mtosi]

@cmsbuild, please test

[elusian]

The new changes were tested on 10824.8 workflow, which enables the bParking modifier. The fix was confirmed to be working with this workflow, while it is not applied on the 136.8311 workflow. Removing the bParking era modifier from the cmsDriver invocation for step3 in the 10824.8 workflow also disabled the bugfix.

[cmsbuild]

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-74d077/13799/summary.html
COMMIT: 70cc48c
CMSSW: CMSSW_10_6_X_2021-03-26-1100/slc7_amd64_gcc700
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/33258/13799/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: 2 differences found in the comparisons
• DQMHistoTests: Total files compared: 35
• DQMHistoTests: Total histograms compared: 3215541
• DQMHistoTests: Total failures: 2
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 3215205
• DQMHistoTests: Total skipped: 334
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 34 files compared)
• Checked 143 log files, 29 edm output root files, 35 DQM output files
• TriggerResults: no differences found

[jpata]

+reconstruction

• backport of Fix lostTracks producer not setting lostInnerHits #32814, enabled only with the bParking modifier
• backport looks correct wrt. the original
• no changes observed in the matrix workflows, as expected

[cmsbuild]

This pull request is fully signed and it will be integrated in one of the next CMSSW_10_6_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_11_3_X is complete. This pull request will now be reviewed by the release team before it's merged. @silviodonato, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

[qliphy]

+1