Open issues regarding the Pixel local reconstruction on GPU

[fwyzard]

Open issues regarding the Pixel local reconstruction on GPU

• high level: add support for the Phase 2 geometry
  • make the CPU and GPU buffers used for the pixel local reconstruction configurable at runtime
  • extend or duplicate PixelCPEFast for the Phase 2 geometry (see #31721 (comment))
  • once support for the Phase 2 geometry is available, rename Phase 1-only modules accordingly
• high level: reduce code duplication
  • reuse existing constants in SiPixelRawToClusterGPUKernel (see #31721 (comment))
  • factor common code between PixelCPEGeneric and PixelCPEFast (see #31721 (comment), #31721 (comment), #31721 (comment))
  • factor common code between SiPixelDigiErrorsFromSoA and SiPixelRawToDigi (see #31721 (comment))
• reduce the number of dedicated namespacess for Pixel reconstruction on GPU
• add more comments to the code
• use gpuClustering::maxNumModules where appropriate: done in Clean up the pixel local reconstruction code cms-patatrack/cmssw#593 / cms-patatrack@f6924bb
• use phase1PixelTopology::numberOfLayers where appropriate
• make the pixel cluster thresholds configurable
• avoid explicit memory operations (cudaMalloc, cudaFree, cudaMemcpy, cudaMemcpyAsync, etc.) in favour of the make_device_unique, copyAsync, etc. wrappers
• implement proper encapsulation for pixelCPEforGPU::ParamsOnGPU
• decide if there is better place and mechanism for storing the beamspot-corrected geometry

[cmsbuild]

A new Issue was created by @fwyzard Andrea Bocci.

@Dr15Jones, @dpiparo, @silviodonato, @smuzaffar, @makortel, @qliphy can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

[fwyzard]

make the CPU and GPU buffers used for the pixel local reconstruction configurable at runtime

See https://github.com/cms-sw/cmssw/pull/31721/files#r521009826 for an initial discussion.

The dimensions defined in CUDADataFormats/SiPixelCluster/interface/gpuClusteringConstants.h should eventually be changes to accommodate the Phase 2 and Heavy Ions conditions.

This is possible in different ways:

• making 2-3 variants of the classes (e.g. templating them) for few well-defined use cases, and pick a different implementation at runtime based on the configuration
• making the buffer sizes dependent on the job configuration, and allocate them at module construction time
• making the buffers fully resizable at runtime (or at least event by event) (unlikely to be useful)
• etc.

Some preliminary work was done in cms-patatrack#545, cms-patatrack#583, cms-patatrack#590 .

A likely requirement is a more user-friendly SoA interface (see cms-patatrack#272).

[fwyzard]

assign reconstruction

[fwyzard]

assign heterogeneous

[cmsbuild]

New categories assigned: heterogeneous,reconstruction

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

[fwyzard]

@tsusa @mmusich @VinInn FYI

[fwyzard]

Reduce the number of dedicated namespacess for Pixel reconstruction on GPU

• namespace gpuClustering is used in
  • CUDADataFormats/SiPixelCluster/interface/gpuClusteringConstants.h
  • RecoLocalTracker/SiPixelClusterizer/plugins/gpuClusterChargeCut.h
  • RecoLocalTracker/SiPixelClusterizer/plugins/gpuClustering.h
• namespace pixelGPUConstants is used in
  • CUDADataFormats/SiPixelCluster/interface/gpuClusteringConstants.h
• namespace gpuCalibPixel is used in
  • RecoLocalTracker/SiPixelClusterizer/plugins/gpuCalibPixel.h
• namespace pixelCPEforGPU is used in
  • RecoLocalTracker/SiPixelRecHits/interface/pixelCPEforGPU.h
• namespace pixelgpudetails is used in
  • CondFormats/SiPixelObjects/interface/SiPixelROCsStatusAndMapping.h
  • RecoLocalTracker/SiPixelClusterizer/plugins/SiPixelRawToClusterGPUKernel.h and RecoLocalTracker/SiPixelClusterizer/plugins/SiPixelRawToClusterGPUKernel.cu
  • RecoLocalTracker/SiPixelRecHits/plugins/PixelRecHits.h and RecoLocalTracker/SiPixelRecHits/plugins/PixelRecHits.cu

[fwyzard]

Add more comments to the code

• in TrackingRecHit2DHeterogeneous.h:

    static constexpr uint32_t n16 = 4;
    static constexpr uint32_t n32 = 9;

[fwyzard]

Use phase1PixelTopology::numberOfLayers where appropriate

In

• Geometry/TrackerGeometryBuilder/test/phase1PixelTopology_t.cpp
• RecoLocalTracker/SiPixelRecHits/plugins/SiPixelRecHitSoAFromLegacy.cc

use phase1PixelTopology::numberOfLayers (+ 1) in place of 10 (11).

[VinInn]

not these one

RecoLocalTracker/SiPixelClusterizer/plugins/SiPixelDigisClustersFromSoA.cc:

   /// this is an algorithm constant defined by Danek
    auto clusterThreshold = (layer == 1) ? 2000 : 4000;


RecoLocalTracker/SiPixelClusterizer/plugins/SiPixelRawToClusterGPUKernel.h:

// this is a property of FED/ROC and already proposed to be increased for HI
const uint32_t MAX_WORD = 2000;

[fwyzard]

Make the pixel cluster thresholds configurable

RecoLocalTracker/SiPixelClusterizer/plugins/gpuClusterChargeCut.h and RecoLocalTracker/SiPixelClusterizer/plugins/SiPixelDigisClustersFromSoA.cc use hard-coded cuts of 2000 and 4000, while the CPU code at RecoLocalTracker/SiPixelClusterizer/plugins/PixelThresholdClusterizer.cc takes the same values from the runtime configuration.

The GPU code could be updated to do the same.

[czangela]

I would like to reiterate and start a conversation on the matter of namespaces, listed in this previous comment.

Some previous conversations about this went on at #31721 and can be found at comment 1, comment 2. Please add more if you know of any. (threads, comments, hypernews, anything)

As far as I understand, (correct me @mmusich if I'm wrong) one namespace should do.

Ideas on what this should be are welcome.

[VinInn]

namespaces are used to group "items" no more no less than structs or classes. The same OOD criteria apply.
One structural difference is that declarations in namespaces may span multiple files.
phase1PixelTopology is the kind of entity that in the old days would have been modeled using a struct.
I find a namespace more appropriate (and in CMSSW we had campaigns to move "struct" to "namespace").
Still, as we deplorate multi-purpose classes, we shall not, in my opinion, clusterize everything is a single namespace just for the sake of "economy".
In summary: just use your best OOD judgment.

[mmusich]

@jpata, can you please move the open issues connected to pixel tracking and vertexing to a separate issue?
As far as I can tell (based on the title) this issue is reserved to pixel local reconstruction.
Thanks.