Ecal Phase2 Weights-based Reconstruction, Alpaka integration #47124

Jakub-Gajownik · 2025-01-17T13:49:32Z

PR description:

Migration of ECAL weights amplitude reconstruction to ALPAKA framework from CUDA.
Time (jitter) reconstruction has been added, to match with the existing CPU-based reconstruction.
The .612 workflow has been updated to use Alpaka modifier instead of the gpu.

PR validation:

The existing 24034.612 has been ran to validate the results of the reconstruction against the existing CPU and CUDA-based reconstructions. The tests have been performed using a sample of 8900 events.
A perfect agreement with the CUDA code has been achieved. A small deviation of the order of e(-22) on the mean has been observed when comparing the reconstructed amplitudes of Alpaka GPU code with the CPU, which fits the observations from the other studies. A perfect agreement of the Portable CPU with the native CPU has additionally been observed.
The 29634.612 has been successfully.

cmsbuild · 2025-01-17T13:50:01Z

cms-bot internal usage

cmsbuild · 2025-01-17T13:51:44Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-47124/43345

Found files with invalid states:
- RecoLocalCalo/EcalRecProducers/plugins/EcalUncalibRecHitConvertPortable2CPUFormat.cc:
  - Added: 4fb1a54
  - Deleted: 59d43bb
There are other open Pull requests which might conflict with changes you have proposed:
- File RecoLocalCalo/EcalRecProducers/python/ecalUncalibRecHitPhase2_cff.py modified in PR(s): Remove the configuration of the legacy CUDA workflows #46076

cmsbuild · 2025-01-17T13:52:01Z

A new Pull Request was created by @Jakub-Gajownik for master.

It involves the following packages:

RecoLocalCalo/EcalRecProducers (reconstruction)

@cmsbuild, @jfernan2, @mandrenguyen can you please review it and eventually sign? Thanks.
@ReyerBand, @apsallid, @argiro, @denizsun, @missirol, @rchatter, @salimcerci, @thomreis, @wang0jin, @youyingli this is something you requested to watch as well.
@antoniovilela, @mandrenguyen, @rappoccio, @sextonkennedy you are the release manager for this.

cms-bot commands are listed here

Jakub-Gajownik · 2025-01-17T13:53:13Z

type ecal

Jakub-Gajownik · 2025-01-17T13:54:09Z

enable gpu

thomreis · 2025-01-17T13:57:57Z

enable gpu

thomreis · 2025-01-17T14:00:13Z

please test

cmsbuild · 2025-01-17T16:25:42Z

+1

Size: This PR adds an extra 32KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-2405fb/43825/summary.html
COMMIT: de7ec1c
CMSSW: CMSSW_15_0_X_2025-01-16-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/47124/43825/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: 49
DQMHistoTests: Total histograms compared: 3819085
DQMHistoTests: Total failures: 82
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3818983
DQMHistoTests: Total skipped: 20
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
Checked 214 log files, 184 edm output root files, 49 DQM output files
TriggerResults: no differences found

GPU Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 7
DQMHistoTests: Total histograms compared: 53071
DQMHistoTests: Total failures: 33
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 53038
DQMHistoTests: Total skipped: 0
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 6 files compared)
Checked 24 log files, 30 edm output root files, 7 DQM output files
TriggerResults: no differences found

jfernan2 · 2025-01-17T16:36:45Z

assign heterogeneous

cmsbuild · 2025-01-17T16:37:07Z

New categories assigned: heterogeneous

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

jfernan2 · 2025-01-22T10:28:59Z

+1

makortel

Is this code tested in any workflow?

makortel · 2025-01-22T16:47:24Z

RecoLocalCalo/EcalRecProducers/plugins/alpaka/EcalPhase2DigiToPortableProducer.cc

+  EcalPhase2DigiToPortableProducer::EcalPhase2DigiToPortableProducer(edm::ParameterSet const &ps)
+      : inputDigiToken_(consumes<EBDigiCollectionPh2>(ps.getParameter<edm::InputTag>("BarrelDigis"))),


We are in a process of requiring the Alpaka-based EDProducers to pass the edm::ParameterSet to the base class constructor (with the goal of preventing base class default constructor in 15_0_0_pre3). It would be helpful to do that already in this PR. I'd suggest to rebase to 15_0_0_pre2 (or any IB of this year) and

Suggested change

EcalPhase2DigiToPortableProducer::EcalPhase2DigiToPortableProducer(edm::ParameterSet const &ps)

: inputDigiToken_(consumes<EBDigiCollectionPh2>(ps.getParameter<edm::InputTag>("BarrelDigis"))),

EcalPhase2DigiToPortableProducer::EcalPhase2DigiToPortableProducer(edm::ParameterSet const &ps)

: EDProducer<>(ps),

inputDigiToken_(consumes<EBDigiCollectionPh2>(ps.getParameter<edm::InputTag>("BarrelDigis"))),

makortel · 2025-01-22T16:53:38Z

RecoLocalCalo/EcalRecProducers/plugins/alpaka/EcalPhase2DigiToPortableProducer.cc

+    digisHostCollView.size() = i;
+
+    //copy collection from host to device
+    alpaka::memcpy(event.queue(), digisDevColl.buffer(), digisHostColl.buffer());


Since this EDProducer only copies data from host to device (without any kernels), in 15_0_0_pre2 this explicit copy from host to device could be avoided by the EDProducer producing the EcalDigiPhase2HostCollection as a host-side data product (i.e. with edm::EDPutTokenT). The framework would then implicitly copy that to the device (see https://github.com/cms-sw/cmssw/blob/80d1847/HeterogeneousCore/AlpakaCore/README.md#edproducer for more details).

Could you clarify whether the Digi producer in this case becomes a normal edm::global producer and should be moved outside of the Alpaka namespace?

It should still be an alapaka-based producer in the ALPAKA_ACCELERATOR_NAMESPACE namespace.

See EventFilter/HcalRawToDigi/plugins/alpaka/HcalDigisSoAProducer.cc for an example.

makortel · 2025-01-22T16:54:47Z

RecoLocalCalo/EcalRecProducers/plugins/alpaka/EcalUncalibRecHitPhase2WeightsAlgoPortable.dev.cc

+
+#include "DataFormats/EcalDigi/interface/alpaka/EcalDigiPhase2DeviceCollection.h"
+#include "DataFormats/EcalRecHit/interface/alpaka/EcalUncalibratedRecHitDeviceCollection.h"
+#include <alpaka/alpaka.hpp>


It would be more clear to have the #includes of external packages and CMSSW headers in separate "blocks", and order the CMSSW headers in alphabetical order.

makortel · 2025-01-22T16:57:07Z

RecoLocalCalo/EcalRecProducers/plugins/alpaka/EcalUncalibRecHitPhase2WeightsAlgoPortable.dev.cc

+        template <typename TAcc>
+        ALPAKA_FN_ACC void operator()(TAcc const &acc,


Could be

Suggested change

template <typename TAcc>

ALPAKA_FN_ACC void operator()(TAcc const &acc,

ALPAKA_FN_ACC void operator()(Acc1D const &acc,

makortel · 2025-01-22T16:58:01Z

RecoLocalCalo/EcalRecProducers/plugins/alpaka/EcalUncalibRecHitPhase2WeightsAlgoPortable.dev.cc

+      };
+
+      template <typename TAcc>
+      ALPAKA_FN_ACC void Phase2WeightsKernel::operator()(


There would be less repetition (of function arguments) if the implementation would be at the function declaration few lines above.

makortel · 2025-01-22T23:17:57Z

RecoLocalCalo/EcalRecProducers/python/ecalUncalibRecHitPhase2Portable_cfi.py

@@ -0,0 +1,2 @@
+import  RecoLocalCalo.EcalRecProducers.ecalUncalibRecHitPhase2WeightsProducerPortable_cfi as _mod
+ecalUncalibRecHitPhase2Portable = _mod.ecalUncalibRecHitPhase2WeightsProducerPortable.clone()


What is the purpose of this file? I mean, is it useful to have both ecalUncalibRecHitPhase2WeightsProducerPortable_cfi and ecalUncalibRecHitPhase2Portable_cfi?

makortel · 2025-01-22T23:38:15Z

RecoLocalCalo/EcalRecProducers/plugins/alpaka/EcalPhase2DigiToPortableProducer.cc

+
+  private:
+    const edm::EDGetTokenT<EBDigiCollectionPh2> inputDigiToken_;
+    const device::EDPutToken<EcalDigiPhase2DeviceCollection> outputDigiDevToken_;


Just out of curiousity, does the EcalDigiPhase2DeviceCollection have other potential use than the input for EcalUncalibRecHitPhase2WeightsProducerPortable, or do you foresee other alternative modules that would produce EcalDigiPhase2DeviceCollection as well?

I'm asking only because if there is high certainty that this and EcalUncalibRecHitPhase2WeightsProducerPortable will always come together, and nothing else won't use EcalDigiPhase2DeviceCollection (or EcalDigiPhase2HostCollection), then these two modules could be merged into one module (that would lead to a somewhat simpler setup especially in the configuration). I'm not suggesting to change the setup in this PR, but only to give an aspect to think about.

Yes, we have other modules in mind. The weights producer is a baseline, but we are thinking of porting the multifit as well.

makortel · 2025-01-22T23:38:37Z

RecoLocalCalo/EcalRecProducers/plugins/alpaka/EcalPhase2DigiToPortableProducer.cc

+
+namespace ALPAKA_ACCELERATOR_NAMESPACE {
+
+  class EcalPhase2DigiToPortableProducer : public stream::EDProducer<> {


Looks like this module could be very easily made global::EDProducer<>.

makortel · 2025-01-22T23:39:35Z

RecoLocalCalo/EcalRecProducers/plugins/alpaka/EcalUncalibRecHitPhase2WeightsAlgoPortable.dev.cc

+namespace ALPAKA_ACCELERATOR_NAMESPACE {
+  namespace ecal {
+    namespace weights {


These could be expressed as

Suggested change

namespace ALPAKA_ACCELERATOR_NAMESPACE {

namespace ecal {

namespace weights {

namespace ALPAKA_ACCELERATOR_NAMESPACE::ecal::weights {

in order to have shorter indentation in the code below.

makortel · 2025-01-22T23:51:31Z

RecoLocalCalo/EcalRecProducers/plugins/alpaka/EcalUncalibRecHitPhase2WeightsAlgoPortable.dev.cc

+          double const *timeWeightsdata,
+          EcalDigiPhase2DeviceCollection::ConstView digisDev,
+          EcalUncalibratedRecHitDeviceCollection::View uncalibratedRecHitsDev) const {
+        constexpr int nsamples = EcalDataFrame_Ph2::MAXSAMPLES;


I'd suggest to use either ecalPh2::sampleSize or EcalDataFrame_Ph2::MAXSAMPLES (I see the latter is defined to be the same as the former) both here and where weightsData and timeWeightsdata are allocated/defined so that a future reader can easily convince themselves that the loop below does not go over the bounds of weightsData/timeWeightsdata.

Change the UncalibratedRecHitsProducer to a global producer Run code-checks and code-format

cmsbuild · 2025-04-19T23:03:09Z

-1

Failed Tests: RelVals-CUDA
Size: This PR adds an extra 16KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-2405fb/45649/summary.html
COMMIT: e8bf0bd
CMSSW: CMSSW_15_1_X_2025-04-19-1100/el8_amd64_gcc12
Additional Tests: CUDA,ROCM
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/47124/45649/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals-CUDA

12834.40612834.406_TTbar_14TeV+2024_Patatrack_PixelOnlyTripletsAlpaka/step3_TTbar_14TeV+2024_Patatrack_PixelOnlyTripletsAlpaka.log
12834.40312834.403_TTbar_14TeV+2024_Patatrack_PixelOnlyAlpaka_Validation/step3_TTbar_14TeV+2024_Patatrack_PixelOnlyAlpaka_Validation.log
12834.40212834.402_TTbar_14TeV+2024_Patatrack_PixelOnlyAlpaka/step3_TTbar_14TeV+2024_Patatrack_PixelOnlyAlpaka.log

Comparison Summary

Summary:

You potentially added 5 lines to the logs
Reco comparison results: 2 differences found in the comparisons
DQMHistoTests: Total files compared: 50
DQMHistoTests: Total histograms compared: 3916361
DQMHistoTests: Total failures: 49
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3916292
DQMHistoTests: Total skipped: 20
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 49 files compared)
Checked 215 log files, 184 edm output root files, 50 DQM output files
TriggerResults: no differences found

ROCM Comparison Summary

Summary:

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

fwyzard · 2025-04-22T05:39:10Z

ignore tests-rejected with ib-failure

fwyzard · 2025-04-22T05:53:26Z

I've opened #47918 to track the follow-up issues discussed here and elsewhere.

fwyzard · 2025-04-22T05:53:38Z

+heterogeneous

fwyzard · 2025-04-22T05:54:07Z

For me it's fine to work on the follow up issues in separate PRs.
Of course, if you prefer to fix some of them already here, that is fine as well.

jfernan2 · 2025-04-22T09:52:49Z

+1

Moanwar · 2025-04-22T10:01:02Z

+Upgrade

AdrianoDee · 2025-04-23T08:22:39Z

+pdmv

cmsbuild · 2025-04-23T08:23:05Z

This pull request is fully signed and it will be integrated in one of the next master IBs (test failures were overridden). This pull request will now be reviewed by the release team before it's merged. @mandrenguyen, @rappoccio, @sextonkennedy, @antoniovilela (and backports should be raised in the release meeting by the corresponding L2)

mandrenguyen · 2025-04-23T17:13:07Z

+1

cmsbuild added this to the CMSSW_15_0_X milestone Jan 17, 2025

cmsbuild added reconstruction-pending pending-signatures tests-pending orp-pending code-checks-pending labels Jan 17, 2025

cmsbuild added code-checks-approved and removed code-checks-pending labels Jan 17, 2025

cmsbuild added the ecal label Jan 17, 2025

cmsbuild added tests-started and removed tests-pending labels Jan 17, 2025

cmsbuild added tests-approved and removed tests-started labels Jan 17, 2025

cmsbuild added the heterogeneous-pending label Jan 17, 2025

cmsbuild added reconstruction-approved and removed reconstruction-pending labels Jan 22, 2025

makortel reviewed Jan 22, 2025

View reviewed changes

Jakub-Gajownik added 2 commits January 29, 2025 16:00

Add RecoLocalCalo Phase2 Weights amplitude portable reconstruction

b5e933d

Change the UncalibratedRecHitsProducer to a global producer Run code-checks and code-format

Fixed configuration file error

6b882e0

cmsbuild added tests-rejected and removed tests-started labels Apr 19, 2025

cmsbuild added tests-approved tests-ib-failure and removed tests-rejected labels Apr 22, 2025

fwyzard mentioned this pull request Apr 22, 2025

follow up to ECAL Phase 2 alpaka developments #47918

Open

4 tasks

cmsbuild added heterogeneous-approved and removed heterogeneous-pending labels Apr 22, 2025

cmsbuild added reconstruction-approved and removed reconstruction-pending labels Apr 22, 2025

cmsbuild added upgrade-approved and removed upgrade-pending labels Apr 22, 2025

cmsbuild added fully-signed pdmv-approved and removed pending-signatures pdmv-pending labels Apr 23, 2025

cmsbuild added orp-approved and removed orp-pending labels Apr 23, 2025

cmsbuild merged commit 88ee799 into cms-sw:master Apr 23, 2025
16 of 17 checks passed

cmsbuild mentioned this pull request Apr 23, 2025

Refactor PathsAndConsumesOfModules #47937

Merged

		EcalPhase2DigiToPortableProducer::EcalPhase2DigiToPortableProducer(edm::ParameterSet const &ps)
		: inputDigiToken_(consumes<EBDigiCollectionPh2>(ps.getParameter<edm::InputTag>("BarrelDigis"))),

		template <typename TAcc>
		ALPAKA_FN_ACC void operator()(TAcc const &acc,

	template <typename TAcc>
	ALPAKA_FN_ACC void operator()(TAcc const &acc,
	ALPAKA_FN_ACC void operator()(Acc1D const &acc,

		@@ -0,0 +1,2 @@
		import RecoLocalCalo.EcalRecProducers.ecalUncalibRecHitPhase2WeightsProducerPortable_cfi as _mod
		ecalUncalibRecHitPhase2Portable = _mod.ecalUncalibRecHitPhase2WeightsProducerPortable.clone()


		namespace ALPAKA_ACCELERATOR_NAMESPACE {

		class EcalPhase2DigiToPortableProducer : public stream::EDProducer<> {

Ecal Phase2 Weights-based Reconstruction, Alpaka integration #47124

Ecal Phase2 Weights-based Reconstruction, Alpaka integration #47124

Uh oh!

Conversation

Jakub-Gajownik commented Jan 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR description:

PR validation:

Uh oh!

cmsbuild commented Jan 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cmsbuild commented Jan 17, 2025

Uh oh!

cmsbuild commented Jan 17, 2025

Uh oh!

Jakub-Gajownik commented Jan 17, 2025

Uh oh!

Jakub-Gajownik commented Jan 17, 2025

Uh oh!

thomreis commented Jan 17, 2025

Uh oh!

thomreis commented Jan 17, 2025

Uh oh!

cmsbuild commented Jan 17, 2025

Comparison Summary

GPU Comparison Summary

Uh oh!

jfernan2 commented Jan 17, 2025

Uh oh!

cmsbuild commented Jan 17, 2025

Uh oh!

jfernan2 commented Jan 22, 2025

Uh oh!

makortel left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fwyzard Mar 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cmsbuild commented Apr 19, 2025

RelVals-CUDA

Comparison Summary

ROCM Comparison Summary

Uh oh!

fwyzard commented Apr 22, 2025

Uh oh!

fwyzard commented Apr 22, 2025

Uh oh!

fwyzard commented Apr 22, 2025

Uh oh!

fwyzard commented Apr 22, 2025

Uh oh!

jfernan2 commented Apr 22, 2025

Uh oh!

Jakub-Gajownik commented Jan 17, 2025 •

edited

Loading

cmsbuild commented Jan 17, 2025 •

edited

Loading

fwyzard Mar 14, 2025 •

edited

Loading