Digi Morphing for HLT by Chirayu18 · Pull Request #48734 · cms-sw/cmssw

Chirayu18 · 2025-08-14T04:23:03Z

PR description:

Morphing Algorithm for Fake Digi Addition

This PR introduces an alternative alpaka-based algorithm for pixel digi morphing. This implementation is equivalent in principle to the legacy implementation and to the other alpaka-based implementation in PR #48343 but supercedes the latter as it has issues with throughput due to memory usage. Like the other implementations, digi morphing is applied here only to specific detector regions which can be configured as shown below.

Configuration Options

The morphing behavior can be controlled in the EDProducer configuration with the following options:

Enable Morphing:

DoDigiMorphing = cms.bool(True)  # Default: False

Maximum Fakes per Module:

MaxFakesInModule = cms.uint32(10000)  # Default: maxPixInModule * 2 / 5

Regional Morphing

Regions can be specified in the configuration as follows:

Barrel Regions:
Use LAYER,LADDER,MODULE coordinates, with support for individual values or ranges (e.g., 1-12 for ladders 1 through 12).

barrelRegions = cms.vstring(
    "1,1-12,1-2",
    "1,1-12,7-8",
    "2,1-28,1",
    "2,1-28,8"
)  # Default: as shown above

Endcap Regions:
Use DISK,BLADE,SIDE,PANEL coordinates, with support for ranges.

endcapRegions = cms.vstring(
    # Default: empty (no endcap regions selected)
)

PR validation:

Validation results can be found here: https://indico.cern.ch/event/1567986/contributions/6647897/attachments/3116797/5527819/HLT_digi_morphing_validation.pdf

cmsbuild · 2025-08-14T04:24:48Z

This PR contains too many commits (644 >= 240) and will not be processed.
Please ensure you have selected the correct target branch and consider squashing unnecessary commits.
The processing of this PR will resume once the commit count drops below the limit.

cmsbuild · 2025-08-14T05:26:21Z

cms-bot internal usage

cmsbuild · 2025-08-14T05:27:52Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-48734/45791

ERROR: Build errors found during clang-tidy run.

src/RecoLocalTracker/SiPixelClusterizer/plugins/alpaka/SiPixelRawToCluster.cc:376:43: error: too few arguments to function call, expected 12, have 11 [clang-diagnostic-error]
  366 |     Algo_.makePhase1ClustersAsync(iEvent.queue(),
      |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  367 |                                   clusterThresholds_,
--
gmake: *** [config/SCRAM/GMake/Makefile.coderules:129: code-checks] Error 2
gmake: *** [There are compilation/build errors. Please see the detail log above.] Error 2

fwyzard · 2025-08-14T06:30:21Z

please remove these changes

fwyzard · 2025-08-14T06:31:39Z

    }

+    ALPAKA_FN_ACC
+    bool isMorphingModule(uint32_t moduleId, const uint32_t* morphingModules, uint32_t nMorphingModules) {


if there are more than about 10 modules, it would be much more efficient to

sort the modules (once, on the cpu, in the constructor of the module)

use a binary search to check if a module is in the list

Thanks!

Could you make the implementation generic and move it to a central place, like HeterogeneousCore/AlpakaInterface/interface/alpakastdAlgorithm.h ?
You may be able to reuse some of the functionality from that file.

See also HeterogeneousCore/CUDAUtilities/interface/cudastdAlgorithm.h for the old CUDA-only version.

Has this been addressed?

No, but I guess it can be done later.

fwyzard · 2025-08-14T06:31:59Z

+    // FIXME: this is just an estimate, to be studied and optimised
+    //static constexpr uint32_t maxFakesInModule = TrackerTraits::maxPixInModule * 2 / 5;


this can be removed

fwyzard · 2025-08-14T06:35:35Z

        uint32_t thisModuleId = digi_view[firstPixel].moduleId();
+        uint32_t rawModuleId = digi_view[firstPixel].rawIdArr();
+        applyDigiMorphing =
+            applyDigiMorphing && pixelStatus::isMorphingModule(rawModuleId, morphingModules, nMorphingModules);


I think this is wrong: a kernel block may loop over different modules (line 196); once it encounters a module that is not in the list, it will change the applyDigiMorphing to false; at that point all other modules will be ignored.

fwyzard · 2025-08-14T06:35:49Z

    ALPAKA_FN_ACC void operator()(Acc1D const& acc,
                                  SiPixelDigisSoAView digi_view,
+                                  SiPixelDigisSoAView fakes_view,
+                                  bool applyDigiMorphing,


Suggested change

bool applyDigiMorphing,

bool enableDigiMorphing,

fwyzard · 2025-08-14T06:36:17Z

+        applyDigiMorphing =
+            applyDigiMorphing && pixelStatus::isMorphingModule(rawModuleId, morphingModules, nMorphingModules);


Suggested change

applyDigiMorphing =

applyDigiMorphing && pixelStatus::isMorphingModule(rawModuleId, morphingModules, nMorphingModules);

bool applyDigiMorphing =

enableDigiMorphing && pixelStatus::isMorphingModule(rawModuleId, morphingModules, nMorphingModules);

fwyzard · 2025-08-14T06:38:49Z

+        uint32_t rawModuleId = digi_view[firstPixel].rawIdArr();
+        applyDigiMorphing =
+            applyDigiMorphing && pixelStatus::isMorphingModule(rawModuleId, morphingModules, nMorphingModules);
+        //applyDigiMorphing = applyDigiMorphing && isMorphingModule;


this can be removed

fwyzard · 2025-08-14T06:41:46Z

+        std::vector<std::string> limits;
+        boost::split(limits, r, boost::is_any_of("-"));
+        try {
+          if (limits.size() > 1) {


maybe add a check that there are exactly 0 or 1 hyphens, and not more than 1 ?

cmsbuild · 2025-08-14T06:46:04Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-48734/45792

There are other open Pull requests which might conflict with changes you have proposed:
- File RecoLocalTracker/SiPixelClusterizer/plugins/alpaka/PixelClustering.h modified in PR(s): Pixelimage morphing #48343
- File RecoLocalTracker/SiPixelClusterizer/plugins/alpaka/SiPixelMorphingConfig.h modified in PR(s): Pixelimage morphing #48343
- File RecoLocalTracker/SiPixelClusterizer/plugins/alpaka/SiPixelRawToCluster.cc modified in PR(s): Pixelimage morphing #48343, Updated SoA View accessors from raw pointers to span #48377
- File RecoLocalTracker/SiPixelClusterizer/plugins/alpaka/SiPixelRawToClusterKernel.dev.cc modified in PR(s): Pixelimage morphing #48343, Updated SoA View accessors from raw pointers to span #48377
- File RecoLocalTracker/SiPixelClusterizer/plugins/alpaka/SiPixelRawToClusterKernel.h modified in PR(s): Pixelimage morphing #48343

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

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

fwyzard · 2025-08-14T06:49:09Z

+          auto morphingModules_d =
+              cms::alpakatools::make_device_buffer<uint32_t[]>(queue, digiMorphingConfig.morphingModules.size());
+          auto morphingModules_h = cms::alpakatools::make_host_view(digiMorphingConfig.morphingModules.data(),
+                                                                    digiMorphingConfig.morphingModules.size());
+          alpaka::memcpy(queue, morphingModules_d, morphingModules_h);


This is going to stay the same for the whole job.
Can you move the allocation of the buffer and the copy outside of the event loop ?
One option would be to use the cms::alpakatools::MoveToDeviceCache functionality (see the README.md).

Hi, I couln't manage to use MoveToDevice functionality as the struct I used in place of TestAlgo::UpdateInfo ( as done in the example) contains std::vector which is not trivially destructable and there are compilation issues with that.

Instead I moved this part to where the cabling map changes so that it's not repeated for every event.
Please let me know if this is fine or there is a better way

fwyzard · 2025-08-14T06:49:48Z

+#if 0
+      alpaka::exec<Acc1D>(queue,
+                          workDivMaxNumModules,
+                          FindClus<TrackerTraits>{},
+                          digis_view,
+                          unused.view(),
+                          clusters_d->view(),
+                          numDigis);
+#endif


this is clearly not OK 🤷🏻

fwyzard · 2025-08-14T06:55:03Z

+      tTopo_ = &iSetup.getData(trackerTopologyToken_);
+      digiMorphingConfig_.morphingModules.clear();
+      for (const auto& connection : cablingMap_->det2fedMap()) {
+        auto rawId = connection.first;
+        if (rawId == 0)
+          continue;
+        DetId detId(rawId);
+        if (!skipDetId(tTopo_, detId, theBarrelRegions_, theEndcapRegions_)) {
+          digiMorphingConfig_.morphingModules.push_back(rawId);
+        }
+      }


(sorry, it is already done only if there are changes, ignore my previous comment)

However, the tracker topology is in a different record (TrackerTopologyRcd) than the cabling map (SiPixelFedCablingMapRcd), so the check needs to be split ?

Sorry, I am not sure what you mean by the check needs to be split... Could you please clarify?

cmsbuild · 2025-09-01T19:33:57Z

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

fwyzard · 2025-09-01T21:36:06Z

please test

mmusich · 2025-09-02T15:17:06Z

tests here seem stuck. Are we expecting them to finish in a finite amount of time?

iarspider · 2025-09-02T15:38:23Z

@mmusich I have restarted the unit test

cmsbuild · 2025-09-02T15:39:04Z

+1

Size: This PR adds an extra 24KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-da6630/47935/summary.html
COMMIT: 386b44c
CMSSW: CMSSW_15_1_X_2025-09-01-1100/el8_amd64_gcc12
Additional Tests: GPU,AMD_MI300X,NVIDIA_T4
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/48734/47935/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially added 2131218 lines to the logs
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 50
DQMHistoTests: Total histograms compared: 4115479
DQMHistoTests: Total failures: 29
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 4115430
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

AMD_MI300X Comparison Summary

Summary:

You potentially added 10 lines to the logs
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 7
DQMHistoTests: Total histograms compared: 53486
DQMHistoTests: Total failures: 8350
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 45136
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

NVIDIA_T4 Comparison Summary

Summary:

You potentially added 10 lines to the logs
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 7
DQMHistoTests: Total histograms compared: 53486
DQMHistoTests: Total failures: 8930
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 44556
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

mmusich · 2025-09-04T09:32:30Z

@cms-sw/heterogeneous-l2 @cms-sw/reconstruction-l2 do you have any other comments and / or requests for this?

fwyzard · 2025-09-04T14:14:54Z

I will not be able to re-check the code in the near future, so let's assume that all comments have been addressed 🤷🏻‍♂️

fwyzard · 2025-09-04T14:14:59Z

+heterogeneous

jfernan2 · 2025-09-04T16:13:53Z

+1

cmsbuild · 2025-09-04T16:14:12Z

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. @mandrenguyen, @sextonkennedy, @ftenchini (and backports should be raised in the release meeting by the corresponding L2)

mandrenguyen · 2025-09-05T15:23:59Z

+1
Looking thru the review, it seems that are still some improvements that can be made to the implementation, but that they can be followed up in subsequent pull requests. Let's integrate this now to facilitate further testing before this is actually deployed in the HLT menu. As is, this PR has no effect on production workflows.

mmusich · 2025-09-09T07:52:13Z

I suspect this PR is causing failures in the GPU relvals since when it was merged in CMSSW_15_1_X_2025-09-05-2300.
See e.g. this log.

Also FWIW, running the addOnTests.py -t hlt_mc_HIon,hlt_data_HIon on lxplus-gpu in CMSSW_15_1_0_pre6 (without this PR), runs fine, whereas in the latest IB (such as CMSSW_15_1_X_2025-09-08-2300) they fail with e.g.:

----- Begin Fatal Exception 09-Sep-2025 09:49:11 CEST-----------------------
An exception of category 'StdException' occurred while
   [0] Processing  Event run: 362321 lumi: 231 event: 111754586 stream: 2
   [1] Running path 'HLT_HIUPC_MinPixelThrust0p8_MaxPixelCluster10000_v4'
   [2] Calling method for module SiPixelRawToClusterHIonPhase1@alpaka/'hltSiPixelClustersPPOnAASoA'
Exception Message:
A std::exception was thrown.
/data/cmsbld/jenkins/workspace/build-any-ib/w/el9_amd64_gcc12/external/alpaka/1.2.0-ca398df5d9e013dd3373eebb7be69807/include/alpaka/event/EventUniformCudaHipRt.hpp(143) 'ret = TApi::eventQuery(event.getN\
ativeHandle())' returned error  : 'cudaErrorLaunchFailure': 'unspecified launch failure'!
----- End Fatal Exception -------------------------------------------------

mmusich · 2025-09-09T08:04:51Z

Also:

You potentially added 2131218 lines to the logs

doesn't look very sane.

mmusich · 2025-09-10T09:38:20Z

+        trackerTopologyToken_(esConsumes<TrackerTopology, TrackerTopologyRcd>()),
        includeErrors_(iConfig.getParameter<bool>("IncludeErrors")),
        useQuality_(iConfig.getParameter<bool>("UseQualityInfo")),
-        verbose_(iConfig.getParameter<bool>("verbose")),


this is undoing #48494, why?

see #48890.

Thanks a lot.

ferencek · 2025-09-10T09:46:14Z

Also:

You potentially added 2131218 lines to the logs

doesn't look very sane.

This is because this PR has unintentionally undone #48494. To be fixed.

cmsbuild added this to the CMSSW_15_1_X milestone Aug 14, 2025

Implement digi morphing

39527b8

Chirayu18 force-pushed the alternate_implementation_v2 branch from cf734a9 to 69c506d Compare August 14, 2025 05:25

cmsbuild added reconstruction-pending core-pending pending-signatures tests-pending orp-pending code-checks-pending trk labels Aug 14, 2025

cmsbuild added code-checks-rejected and removed code-checks-pending labels Aug 14, 2025

fwyzard reviewed Aug 14, 2025

View reviewed changes

fwyzard suggested changes Aug 14, 2025

View reviewed changes

fwyzard reviewed Aug 14, 2025

View reviewed changes

cmsbuild added code-checks-pending and removed code-checks-rejected labels Aug 14, 2025

cmsbuild added code-checks-rejected and removed code-checks-pending labels Aug 14, 2025

fwyzard reviewed Aug 14, 2025

View reviewed changes

cmsbuild added tests-started and removed tests-pending labels Sep 1, 2025

This was referenced Sep 2, 2025

[15_0_X] Digi Morphing for HLT #48831

Closed

[15_0_X] Digi Morphing for HLT #48832

Merged

cmsbuild added tests-approved and removed tests-started labels Sep 2, 2025

cmsbuild added heterogeneous-approved and removed heterogeneous-pending labels Sep 4, 2025

cmsbuild added reconstruction-approved and removed reconstruction-pending pending-signatures labels Sep 4, 2025

mmusich mentioned this pull request Sep 10, 2025

[GPU] Relval 160.03502: SIGSEGV in SiPixelDigisClustersFromSoAAlpaka::produce #48885

Closed

mmusich reviewed Sep 10, 2025

View reviewed changes

This was referenced Sep 10, 2025

silence SiPixelRawToCluster verbose ROC and FED error printouts (again) #48890

Merged

[15.1.X] silence SiPixelRawToCluster verbose ROC and FED error printouts (again) #48891

Merged

ferencek mentioned this pull request Oct 6, 2025

Enabling SiPixel digi morphing from 2025 onward #49065

Merged

		// FIXME: this is just an estimate, to be studied and optimised
		//static constexpr uint32_t maxFakesInModule = TrackerTraits::maxPixInModule * 2 / 5;

		applyDigiMorphing =
		applyDigiMorphing && pixelStatus::isMorphingModule(rawModuleId, morphingModules, nMorphingModules);

Conversation

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

Uh oh!

PR description:

Morphing Algorithm for Fake Digi Addition

Configuration Options

Regional Morphing

PR validation:

Uh oh!

cmsbuild commented Aug 14, 2025

Uh oh!

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

Uh oh!

Uh oh!

cmsbuild commented Aug 14, 2025

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!

fwyzard Aug 25, 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!

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 Aug 14, 2025

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 Sep 1, 2025

Uh oh!

fwyzard commented Sep 1, 2025

Uh oh!

mmusich commented Sep 2, 2025

Uh oh!

iarspider commented Sep 2, 2025

Uh oh!

Chirayu18 commented Aug 14, 2025 •

edited

Loading

cmsbuild commented Aug 14, 2025 •

edited

Loading

fwyzard Aug 25, 2025 •

edited

Loading

fwyzard commented Sep 4, 2025 •

edited

Loading

mmusich commented Sep 9, 2025 •

edited

Loading