cmssw icon indicating copy to clipboard operation
cmssw copied to clipboard
verified publisher icon cms-sw

Pixelimage morphing

Open Chirayu18 opened this issue 6 months ago • 16 comments

PR description: This PR implements a new data format for the clustering algorithm for the pixel digis. The new data format is a densely populated 2D image as compared to the 1D sparsely populated digi_view. Also this PR can optionally apply the morphing algorithm for adding fake digis before doing the clustering. This morphing feature proceeds with a dilation step followed by an erosion. In the config these kernels are defined as a 1D array which store the matrix elements for the 3x3 kernels. The following options can now be set in the EDProducer config.: DoDigiMorphing = cms.bool(True), #Default False DigiMorphing = cms.PSet( kernel1 = cms.vint32(1, 1, 1, 1,1,1,1,1,1), # Example kernel values for the dilation kernel kernel2 = cms.vint32(0, 1, 0, 1,1,1,0,1,1), # Example kernel values for the erosion kernel )

Implementation details:

  • SiPixelMorphing.cc defined for storing configuration of morphing parameters
  • Two ImageSoA’s types are now available and depending on the doDigiMorphing flag, either one is used. For no morphing, the SoA with single layer padding is used and for morphing, 2 layer padding. This saves memory consumption in case morphing is disabled
  • The makePhase1ClustersAsync function is now templated with ImageType and TrackerTraits(default) and all templated function definitions are defined in SiPixelRawToClusterKernel.dev.cc file
  • The templated Image is passed to the FindClus kernel which is also templated with ImageType

PR validation: The code compiles. Comparisions with the legacy CPU code and the current GPU code were also made here: https://indico.cern.ch/event/1555234/contributions/6567409/attachments/3087292/5466320/digimorphing_2%20(1).pdf

Chirayu18 avatar Jun 17 '25 12:06 Chirayu18

cms-bot internal usage

cmsbuild avatar Jun 17 '25 12:06 cmsbuild

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-48343/45226

  • 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): #48320
    • File RecoLocalTracker/SiPixelClusterizer/plugins/alpaka/SiPixelRawToClusterKernel.dev.cc modified in PR(s): #47611, #48320

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-48343/45226/code-format.patch e.g. curl -k https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-48343/45226/code-format.patch | patch -p1 You can also run scram build code-format to apply code format directly

cmsbuild avatar Jun 17 '25 12:06 cmsbuild

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-48343/45227

  • 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): #48320
    • File RecoLocalTracker/SiPixelClusterizer/plugins/alpaka/SiPixelRawToClusterKernel.dev.cc modified in PR(s): #47611, #48320

cmsbuild avatar Jun 17 '25 12:06 cmsbuild

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

It involves the following packages:

  • RecoLocalTracker/SiPixelClusterizer (reconstruction)

@cmsbuild, @jfernan2, @mandrenguyen can you please review it and eventually sign? Thanks. @GiacomoSguazzoni, @VinInn, @VourMa, @dkotlins, @felicepantaleo, @ferencek, @gpetruc, @missirol, @mmusich, @mroguljic, @mtosi, @rovere, @threus, @tsusa, @tvami 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

cmsbuild avatar Jun 17 '25 12:06 cmsbuild

assign heterogeneous

mmusich avatar Jun 17 '25 12:06 mmusich

New categories assigned: heterogeneous

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

cmsbuild avatar Jun 17 '25 12:06 cmsbuild

test parameters:

  • enable = gpu, hlt_p2_timing

mmusich avatar Jun 22 '25 07:06 mmusich

@cmsbuild, please test

mmusich avatar Jun 22 '25 07:06 mmusich

+1

Size: This PR adds an extra 16KB to repository Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-e35cdb/46865/summary.html COMMIT: 94f139468b7eae505735e8032c0faee4b159da4a CMSSW: CMSSW_15_1_X_2025-06-22-0000/el8_amd64_gcc12 User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/48343/46865/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • You potentially removed 1 lines from the logs
  • Reco comparison results: 2 differences found in the comparisons
  • DQMHistoTests: Total files compared: 50
  • DQMHistoTests: Total histograms compared: 4058157
  • DQMHistoTests: Total failures: 10560
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 4047577
  • 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: found differences in 2 / 48 workflows

cmsbuild avatar Jun 22 '25 13:06 cmsbuild

+1

Looks like the bot didn't parse the commands at https://github.com/cms-sw/cmssw/pull/48343#issuecomment-2994008267

mmusich avatar Jun 22 '25 15:06 mmusich

enable gpu

mmusich avatar Jun 22 '25 15:06 mmusich

test parameters:

  • enable = hlt_p2_timing

mmusich avatar Jun 22 '25 15:06 mmusich

@cmsbuild, please test

mmusich avatar Jun 22 '25 15:06 mmusich

+1

Size: This PR adds an extra 16KB to repository Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-e35cdb/46869/summary.html COMMIT: 94f139468b7eae505735e8032c0faee4b159da4a CMSSW: CMSSW_15_1_X_2025-06-22-0000/el8_amd64_gcc12 Additional Tests: HLT_P2_TIMING User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/48343/46869/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: 6 differences found in the comparisons
  • DQMHistoTests: Total files compared: 50
  • DQMHistoTests: Total histograms compared: 4058157
  • DQMHistoTests: Total failures: 10531
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 4047606
  • 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: found differences in 2 / 48 workflows

cmsbuild avatar Jun 22 '25 17:06 cmsbuild

+1

jfernan2 avatar Jun 23 '25 15:06 jfernan2

-heterogeneous

based on the performance presented on June 16th 2025

fwyzard avatar Jun 23 '25 16:06 fwyzard

Apparently the GPU tests didn't run anyway since they get overwritten with the test parameters options. Capture d’écran 2025-06-27 à 15 29 50

AdrianoDee avatar Jun 27 '25 13:06 AdrianoDee

enable gpu

AdrianoDee avatar Jun 27 '25 13:06 AdrianoDee

please test (to see the effect on GPU wfs)

AdrianoDee avatar Jun 27 '25 13:06 AdrianoDee

please abort

AdrianoDee avatar Jun 27 '25 13:06 AdrianoDee

test parameters:

AdrianoDee avatar Jun 27 '25 13:06 AdrianoDee

enable gpu

AdrianoDee avatar Jun 27 '25 13:06 AdrianoDee

please test (I'm not sure this is the right way to do this, but it's true the GPU tests were not run)

AdrianoDee avatar Jun 27 '25 13:06 AdrianoDee

Apparently the GPU tests didn't run anyway since they get overwritten with the test parameters options.

actually I noticed and meant to ask @smuzaffar or @iarspider about it. What's the right way to enable "gpu" and "hlt_p2_timing" at the same time?

mmusich avatar Jun 27 '25 13:06 mmusich

both enable gpu,hlt_p2_timing comment or test parameters: comment with enable = gpu,hlt_p2_timing option should allow to run gpu and hlt_p2_timing tests

smuzaffar avatar Jun 27 '25 13:06 smuzaffar

both enable gpu,hlt_p2_timing comment or test parameters: comment with enable = gpu,hlt_p2_timing option should allow to run gpu and hlt_p2_timing tests

this didn't work at https://github.com/cms-sw/cmssw/pull/48343#issuecomment-2994008267.

mmusich avatar Jun 27 '25 13:06 mmusich

+1

Size: This PR adds an extra 16KB to repository Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-e35cdb/46969/summary.html COMMIT: 94f139468b7eae505735e8032c0faee4b159da4a CMSSW: CMSSW_15_1_X_2025-06-26-2300/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/48343/46969/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

CUDA Comparison Summary

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: 53212
  • DQMHistoTests: Total failures: 1071
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 52141
  • 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

ROCM 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: 53212
  • DQMHistoTests: Total failures: 39
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 53173
  • 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

cmsbuild avatar Jun 27 '25 16:06 cmsbuild

@Chirayu18 could you synchronise this branch with yours ?

fwyzard avatar Jul 15 '25 07:07 fwyzard

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-48343/45508

ERROR: Unable to merge PR.

See log https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-48343/45508/cms-checkout-topic.log

cmsbuild avatar Jul 15 '25 14:07 cmsbuild

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-48343/45509

  • There are other open Pull requests which might conflict with changes you have proposed:
    • File RecoLocalTracker/SiPixelClusterizer/plugins/alpaka/SiPixelRawToCluster.cc modified in PR(s): #48377
    • File RecoLocalTracker/SiPixelClusterizer/plugins/alpaka/SiPixelRawToClusterKernel.dev.cc modified in PR(s): #48377

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-48343/45509/code-format.patch e.g. curl -k https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-48343/45509/code-format.patch | patch -p1 You can also run scram build code-format to apply code format directly

cmsbuild avatar Jul 15 '25 14:07 cmsbuild