Irradiation Bias Correction (IBC) of pixel clusters at Alpaka #44569

mroguljic · 2024-03-28T12:04:17Z

PR description:

Implemented Irradiation Bias Correction (IBC) in the Alpaka version of the generic algorithm for esimating pixel cluster positions. Legacy implementation can be found here and here. The feature may be used to study how much the IBC will help with generic CPE algorithm at high irradiation doses and may be turned on if favorable.

The PR also includes a bug fix in the SiPixelRecHits/src/PixelCPEFastParamsHost.cc where template error parameters would be loaded at incorrect charge categories for certain modules. Small changes in standard workflows are expected as the errors of the RecHit positions on affected modules will differ.

PR validation:

Ran "11634.402" upgrade workflow in CMSSW_14_1_0_pre1 on 1000 events. Compared the results of "step3_RAW2DIGI_RECO_VALIDATION_DQM" with and without IBC using harvestTrackValidationPlots.py and makeTrackValidationPlots.py. Comparison can be found here. Some effects to tracking are observerd, but no major changes, which was expected.

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.

cmsbuild · 2024-03-28T12:04:43Z

cms-bot internal usage

cmsbuild · 2024-03-28T12:09:37Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44569/39720

This PR adds an extra 28KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File RecoLocalTracker/SiPixelRecHits/interface/pixelCPEforDevice.h modified in PR(s): Alpaka vs CUDA DQM compare modules for pixel tracks objects #43964
- File RecoLocalTracker/SiPixelRecHits/plugins/alpaka/PixelRecHits.h modified in PR(s): Alpaka vs CUDA DQM compare modules for pixel tracks objects #43964

cmsbuild · 2024-03-28T12:10:01Z

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

It involves the following packages:

RecoLocalTracker/SiPixelRecHits (reconstruction)

@jfernan2, @cmsbuild, @mandrenguyen can you please review it and eventually sign? Thanks.
@threus, @felicepantaleo, @JanFSchulte, @gpetruc, @missirol, @mmusich, @ferencek, @mroguljic, @GiacomoSguazzoni, @tvami, @VinInn, @mtosi, @rovere, @dkotlins, @VourMa, @tsusa this is something you requested to watch as well.
@rappoccio, @sextonkennedy, @antoniovilela you are the release manager for this.

cms-bot commands are listed here

RecoLocalTracker/SiPixelRecHits/interface/PixelCPEFastParamsHost.h

RecoLocalTracker/SiPixelRecHits/interface/pixelCPEforDevice.h

cmsbuild · 2024-03-29T12:58:07Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44569/39733

This PR adds an extra 28KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File RecoLocalTracker/SiPixelRecHits/interface/pixelCPEforDevice.h modified in PR(s): Alpaka vs CUDA DQM compare modules for pixel tracks objects #43964
- File RecoLocalTracker/SiPixelRecHits/plugins/alpaka/PixelRecHits.h modified in PR(s): Alpaka vs CUDA DQM compare modules for pixel tracks objects #43964

cmsbuild · 2024-03-29T12:58:33Z

Pull request #44569 was updated. @mandrenguyen, @jfernan2, @cmsbuild can you please check and sign again.

mmusich · 2024-03-29T12:58:51Z

enable gpu

mmusich · 2024-03-29T13:03:19Z

test parameters:

workflow_opts_gpu= -w upgrade
workflows_gpu= 12434.403, 12434.503

mmusich · 2024-03-29T13:03:27Z

@cmsbuild, please test

cmsbuild · 2024-03-29T16:13:44Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-84c28a/38509/summary.html
COMMIT: 7216e30
CMSSW: CMSSW_14_1_X_2024-03-28-2300/el8_amd64_gcc12
Additional Tests: GPU
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/44569/38509/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially removed 20 lines from the logs
Reco comparison results: 50 differences found in the comparisons
DQMHistoTests: Total files compared: 48
DQMHistoTests: Total histograms compared: 3297469
DQMHistoTests: Total failures: 9
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3297440
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

GPU Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 30 differences found in the comparisons
DQMHistoTests: Total files compared: 5
DQMHistoTests: Total histograms compared: 72013
DQMHistoTests: Total failures: 3225
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 68788
DQMHistoTests: Total skipped: 0
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 4 files compared)
Checked 16 log files, 20 edm output root files, 5 DQM output files
TriggerResults: no differences found

cmsbuild · 2024-04-03T15:22:23Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44569/39788

This PR adds an extra 28KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File RecoLocalTracker/SiPixelRecHits/interface/pixelCPEforDevice.h modified in PR(s): Alpaka vs CUDA DQM compare modules for pixel tracks objects #43964
- File RecoLocalTracker/SiPixelRecHits/plugins/alpaka/PixelRecHits.h modified in PR(s): Alpaka vs CUDA DQM compare modules for pixel tracks objects #43964

cmsbuild · 2024-04-03T15:22:48Z

Pull request #44569 was updated. @jfernan2, @cmsbuild, @mandrenguyen can you please check and sign again.

mroguljic · 2024-04-03T15:40:20Z

What is the impact of the changes in PixelCPEFastParamsHost<TrackerTraits>::fillParamsForDevice() when the IBC is disabled ?

The loop assigning error parameters to detParameters runs slightly faster because it loops only up to the charge belonging to the largest qBin category. I tried validating the results on tracking using some TTbar events as explained in this comment. As stated, small changes are present, but I am not sure if this validation is enough.

jfernan2 · 2024-04-10T14:32:46Z

type bug-fix

jfernan2 · 2024-04-10T14:34:00Z

assign heterogeneous

cmsbuild · 2024-04-10T14:34:24Z

New categories assigned: heterogeneous

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

jfernan2 · 2024-04-10T19:14:27Z

please test

fwyzard

Apart from a couple of minor suggestions, the changes look good to me.

Could you measure the impact on the HLT timing of

only the bugfix
the bugfix and the IBC enabled

?

fwyzard · 2024-04-10T19:47:04Z

RecoLocalTracker/SiPixelRecHits/interface/PixelCPEFastParamsHost.h

@@ -61,6 +62,7 @@ class PixelCPEFastParamsHost : public PixelCPEGenericBase {
  void fillParamsForDevice();

  Buffer buffer_;
+  bool irradiationBiasCorrection_;


can this be const ?

fwyzard · 2024-04-10T19:47:53Z

RecoLocalTracker/SiPixelRecHits/src/PixelCPEFastParamsHost.cc

@@ -31,6 +32,7 @@ PixelCPEFastParamsHost<TrackerTraits>::PixelCPEFastParamsHost(edm::ParameterSet
          << (*genErrorDBObject_).version();
  }

+  irradiationBiasCorrection_ = irradiationBiasCorrection;


can this be set in the member initialiser list above ?

const SiPixelLorentzAngle* lorentzAngleWidth, const bool irradiationBiasCorrection) : PixelCPEGenericBase(conf, mag, geom, ttopo, lorentzAngle, genErrorDBObject, lorentzAngleWidth), buffer_(cms::alpakatools::make_host_buffer<pixelCPEforDevice::ParamsOnDeviceT<TrackerTraits>>()), irradiationBiasCorrection_(irradiationBiasCorrection) {

cmsbuild · 2024-04-10T22:16:57Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-84c28a/38754/summary.html
COMMIT: 6226f19
CMSSW: CMSSW_14_1_X_2024-04-10-1100/el8_amd64_gcc12
Additional Tests: GPU
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/44569/38754/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially removed 2 lines from the logs
Reco comparison results: 48 differences found in the comparisons
DQMHistoTests: Total files compared: 48
DQMHistoTests: Total histograms compared: 3316263
DQMHistoTests: Total failures: 6
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3316237
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

GPU Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 57 differences found in the comparisons
DQMHistoTests: Total files compared: 5
DQMHistoTests: Total histograms compared: 72013
DQMHistoTests: Total failures: 4837
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 67176
DQMHistoTests: Total skipped: 0
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 4 files compared)
Checked 16 log files, 20 edm output root files, 5 DQM output files
TriggerResults: no differences found

mroguljic · 2024-04-11T04:50:29Z

Apart from a couple of minor suggestions, the changes look good to me.

Could you measure the impact on the HLT timing of
* only the bugfix

* the bugfix and the IBC enabled
?

Sure, are there instructions on how to do so?

mmusich · 2024-04-11T05:52:02Z

Sure, are there instructions on how to do so?

e.g. https://twiki.cern.ch/twiki/bin/view/CMS/TriggerStudiesTiming

AdrianoDee · 2024-04-11T08:53:16Z

Find few results with 2023 conditions TTbar (with PU) MC here. E.g., the hit resolution in X for Barrel 1 attached here. Legend:

blue (invisible) the results for `master;
orange results for this PR without IBC;
red results for this PR with IBC.

mmusich · 2024-04-11T08:55:57Z

doesn't look good.

mroguljic · 2024-04-23T10:25:36Z

@fwyzard The IBC part needs further studies to understand why the resolution apparently becomes worse. I've added the bugfix in a separate PR.

mroguljic added 3 commits March 20, 2024 12:26

IBC at GPU

901d508

update ibc implementation for 14_1_0

5d60ec8

code quality checks

34e977b

cmsbuild added this to the CMSSW_14_1_X milestone Mar 28, 2024

cmsbuild added reconstruction-pending pending-signatures tests-pending orp-pending code-checks-pending trk labels Mar 28, 2024

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

mroguljic marked this pull request as ready for review March 28, 2024 14:27

mmusich reviewed Mar 29, 2024

View reviewed changes

RecoLocalTracker/SiPixelRecHits/interface/PixelCPEFastParamsHost.h Outdated Show resolved Hide resolved

RecoLocalTracker/SiPixelRecHits/interface/pixelCPEforDevice.h Outdated Show resolved Hide resolved

Start data member names with lower case

7216e30

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

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

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

code review comments

6226f19

cmsbuild added code-checks-pending and removed code-checks-approved labels Apr 3, 2024

cmsbuild added code-checks-approved and removed code-checks-pending labels Apr 3, 2024

cmsbuild added the bug-fix label Apr 10, 2024

cmsbuild added the heterogeneous-pending label Apr 10, 2024

cmsbuild added tests-started and removed tests-pending labels Apr 10, 2024

fwyzard suggested changes Apr 10, 2024

View reviewed changes

cmsbuild added tests-approved and removed tests-started labels Apr 10, 2024

cmsbuild mentioned this pull request Apr 12, 2024

Alpaka vs CUDA DQM compare modules for pixel tracks objects #43964

Open

cmsbuild mentioned this pull request Apr 22, 2024

Migrate assert to to ALPAKA_ASSERT_ACC #44802

Merged

mroguljic mentioned this pull request Apr 23, 2024

fix skipping qBin when reading template info #44813

Merged

mroguljic closed this Apr 23, 2024

mroguljic mentioned this pull request Apr 23, 2024

[14_0_X]fix skipping qBin when reading template info #44816

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Irradiation Bias Correction (IBC) of pixel clusters at Alpaka #44569

Irradiation Bias Correction (IBC) of pixel clusters at Alpaka #44569

mroguljic commented Mar 28, 2024 •

edited

cmsbuild commented Mar 28, 2024 •

edited

cmsbuild commented Mar 28, 2024

cmsbuild commented Mar 28, 2024

cmsbuild commented Mar 29, 2024

cmsbuild commented Mar 29, 2024

mmusich commented Mar 29, 2024

mmusich commented Mar 29, 2024

mmusich commented Mar 29, 2024

cmsbuild commented Mar 29, 2024

cmsbuild commented Apr 3, 2024

cmsbuild commented Apr 3, 2024

mroguljic commented Apr 3, 2024

jfernan2 commented Apr 10, 2024

jfernan2 commented Apr 10, 2024

cmsbuild commented Apr 10, 2024

jfernan2 commented Apr 10, 2024

fwyzard left a comment

fwyzard Apr 10, 2024

fwyzard Apr 10, 2024

cmsbuild commented Apr 10, 2024

mroguljic commented Apr 11, 2024

mmusich commented Apr 11, 2024

AdrianoDee commented Apr 11, 2024

mmusich commented Apr 11, 2024

mroguljic commented Apr 23, 2024

Irradiation Bias Correction (IBC) of pixel clusters at Alpaka #44569

Irradiation Bias Correction (IBC) of pixel clusters at Alpaka #44569

Conversation

mroguljic commented Mar 28, 2024 • edited

PR description:

PR validation:

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:

cmsbuild commented Mar 28, 2024 • edited

cmsbuild commented Mar 28, 2024

cmsbuild commented Mar 28, 2024

cmsbuild commented Mar 29, 2024

cmsbuild commented Mar 29, 2024

mmusich commented Mar 29, 2024

mmusich commented Mar 29, 2024

mmusich commented Mar 29, 2024

cmsbuild commented Mar 29, 2024

Comparison Summary

GPU Comparison Summary

cmsbuild commented Apr 3, 2024

cmsbuild commented Apr 3, 2024

mroguljic commented Apr 3, 2024

jfernan2 commented Apr 10, 2024

jfernan2 commented Apr 10, 2024

cmsbuild commented Apr 10, 2024

jfernan2 commented Apr 10, 2024

fwyzard left a comment

Choose a reason for hiding this comment

fwyzard Apr 10, 2024

Choose a reason for hiding this comment

fwyzard Apr 10, 2024

Choose a reason for hiding this comment

cmsbuild commented Apr 10, 2024

Comparison Summary

GPU Comparison Summary

mroguljic commented Apr 11, 2024

mmusich commented Apr 11, 2024

AdrianoDee commented Apr 11, 2024

mmusich commented Apr 11, 2024

mroguljic commented Apr 23, 2024

mroguljic commented Mar 28, 2024 •

edited

cmsbuild commented Mar 28, 2024 •

edited