Patatrack integration - calorimeters shared code (6/N) #31704

fwyzard · 2020-10-07T17:15:48Z

PR description:

Data formats and algorithms shared by the ECAL and HCAL local reconstruction (multifit and MAHI).

PR validation:

Changes in use in the Patatrack releases.

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

Includes changes from:

Skip CUDA-related tests if no GPU is present cms-patatrack/cmssw#252: Skip CUDA-related tests if no GPU is present
Implement inversion by Cholesky Decomposition cms-patatrack/cmssw#337: Implement inversion by Cholesky Decomposition
Replace remaining cuda::device operations with native CUDA calls. cms-patatrack/cmssw#408: Replace cuda::device operations with native CUDA calls
Rename exitSansCUDADevices to requireCUDADevices cms-patatrack/cmssw#423: Rename exitSansCUDADevices to requireCUDADevices
[WIP] Follow-ups to the CUDA framework PR review cms-patatrack/cmssw#429: Implement changes from the CUDA framework review
Integrate the comments from the upstream PRs cms-patatrack/cmssw#442: Integrate the comments from the upstream PRs
Update ECAL and HCAL reconstruction to run on multple GPUs cms-patatrack/cmssw#502: Update ECAL and HCAL reconstruction to run on multple GPUs [1/3]
ECAL-HCAL: code refactoring cms-patatrack/cmssw#523: Refactor common ECAL and HCAL code
Move multifit/MAHI common code to DataFormats/CaloRecHit cms-patatrack/cmssw#557: Move multifit/MAHI common code to DataFormats/CaloRecHit

cmsbuild · 2020-10-07T17:16:08Z

The code-checks are being triggered in jenkins.

fwyzard · 2020-10-07T17:16:56Z

For all questions, please address @amassiro @mariadalfonso @vkhristenko .
For all changes, please make PRs against cms-patatrack:patatrack_integration_6_N_calorimeters_common .

fwyzard · 2020-10-07T17:17:25Z

@cmsbuild, please test with #31703

cmsbuild · 2020-10-07T17:22:53Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-31704/18866

This PR adds an extra 24KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File CUDADataFormats/CaloCommon/BuildFile.xml modified in PR(s): Squash all Patatrack developments on top of CMSSW_11_3_0_pre5 #27983
- File CUDADataFormats/CaloCommon/interface/Common.h modified in PR(s): Squash all Patatrack developments on top of CMSSW_11_3_0_pre5 #27983
- File CUDADataFormats/CaloCommon/src/classes.h modified in PR(s): Squash all Patatrack developments on top of CMSSW_11_3_0_pre5 #27983
- File CUDADataFormats/CaloCommon/src/classes_def.xml modified in PR(s): Squash all Patatrack developments on top of CMSSW_11_3_0_pre5 #27983
- File DataFormats/Math/interface/EigenComputations.h modified in PR(s): Squash all Patatrack developments on top of CMSSW_11_3_0_pre5 #27983
- File DataFormats/Math/test/BuildFile.xml modified in PR(s): Squash all Patatrack developments on top of CMSSW_11_3_0_pre5 #27983

cmsbuild · 2020-10-07T17:23:21Z

The tests are being triggered in jenkins.
Tested with other pull request(s) #31703

CMSSW_11_2_X_2020-10-07-1100/slc7_amd64_gcc820: https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/9784/console Started: 2020/10/07 19:24

cmsbuild · 2020-10-07T17:23:25Z

A new Pull Request was created by @fwyzard (Andrea Bocci) for master.

It involves the following packages:

CUDADataFormats/CaloCommon
DataFormats/Math

The following packages do not have a category, yet:

CUDADataFormats/CaloCommon
Please create a PR for https://github.com/cms-sw/cms-bot/blob/master/categories_map.py to assign category

@perrotta, @jpata, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks.
@makortel, @felicepantaleo, @fabiocos, @rovere 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

cmsbuild · 2020-10-07T18:38:31Z

-1

Tested at: f6571a8

CMSSW: CMSSW_11_2_X_2020-10-07-1100
SCRAM_ARCH: slc7_amd64_gcc820
You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-aa7054/9784/summary.html

I found follow errors while testing this PR

Failed tests: HeaderConsistency

cmsbuild · 2020-10-07T18:38:36Z

Comparison job queued.

cmsbuild · 2020-10-07T20:03:05Z

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-aa7054/9784/summary.html

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 35
DQMHistoTests: Total histograms compared: 2542225
DQMHistoTests: Total failures: 1
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2542202
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 34 files compared)
Checked 149 log files, 22 edm output root files, 35 DQM output files

cmsbuild · 2020-10-20T17:39:06Z

+1
Tested at: 8938c57
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-bad05c/10144/summary.html
CMSSW: CMSSW_11_2_X_2020-10-19-2300
SCRAM_ARCH: slc7_amd64_gcc820

cmsbuild · 2020-10-20T17:39:08Z

Comparison job queued.

cmsbuild · 2020-10-20T18:54:11Z

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-bad05c/10144/summary.html

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 35
DQMHistoTests: Total histograms compared: 2544100
DQMHistoTests: Total failures: 1
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2544077
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 34 files compared)
Checked 149 log files, 22 edm output root files, 35 DQM output files

jpata · 2020-10-21T05:39:24Z

Even with enabling the GPU in the test, test_calo_rechit did not run.

===== Test "test_calo_rechit" ====
Failed to initialise the CUDA runtime, the test will be skipped.

---> test test_calo_rechit succeeded

I think we need to raise this as a separate issue (unless I misunderstood how this is supposed to work), but it's not a blocker for this PR as I have been able to verify the test runs locally, on top of the tests in the patatrack branch.

jpata · 2020-10-21T05:42:51Z

+reconstruction

introduces DataFormats/CaloRecHit which contains shared code for calorimeters for the patatrack integration
unit tests are present for a part of the code and have been verified to run independently
- more unit tests should be added in a follow-up, the corresponding issue is here unit tests for patatrack integration: MultifitComputations #31880
code checks and tests pass, so it won't break the build
minor review items (namespace & file names, commented-out code for fnnls) have been addressed
no reco differences are expected (this code does not run in any workflow) or observed
the patatrack branch validation is here and looks OK
this PR is urgent for CMSSW_11_2_X_pre8 in the timescale of today

cmsbuild · 2020-10-21T05:43: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. @silviodonato, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

silviodonato · 2020-10-21T07:05:53Z

+1
@jpata @fwyzard you might create an issue with the list of the "nice-to-have" features after the integration of each Patatrack PR (eg. EigenComputations unit tests)

cmsbuild · 2020-10-23T07:33:20Z

+1
Tested at: 8938c57
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-bad05c/10227/summary.html
CMSSW: CMSSW_11_2_X_2020-10-22-2300
SCRAM_ARCH: slc7_amd64_gcc820

cmsbuild · 2020-10-23T07:33:22Z

Comparison not run due to Build errors/Fireworks only changes/No short matrix requested (RelVals and Igprof tests were also skipped)

davidlange6 · 2020-10-27T13:42:57Z

looks likely that the clang build will complain about this PR once its new headers are used - we see this in the modules IB

/data/cmsbld/jenkins/workspace/build-any-ib/w/tmp/BUILDROOT/9faf547acdcdd0e319dedacbbb9eebbe/opt/cmssw/slc7_amd64_gcc820/cms/cmssw/CMSSW_11_2_CXXMODULE_X_2020-10-26-2300/src/DataFormats/CaloRecHit/interface/MultifitComputations.h:341:58: warning: '&&' within '||' [-Wlogical-op-parentheses]
if (w_max < eps || w_max_idx == w_max_idx_prev && w_max == w_max_prev)
~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~
/data/cmsbld/jenkins/workspace/build-any-ib/w/tmp/BUILDROOT/9faf547acdcdd0e319dedacbbb9eebbe/opt/cmssw/slc7_amd64_gcc820/cms/cmssw/CMSSW_11_2_CXXMODULE_X_2020-10-26-2300/src/DataFormats/CaloRecHit/interface/MultifitComputations.h:341:58: note: place parentheses around the '&&' expression to silence this warning
if (w_max < eps || w_max_idx == w_max_idx_prev && w_max == w_max_prev)
^
( )

cmsbuild added this to the CMSSW_11_2_X milestone Oct 7, 2020

cmsbuild added code-checks-pending comparison-pending new-package-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Oct 7, 2020

cmsbuild added code-checks-approved and removed code-checks-pending labels Oct 7, 2020

cmsbuild mentioned this pull request Oct 7, 2020

Patatrack integration - common data formats (5/N) #31703

Merged

cmsbuild added requires-external tests-started and removed tests-pending labels Oct 7, 2020

cmsbuild added tests-rejected and removed tests-started labels Oct 7, 2020

cmsbuild added comparison-available and removed comparison-pending labels Oct 7, 2020

fwyzard force-pushed the patatrack_integration_6_N_calorimeters_common branch from f6571a8 to d07d030 Compare October 7, 2020 21:53

cmsbuild removed code-checks-approved comparison-available labels Oct 7, 2020

cmsbuild added tests-approved and removed tests-started labels Oct 20, 2020

cmsbuild added comparison-available and removed comparison-pending labels Oct 20, 2020

cmsbuild added fully-signed reconstruction-approved and removed pending-signatures reconstruction-pending labels Oct 21, 2020

jpata mentioned this pull request Oct 21, 2020

GPU test does not seem to run in the bot #31878

Closed

cmsbuild added orp-approved and removed orp-pending labels Oct 21, 2020

cmsbuild merged commit cb16ffd into cms-sw:master Oct 21, 2020

jpata mentioned this pull request Oct 21, 2020

unit tests for patatrack integration: MultifitComputations #31880

Open

cmsbuild added comparison-notrun and removed comparison-available labels Oct 23, 2020

davidlange6 mentioned this pull request Oct 24, 2020

add missing includes #31932

Merged

mariadalfonso mentioned this pull request Oct 28, 2020

GPUcaloCommon: fix parentheses #31972

Merged

fwyzard mentioned this pull request Nov 16, 2020

GPU refactor common code from ECAL multifit and HCAL mahi #32144

Merged

jpata mentioned this pull request Nov 17, 2020

Relocate algos from DataFormats/CaloRecHit, clean up for coding rules #32159

Open

fwyzard deleted the patatrack_integration_6_N_calorimeters_common branch December 27, 2020 09:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Patatrack integration - calorimeters shared code (6/N) #31704

Patatrack integration - calorimeters shared code (6/N) #31704

fwyzard commented Oct 7, 2020 •

edited

cmsbuild commented Oct 7, 2020

fwyzard commented Oct 7, 2020

fwyzard commented Oct 7, 2020

cmsbuild commented Oct 7, 2020

cmsbuild commented Oct 7, 2020 •

edited

cmsbuild commented Oct 7, 2020

cmsbuild commented Oct 7, 2020

cmsbuild commented Oct 7, 2020

cmsbuild commented Oct 7, 2020

cmsbuild commented Oct 20, 2020

cmsbuild commented Oct 20, 2020

cmsbuild commented Oct 20, 2020

jpata commented Oct 21, 2020 •

edited

jpata commented Oct 21, 2020 •

edited

cmsbuild commented Oct 21, 2020

silviodonato commented Oct 21, 2020

cmsbuild commented Oct 23, 2020

cmsbuild commented Oct 23, 2020

davidlange6 commented Oct 27, 2020

Patatrack integration - calorimeters shared code (6/N) #31704

Patatrack integration - calorimeters shared code (6/N) #31704

Conversation

fwyzard commented Oct 7, 2020 • 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 Oct 7, 2020

fwyzard commented Oct 7, 2020

fwyzard commented Oct 7, 2020

cmsbuild commented Oct 7, 2020

cmsbuild commented Oct 7, 2020 • edited

cmsbuild commented Oct 7, 2020

cmsbuild commented Oct 7, 2020

cmsbuild commented Oct 7, 2020

cmsbuild commented Oct 7, 2020

cmsbuild commented Oct 20, 2020

cmsbuild commented Oct 20, 2020

cmsbuild commented Oct 20, 2020

jpata commented Oct 21, 2020 • edited

jpata commented Oct 21, 2020 • edited

cmsbuild commented Oct 21, 2020

silviodonato commented Oct 21, 2020

cmsbuild commented Oct 23, 2020

cmsbuild commented Oct 23, 2020

davidlange6 commented Oct 27, 2020

fwyzard commented Oct 7, 2020 •

edited

cmsbuild commented Oct 7, 2020 •

edited

jpata commented Oct 21, 2020 •

edited

jpata commented Oct 21, 2020 •

edited