Pixel Heterogeneous DQM: add module rechit @cpu / @gpu comparisons #37860

sroychow · 2022-05-09T13:32:57Z

PR description:

Code is by @arossi83
Adding a module to compare rechits @gpu/@cpu comparisons. The 2D correlation plots are done per layer/disk for Bpix/Fpix.

PR validation:

wfs with .503 runs successfully.

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

Backport to 12.3.X will be needed.

mmusich · 2022-05-09T13:39:37Z

test parameters:

enable_tests = gpu
workflows_gpu = 11634.503, 11634.506, 11634.583, 11634.587

cmsbuild · 2022-05-09T13:40:19Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37860/29808

This PR adds an extra 16KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File DQM/SiPixelPhase1Heterogeneous/python/SiPixelPhase1HeterogenousDQM_FirstStep_cff.py modified in PR(s): Follow up to 37617 #37798

Code check has found code style and quality issues which could be resolved by applying following patch(s)

code-format:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37860/29808/code-format.patch
e.g. curl -k https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37860/29808/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

mmusich

some preliminary comments

DQM/SiPixelPhase1Heterogeneous/plugins/SiPixelPhase1CompareRecHitsSoA.cc

mmusich · 2022-05-09T13:44:00Z

DQM/SiPixelPhase1Heterogeneous/python/SiPixelPhase1HeterogenousDQM_FirstStep_cff.py

@@ -35,13 +36,25 @@
  topFolderName = 'SiPixelHeterogeneous/PixelVertexSoAGPU',
 )

+siPixelPhase1MonitorRecHitsSoACPU = siPixelPhase1MonitorRecHitsSoA.clone(


isn't this conflicting with https://github.com/cms-sw/cmssw/pull/37798/files#diff-9028f77f088f106cd424e7fa0b3ee30e8f1efd6f2f489b3b8ee3fde183babfebR37 ?

mmusich · 2022-05-09T13:45:16Z

assign heterogeneous

cmsbuild · 2022-05-09T13:51:51Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37860/29810

This PR adds an extra 16KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File DQM/SiPixelPhase1Heterogeneous/python/SiPixelPhase1HeterogenousDQM_FirstStep_cff.py modified in PR(s): Follow up to 37617 #37798

cmsbuild · 2022-05-09T13:52:13Z

New categories assigned: heterogeneous

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

cmsbuild · 2022-05-09T13:52:15Z

A new Pull Request was created by @sroychow (Suvankar Roy Chowdhury) for master.

It involves the following packages:

DQM/SiPixelPhase1Heterogeneous (dqm)

@makortel, @emanueleusai, @ahmad3213, @cmsbuild, @jfernan2, @fwyzard, @pmandrik, @micsucmed, @rvenditti can you please review it and eventually sign? Thanks.
@hdelanno, @fioriNTU, @jandrea, @idebruyn, @mmusich, @threus 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

mmusich · 2022-05-09T14:30:24Z

type trk

DQM/SiPixelPhase1Heterogeneous/plugins/SiPixelPhase1CompareRecHitsSoA.cc

emanueleusai · 2022-05-10T13:13:59Z

waiting for Marco's comments to be addressed before re-issuing the test command.

fwyzard · 2022-05-11T17:03:35Z

DQM/SiPixelPhase1Heterogeneous/plugins/SiPixelPhase1CompareRecHitsSoA.cc

+        float dx = soa2dCPU->xLocal(i) - soa2dGPU->xLocal(i);
+        float dy = soa2dCPU->yLocal(i) - soa2dGPU->yLocal(i);


Is't the GPU index j ?
Then these should be

Suggested change

float dx = soa2dCPU->xLocal(i) - soa2dGPU->xLocal(i);

float dy = soa2dCPU->yLocal(i) - soa2dGPU->yLocal(i);

float dx = soa2dCPU->xLocal(i) - soa2dGPU->xLocal(j);

float dy = soa2dCPU->yLocal(i) - soa2dGPU->yLocal(j);

DQM/SiPixelPhase1Heterogeneous/plugins/SiPixelPhase1CompareRecHitsSoA.cc

+        if (distance < minD) {
+          minD = distance;
+          matchedHit = j;
+        }


fwyzard · 2022-05-11T17:07:32Z

DQM/SiPixelPhase1Heterogeneous/plugins/SiPixelPhase1CompareRecHitsSoA.cc

+    int16_t sizeXCPU = std::ceil(float(std::abs(soa2dCPU->clusterSizeX(i)) / 8.));
+    int16_t sizeYCPU = std::ceil(float(std::abs(soa2dCPU->clusterSizeY(i)) / 8.));


why the std::abs ?

AdrianoDee · 2022-05-12T18:06:09Z

If a backport to 12_3_X is needed for this PR, we will need backports of #37798 and #37617. I've opened #37933 if that's the case.

mmusich · 2022-05-17T19:44:25Z

please test

let's try again

cmsbuild · 2022-05-17T23:47:53Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-46f431/24803/summary.html
COMMIT: 8e54100
CMSSW: CMSSW_12_5_X_2022-05-17-1100/slc7_amd64_gcc10
Additional Tests: GPU
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/37860/24803/install.sh to create a dev area with all the needed externals and cmssw changes.

GPU Comparison Summary

@slava77 comparisons for the following workflows were not done due to missing matrix map:

/data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-46f431/11634.503_TTbar_14TeV+2021_Patatrack_PixelOnlyGPU_Validation+TTbar_14TeV_TuneCP5_GenSim+Digi+RecoNano+HARVESTNano
/data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-46f431/11634.583_TTbar_14TeV+2021_Patatrack_AllGPU_Validation+TTbar_14TeV_TuneCP5_GenSim+Digi+RecoNano+HARVESTNano
/data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-46f431/11634.587_TTbar_14TeV+2021_Patatrack_AllTripletsGPU_Validation+TTbar_14TeV_TuneCP5_GenSim+Digi+RecoNano+HARVESTNano

Summary:

No significant changes to the logs found
Reco comparison results: 24 differences found in the comparisons
DQMHistoTests: Total files compared: 7
DQMHistoTests: Total histograms compared: 76180
DQMHistoTests: Total failures: 984
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 75196
DQMHistoTests: Total skipped: 0
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 1528.4199999999996 KiB( 6 files compared)
DQMHistoSizes: changed ( 11634.506 ): -3488.195 KiB SiPixelHeterogeneous/PixelRecHitsSoA
DQMHistoSizes: changed ( 11634.503,... ): 8648.654 KiB SiPixelHeterogeneous/PixelRecHitsCompareGPUvsCPU
DQMHistoSizes: changed ( 11634.503,... ): -3488.225 KiB SiPixelHeterogeneous/PixelRecHitsSoACPU
DQMHistoSizes: changed ( 11634.503,... ): -3488.225 KiB SiPixelHeterogeneous/PixelRecHitsSoAGPU
Checked 24 log files, 18 edm output root files, 7 DQM output files
TriggerResults: no differences found

Comparison Summary

@slava77 comparisons for the following workflows were not done due to missing matrix map:

/data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-46f431/11634.301_TTbar_14TeV+2021_Run3FS+TTbar_14TeV_TuneCP5_GenSim+HARVESTNano

Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 51
DQMHistoTests: Total histograms compared: 3688458
DQMHistoTests: Total failures: 2
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3688434
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: -3488.195 KiB( 50 files compared)
DQMHistoSizes: changed ( 11634.501 ): -3488.195 KiB SiPixelHeterogeneous/PixelRecHitsSoA
Checked 212 log files, 48 edm output root files, 51 DQM output files
TriggerResults: no differences found

cmsbuild · 2022-05-18T06:55:15Z

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)

emanueleusai · 2022-05-18T07:10:01Z

a few comments:

There's a large number of failures, is this understood?
In 11634.506 baseline GUI in a large number of histos is changed and some are adde. I understand this is most likely caused by this PR, but I just want to make sure this is understood
Comparison and Comparison GPU have failed (probably because of the point above?)
[cosmetics] perhaps some 2d plots could be displayed in colz?

mmusich · 2022-05-18T07:31:16Z

Hi @emanueleusai

There's a large number of failures, is this understood?

if you are referring to the histograms that change (and not the ones that are added or removed), as you might remember it was discussed in several other PRs, there is a small residual intrinsic non-reproducibility in the tracking GPU workflows caused by operations that do not commute when the execution order is changed.

In 11634.506 baseline GUI in a large number of histos is changed and some are adde. I understand this is most likely caused by this PR, but I just want to make sure this is understood.

I think it is.

Comparison and Comparison GPU have failed (probably because of the point above?)

Not sure to get what you mean by "failed" in this context

[cosmetics] perhaps some 2d plots could be displayed in colz?

the particular plot you link in the comment above #37860 (comment) has not been added in this PR, but in general, as you might be aware, that sort of cosmetics is generally dealt with at the level of render plugins in the GUI code and not in cmssw (we actually do plan to provide new render plugins once this is merged).

emanueleusai · 2022-05-19T05:24:45Z

+1

cmsbuild · 2022-05-19T05:25:08Z

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)

qliphy · 2022-05-20T06:52:27Z

+1

cmsbuild added this to the CMSSW_12_4_X milestone May 9, 2022

cmsbuild added code-checks-pending dqm-pending orp-pending pending-signatures tests-pending labels May 9, 2022

cmsbuild added code-checks-rejected code-checks-pending and removed code-checks-pending code-checks-rejected labels May 9, 2022

mmusich reviewed May 9, 2022

View reviewed changes

cmsbuild added the heterogeneous-pending label May 9, 2022

mmusich mentioned this pull request May 9, 2022

Follow up to 37617 #37798

Merged

cmsbuild added code-checks-approved and removed code-checks-pending labels May 9, 2022

cmsbuild added the trk label May 9, 2022

mmusich reviewed May 10, 2022

View reviewed changes

DQM/SiPixelPhase1Heterogeneous/plugins/SiPixelPhase1CompareRecHitsSoA.cc Show resolved Hide resolved

fwyzard reviewed May 11, 2022

View reviewed changes

DQM/SiPixelPhase1Heterogeneous/plugins/SiPixelPhase1CompareRecHitsSoA.cc

Comment on lines +103 to +132

if (distance < minD) {

minD = distance;

matchedHit = j;

}

This comment was marked as outdated.

Sign in to view

fwyzard reviewed May 11, 2022

View reviewed changes

AdrianoDee mentioned this pull request May 12, 2022

[12_3_X] Backport of 37617 + 37798 #37933

Merged

cmsbuild added the tests-rejected label May 17, 2022

cmsbuild added tests-started and removed tests-rejected labels May 17, 2022

cmsbuild added tests-approved dqm-approved fully-signed and removed tests-started dqm-pending pending-signatures labels May 17, 2022

cmsbuild added dqm-pending pending-signatures and removed dqm-approved fully-signed labels May 18, 2022

cmsbuild added dqm-approved fully-signed and removed dqm-pending pending-signatures labels May 19, 2022

cmsbuild added orp-approved and removed orp-pending labels May 20, 2022

cmsbuild merged commit 97aba66 into cms-sw:master May 20, 2022

This was referenced May 20, 2022

Adding DeepMET SONIC Producer to CMSSW #37963

Merged

DeepSC algorithm for ECAL SuperClusters #37115

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pixel Heterogeneous DQM: add module rechit @cpu / @gpu comparisons #37860

Pixel Heterogeneous DQM: add module rechit @cpu / @gpu comparisons #37860

sroychow commented May 9, 2022

mmusich commented May 9, 2022

cmsbuild commented May 9, 2022

mmusich left a comment

mmusich May 9, 2022

mmusich commented May 9, 2022

cmsbuild commented May 9, 2022

cmsbuild commented May 9, 2022

cmsbuild commented May 9, 2022

mmusich commented May 9, 2022

emanueleusai commented May 10, 2022

fwyzard May 11, 2022

This comment was marked as outdated.

fwyzard May 11, 2022

AdrianoDee commented May 12, 2022 •

edited

mmusich commented May 17, 2022

cmsbuild commented May 17, 2022

cmsbuild commented May 18, 2022

emanueleusai commented May 18, 2022

mmusich commented May 18, 2022

emanueleusai commented May 19, 2022

cmsbuild commented May 19, 2022

qliphy commented May 20, 2022

		float dx = soa2dCPU->xLocal(i) - soa2dGPU->xLocal(i);
		float dy = soa2dCPU->yLocal(i) - soa2dGPU->yLocal(i);

		int16_t sizeXCPU = std::ceil(float(std::abs(soa2dCPU->clusterSizeX(i)) / 8.));
		int16_t sizeYCPU = std::ceil(float(std::abs(soa2dCPU->clusterSizeY(i)) / 8.));

Pixel Heterogeneous DQM: add module rechit @cpu / @gpu comparisons #37860

Pixel Heterogeneous DQM: add module rechit @cpu / @gpu comparisons #37860

Conversation

sroychow commented May 9, 2022

PR description:

PR validation:

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

mmusich commented May 9, 2022

cmsbuild commented May 9, 2022

mmusich left a comment

Choose a reason for hiding this comment

mmusich May 9, 2022

Choose a reason for hiding this comment

mmusich commented May 9, 2022

cmsbuild commented May 9, 2022

cmsbuild commented May 9, 2022

cmsbuild commented May 9, 2022

mmusich commented May 9, 2022

emanueleusai commented May 10, 2022

fwyzard May 11, 2022

Choose a reason for hiding this comment

This comment was marked as outdated.

fwyzard May 11, 2022

Choose a reason for hiding this comment

AdrianoDee commented May 12, 2022 • edited

mmusich commented May 17, 2022

cmsbuild commented May 17, 2022

GPU Comparison Summary

Comparison Summary

cmsbuild commented May 18, 2022

emanueleusai commented May 18, 2022

mmusich commented May 18, 2022

emanueleusai commented May 19, 2022

cmsbuild commented May 19, 2022

qliphy commented May 20, 2022

AdrianoDee commented May 12, 2022 •

edited