Pixelimage morphing#48343
Conversation
|
cms-bot internal usage |
|
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-48343/45226
Code check has found code style and quality issues which could be resolved by applying following patch(s)
|
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-48343/45227
|
|
A new Pull Request was created by @Chirayu18 for master. It involves the following packages:
@cmsbuild, @jfernan2, @mandrenguyen can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-48343/45518
|
|
please test |
|
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-48343/45529
Code check has found code style and quality issues which could be resolved by applying following patch(s)
|
|
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-48343/45553
Code check has found code style and quality issues which could be resolved by applying following patch(s)
|
| alpaka::syncBlockThreads(acc); | ||
|
|
||
| for (uint32_t i : cms::alpakatools::independent_group_elements(acc, firstPixel, lastPixel - 1)) { | ||
| alpaka::syncBlockThreads(acc); |
There was a problem hiding this comment.
you don;t need to syncBlockThreads one after the other
| alpaka::syncBlockThreads(acc); |
There was a problem hiding this comment.
Yes, will commit this change
| for (uint32_t j : cms::alpakatools::independent_group_elements(acc, morphingsize)) { | ||
| uint16_t row = j / (pixelStatus::pixelSizeY + 2) + 1; | ||
| uint16_t col = j % (pixelStatus::pixelSizeY + 2) + 1; | ||
| for (int i = 0; i < 3 && image.clus()[col][row] == pixelStatus::fakeVal; i++) { |
There was a problem hiding this comment.
shouldn't this be
| for (int i = 0; i < 3 && image.clus()[col][row] == pixelStatus::fakeVal; i++) { | |
| if (image.clus()[col][row] != pixelStatus::fakeVal) { | |
| continue; | |
| } | |
| for (int i = 0; i < 3; i++) { |
There was a problem hiding this comment.
otherwise the case i = 0 is performed also for the non-fake pixels.
There was a problem hiding this comment.
Yes, will commit this change
| for (uint32_t j : cms::alpakatools::independent_group_elements(acc, morphingsize)) { | ||
| uint16_t row = j / (pixelStatus::pixelSizeY + 2) + 1; | ||
| uint16_t col = j % (pixelStatus::pixelSizeY + 2) + 1; | ||
| for (int i = 0; i < 3 && image.clus()[col][row] == pixelStatus::empVal; i++) { |
There was a problem hiding this comment.
I think this should be
| for (int i = 0; i < 3 && image.clus()[col][row] == pixelStatus::empVal; i++) { | |
| if (image.clus()[col][row] != pixelStatus::empVal) { | |
| continue; | |
| } | |
| for (int i = 0; i < 3 && image.clus()[col][row] == pixelStatus::empVal; i++) { |
?
There was a problem hiding this comment.
Yes indeed. I missed the i=0 cases
|
I assume this should be closed in favour of #48734 ? |
|
-heterogeneous Assuming this should be superseded by #48734. |
|
Milestone for this pull request has been moved to CMSSW_16_0_X. Please open a backport if it should also go in to CMSSW_15_1_X. |
|
@cmsbuild, please close |
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:
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