Further optimization of the SiPixel LA PCL workflow #36565

mmusich · 2021-12-21T17:34:23Z

PR description:

This PR builds on top of #36538 and proposes some further optimization and bug-fixes to the SiPixel Lorentz Angle PCL workflow:

further memory footprint reduction is achieved in commit 69b9256 by loading the SiPixelTemplate object in memory on demand in the case in which the module SiPixelLorentzAnglePCLWorker is not run in PCL mode (notInPCL_ = True) and by reducing the number of bins of the 2D drift vs depth maps.
for Run2 replays the list of "new" modules is different. That is dealt with via era modifiers in commit c9aa27a
a bug that was messing up the computation of the value of the Lorentz Angle for the "new" modules is addressed at fb4f126
monitoring of the input number of clusters and output Lorentz Angle (and fit chi2/ndf) is introduced at 0cfb6bf
std::endl at the end of LogMessages is removed as requested at Several updates to SiPixel LA PCL workflow #36538 (comment) in commit 0cfb6bf
some documentation is added in commit 199bcd6

PR validation:

Privately run:

 cmsDriver.py testReAlCa -s ALCA:PromptCalibProdSiPixelLorentzAngle --conditions 121X_dataRun3_Express_TIER0_REPLAY_Run2_v1 --scenario pp --data --era Run2_2018 --datatier ALCARECO --eventcontent ALCARECO --processName=ReAlCa -n 100000 --dasquery='file dataset=/StreamExpress/Tier0_REPLAY_2021-SiPixelCalSingleMuon-Express-v1/ALCARECO' --customise_commands='process.ALCARECOCalSignleMuonFilterForSiPixelLorentzAngle.TriggerResultsTag = cms.InputTag ( "TriggerResults","","HLT" ) ; process.ALCARECOCalSignleMuonFilterForSiPixelLorentzAngle.HLTPaths = ["*"]' --nThreads=4

followed by:

 cmsDriver.py stepHarvest -s ALCAHARVEST:SiPixelLA --conditions 121X_dataRun3_Express_TIER0_REPLAY_Run2_v1 --scenario pp --data --era Run2_2018 --filein file:PromptCalibProdSiPixelLorentzAngle.root -n -1

the results are uploaded to a private DQM GUI at https://tinyurl.com/yxcmsvn6:

necessitates tunneling (ssh -NL 8060:localhost:8060 <USER>@lxplus723.cern.ch).

Profiling the RSS vs time of the job containing the ALCA:PromptCalibProdSiPixelLorentzAngle (run on 4 threads and 4 stream) shows a reduction of about half:

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

Not a backport, but might be backported together with #36538 to CMSSW_12_2_X as agreed at #36538 (comment)

cmsbuild · 2021-12-21T17:41:31Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36565/27510

This PR adds an extra 28KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File CalibTracker/SiPixelLorentzAngle/src/SiPixelLorentzAnglePCLWorker.cc modified in PR(s): Phase II Patatrack Pixel Local Reco #36235

cmsbuild · 2021-12-21T17:41:54Z

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

It involves the following packages:

CalibTracker/SiPixelLorentzAngle (alca)

@cmsbuild, @malbouis, @tvami, @yuanchao, @francescobrivio can you please review it and eventually sign? Thanks.
@tocheng, @OzAmram, @ferencek, @mmusich, @dkotlins, @tvami 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

tvami · 2021-12-21T17:58:00Z

@cmsbuild , plese test

mmusich · 2021-12-21T18:34:49Z

type bugfix

mmusich · 2021-12-21T21:48:25Z

please test

there is a typo at #36565 (comment)

cmsbuild · 2021-12-22T01:38:06Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-4779ad/21424/summary.html
COMMIT: 199bcd6
CMSSW: CMSSW_12_3_X_2021-12-21-1100/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/36565/21424/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: 4 differences found in the comparisons
DQMHistoTests: Total files compared: 43
DQMHistoTests: Total histograms compared: 3461551
DQMHistoTests: Total failures: 24
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3461505
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: -1314.487 KiB( 42 files compared)
DQMHistoSizes: changed ( 1001.0 ): -1314.487 KiB AlCaReco/SiPixelLorentzAngle
Checked 181 log files, 42 edm output root files, 43 DQM output files
TriggerResults: no differences found

yuanchao · 2021-12-22T14:22:36Z

+1

memory optimization and bug fixes
matrix tests and unit tests passed

cmsbuild · 2021-12-22T14:23:02Z

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)

CalibTracker/SiPixelLorentzAngle/src/SiPixelLorentzAnglePCLWorker.cc

CalibTracker/SiPixelLorentzAngle/python/SiPixelLorentzAnglePCLWorker_cfi.py

perrotta · 2021-12-22T15:13:06Z

@mmusich are the differences observed in the DQM for wf 1001 expected because of the code changes integrated here? E.g.:

perrotta · 2021-12-22T15:16:39Z

@mmusich, even if not part of this PR, the static analyzer reports about a non const static variable lower_bin_ in CalibTracker/SiPixelLorentzAngle/src/SiPixelLorentzAngle.cc
As far as I can see, that variable is never used in that code: am I missing something, or its definition can simply get removed from SiPixelLorentzAngle.cc?
(Not necessarily for this PR, unless given the comments above you realize that an update is needed anyhow)

cmsbuild · 2021-12-23T21:25:11Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36565/27549

This PR adds an extra 40KB to repository

cmsbuild · 2021-12-23T21:25:36Z

Pull request #36565 was updated. @cmsbuild, @malbouis, @tvami, @yuanchao, @francescobrivio can you please check and sign again.

mmusich · 2021-12-23T21:55:21Z

please test

cmsbuild · 2021-12-24T01:53:03Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-4779ad/21461/summary.html
COMMIT: 753902a
CMSSW: CMSSW_12_3_X_2021-12-23-1100/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/36565/21461/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: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 43
DQMHistoTests: Total histograms compared: 3461652
DQMHistoTests: Total failures: 0
DQMHistoTests: Total nulls: 69
DQMHistoTests: Total successes: 3461561
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: -3267.507 KiB( 42 files compared)
DQMHistoSizes: changed ( 1001.0 ): -3268.157 KiB AlCaReco/SiPixelLorentzAngle
DQMHistoSizes: changed ( 1001.0 ): 0.650 KiB AlCaReco/SiPixelLorentzAngleHarvesting
Checked 181 log files, 42 edm output root files, 43 DQM output files
TriggerResults: no differences found

mmusich · 2022-01-03T10:16:04Z

@cms-sw/alca-l2 ping

tvami · 2022-01-03T10:39:19Z

Hi @mmusich , Happy New Year! I thought you had a question to DQM in this PR. Is that resolved? Thanks

mmusich · 2022-01-03T10:46:39Z

Is that resolved?

As far as I am concerned, yes

tvami · 2022-01-03T10:58:09Z

Ok, from the AlCa side I think we are good

tvami · 2022-01-03T10:58:15Z

+alca

cmsbuild · 2022-01-03T10:58:34Z

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-04T16:33:56Z

+1

cmsbuild added this to the CMSSW_12_3_X milestone Dec 21, 2021

cmsbuild added alca-pending code-checks-pending orp-pending pending-signatures tests-pending labels Dec 21, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Dec 21, 2021

cmsbuild added the bug-fix label Dec 21, 2021

cmsbuild added tests-started and removed tests-pending labels Dec 21, 2021

cmsbuild added tests-approved and removed tests-started labels Dec 22, 2021

cmsbuild mentioned this pull request Dec 22, 2021

Phase II Patatrack Pixel Local Reco #36235

Merged

cmsbuild added alca-approved fully-signed and removed alca-pending pending-signatures labels Dec 22, 2021

perrotta reviewed Dec 22, 2021

View reviewed changes

CalibTracker/SiPixelLorentzAngle/src/SiPixelLorentzAnglePCLWorker.cc Show resolved Hide resolved

CalibTracker/SiPixelLorentzAngle/python/SiPixelLorentzAnglePCLWorker_cfi.py Outdated Show resolved Hide resolved

cmsbuild removed the alca-approved label Dec 22, 2021

add monitoring of rejected / accepted measurements, delta w.r.t. default

753902a

mmusich force-pushed the furtherOptSiPixelLorentzAnglePCLWorker branch from cf0934c to 753902a Compare December 23, 2021 21:19

cmsbuild added code-checks-approved and removed code-checks-pending labels Dec 23, 2021

cmsbuild added tests-started and removed tests-pending labels Dec 23, 2021

cmsbuild added tests-approved and removed tests-started labels Dec 24, 2021

cmsbuild added alca-approved fully-signed and removed alca-pending pending-signatures labels Jan 3, 2022

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

cmsbuild merged commit 1445eda into cms-sw:master Jan 4, 2022

mmusich deleted the furtherOptSiPixelLorentzAnglePCLWorker branch January 14, 2022 09:37

mmusich mentioned this pull request Jan 14, 2022

[12.2.X] Further optimization of the SiPixel LA PCL workflow #36711

Merged

This was referenced Feb 1, 2022

Improvements on SiPixelLorentzAngle PCL workflow: layer-dependent cluster size cut and output maps #36853

Merged

[12.2.X] Improvements on SiPixelLorentzAngle PCL workflow: layer-dependent cluster size cut and output maps #36866

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Further optimization of the SiPixel LA PCL workflow #36565

Further optimization of the SiPixel LA PCL workflow #36565

mmusich commented Dec 21, 2021 •

edited

cmsbuild commented Dec 21, 2021

cmsbuild commented Dec 21, 2021

tvami commented Dec 21, 2021

mmusich commented Dec 21, 2021

mmusich commented Dec 21, 2021

cmsbuild commented Dec 22, 2021

yuanchao commented Dec 22, 2021

cmsbuild commented Dec 22, 2021

perrotta commented Dec 22, 2021 •

edited

perrotta commented Dec 22, 2021

cmsbuild commented Dec 23, 2021

cmsbuild commented Dec 23, 2021

mmusich commented Dec 23, 2021

cmsbuild commented Dec 24, 2021

mmusich commented Jan 3, 2022

tvami commented Jan 3, 2022

mmusich commented Jan 3, 2022

tvami commented Jan 3, 2022

tvami commented Jan 3, 2022

cmsbuild commented Jan 3, 2022

perrotta commented Jan 4, 2022

Further optimization of the SiPixel LA PCL workflow #36565

Further optimization of the SiPixel LA PCL workflow #36565

Conversation

mmusich commented Dec 21, 2021 • edited

PR description:

PR validation:

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

cmsbuild commented Dec 21, 2021

cmsbuild commented Dec 21, 2021

tvami commented Dec 21, 2021

mmusich commented Dec 21, 2021

mmusich commented Dec 21, 2021

cmsbuild commented Dec 22, 2021

Comparison Summary

yuanchao commented Dec 22, 2021

cmsbuild commented Dec 22, 2021

perrotta commented Dec 22, 2021 • edited

perrotta commented Dec 22, 2021

cmsbuild commented Dec 23, 2021

cmsbuild commented Dec 23, 2021

mmusich commented Dec 23, 2021

cmsbuild commented Dec 24, 2021

Comparison Summary

mmusich commented Jan 3, 2022

tvami commented Jan 3, 2022

mmusich commented Jan 3, 2022

tvami commented Jan 3, 2022

tvami commented Jan 3, 2022

cmsbuild commented Jan 3, 2022

perrotta commented Jan 4, 2022

mmusich commented Dec 21, 2021 •

edited

perrotta commented Dec 22, 2021 •

edited