Second round of comments - part II

AdrianoDee · AdrianoDee · commit e5ae12fa6a60 · 2025-04-07T13:06:54.000+02:00
diff --git a/DQM/SiPixelHeterogeneous/plugins/SiPixelMonitorRecHitsSoAAlpaka.cc b/DQM/SiPixelHeterogeneous/plugins/SiPixelMonitorRecHitsSoAAlpaka.cc
@@ -195,3 +195,4 @@ DEFINE_FWK_MODULE(SiPixelMonitorRecHitsSoAAlpaka);
 DEFINE_FWK_MODULE(SiPixelPhase1MonitorRecHitsSoAAlpaka);
 DEFINE_FWK_MODULE(SiPixelPhase2MonitorRecHitsSoAAlpaka);
 DEFINE_FWK_MODULE(SiPixelHIonPhase1MonitorRecHitsSoAAlpaka);
+
diff --git a/DataFormats/TrackSoA/interface/TracksSoA.h b/DataFormats/TrackSoA/interface/TracksSoA.h
@@ -54,13 +54,13 @@ namespace reco {
     auto end = tracks[i].hitOffsets();
     auto hitId = hits[start].id();
     int nl = 1;
-    auto ol = 0;
+    int ol = 0;
     while (hitId >= layerStarts[ol + 1] and ol < maxLayers)
       ++ol;
     ++start;
     for (; start < end; ++start) {
       hitId = hits[start].id();
-      auto il = 0;
+      int il = 0;
       while (hitId >= layerStarts[il + 1] and il < maxLayers)
         ++il;
       if (il != ol)
diff --git a/DataFormats/TrackSoA/src/alpaka/classes_cuda_def.xml b/DataFormats/TrackSoA/src/alpaka/classes_cuda_def.xml
@@ -3,3 +3,4 @@
   <class name="edm::DeviceProduct<alpaka_cuda_async::reco::TracksSoACollection>" persistent="false"/>
   <class name="edm::Wrapper<edm::DeviceProduct<alpaka_cuda_async::reco::TracksSoACollection>>" persistent="false"/>
 </lcgdict>
+
diff --git a/DataFormats/TrackSoA/src/alpaka/classes_rocm_def.xml b/DataFormats/TrackSoA/src/alpaka/classes_rocm_def.xml
@@ -3,3 +3,4 @@
   <class name="edm::DeviceProduct<alpaka_rocm_async::reco::TracksSoACollection>" persistent="false"/>
   <class name="edm::Wrapper<edm::DeviceProduct<alpaka_rocm_async::reco::TracksSoACollection>>" persistent="false"/>
 </lcgdict>
+
diff --git a/DataFormats/TrackSoA/src/classes_def.xml b/DataFormats/TrackSoA/src/classes_def.xml
@@ -15,3 +15,4 @@
   <class name="portablecollection::CollectionLeaf<1, reco::TrackHitsLayout<128, false>>"/>
 
 </lcgdict>
+
diff --git a/DataFormats/TrackSoA/test/BuildFile.xml b/DataFormats/TrackSoA/test/BuildFile.xml
@@ -21,3 +21,4 @@
 </library>
 
 <test name="testWriteAndReadTrackSoA" command="testWriteAndReadTrackSoA.sh" />
+
diff --git a/DataFormats/TrackSoA/test/alpaka/TrackSoAHeterogeneous_test.cc b/DataFormats/TrackSoA/test/alpaka/TrackSoAHeterogeneous_test.cc
@@ -41,7 +41,7 @@ int main() {
       "the test will be skipped.\n";
     exit(EXIT_FAILURE);
   }
-  
+
   // Run the test on each device
   for (const auto& device : devices) {
     Queue queue(device);
@@ -51,14 +51,14 @@ int main() {
       // Instantiate tracks on device. PortableDeviceCollection allocates
       // SoA on device automatically.
       constexpr auto nTracks = 1000;
-      constexpr auto nHits = nTracks *  5;
+      constexpr auto nHits = nTracks * 5;
 
-      TracksSoACollection tracks_d({{nTracks,nHits}},queue);
+      TracksSoACollection tracks_d({{nTracks, nHits}}, queue);
       testTrackSoA::runKernels(tracks_d.view(), queue);
 
       // Instantate tracks on host. This is where the data will be
       // copied to from device.
-      ::reco::TracksHost tracks_h({{nTracks,nHits}},queue);
+      ::reco::TracksHost tracks_h({{nTracks, nHits}}, queue);
 
       std::cout << "no. of tracks = " << tracks_h.view().metadata().size() << std::endl;
       alpaka::memcpy(queue, tracks_h.buffer(), tracks_d.const_buffer());
diff --git a/DataFormats/TrackSoA/test/testReadHostTrackSoA.py b/DataFormats/TrackSoA/test/testReadHostTrackSoA.py
@@ -16,4 +16,5 @@
 
 process.path = cms.Path(process.testReadHostTrackSoA)
 
-process.endPath = cms.EndPath(process.out)
+process.endPath = cms.EndPath(process.out)
+
diff --git a/DataFormats/TrackSoA/test/testWriteAndReadTrackSoA.sh b/DataFormats/TrackSoA/test/testWriteAndReadTrackSoA.sh
@@ -19,3 +19,4 @@ if [ $? -ne 0 ]; then
 fi
 
 echo '>>>> Done! <<<<'
+
diff --git a/DataFormats/TrackSoA/test/testWriteHostTrackSoA.py b/DataFormats/TrackSoA/test/testWriteHostTrackSoA.py
@@ -18,4 +18,5 @@
 process.path = cms.Path(process.trackSoA)
 process.endPath = cms.EndPath(process.out)
 
-'testTrackSoAWriter.root'
+'testTrackSoAWriter.root'
+
diff --git a/DataFormats/TrackingRecHitSoA/interface/TrackingRecHitsDevice.h b/DataFormats/TrackingRecHitSoA/interface/TrackingRecHitsDevice.h
@@ -53,7 +53,7 @@ namespace reco {
     }
 
     uint32_t nHits() const { return this->template view<TrackingRecHitSoA>().metadata().size(); }
-    uint32_t nModules() const { return this->template view<HitModuleSoA>().metadata().size(); }
+    uint32_t nModules() const { return this->template view<HitModuleSoA>().metadata().size() - 1; }
 
     int32_t offsetBPIX2() const { return offsetBPIX2_; }
 
diff --git a/DataFormats/TrackingRecHitSoA/interface/TrackingRecHitsHost.h b/DataFormats/TrackingRecHitSoA/interface/TrackingRecHitsHost.h
@@ -48,7 +48,7 @@ namespace reco {
     }
 
     uint32_t nHits() const { return this->template view<TrackingRecHitSoA>().metadata().size(); }
-    uint32_t nModules() const { return this->template view<HitModuleSoA>().metadata().size(); }
+    uint32_t nModules() const { return this->template view<HitModuleSoA>().metadata().size() - 1; }
 
     int32_t offsetBPIX2() const { return this->template view<TrackingRecHitSoA>().offsetBPIX2(); }
 
diff --git a/DataFormats/TrackingRecHitSoA/interface/TrackingRecHitsSoA.h b/DataFormats/TrackingRecHitSoA/interface/TrackingRecHitsSoA.h
@@ -28,11 +28,11 @@ namespace reco {
 
   GENERATE_SOA_LAYOUT(HitModulesLayout, SOA_COLUMN(uint32_t, moduleStart));
 
-// N.B. this layout is not really included by default in the hits SoA
-// This holds the needed parameters to activate (via ONLY_TRIPLETS_IN_HOLE) the 
-// calculations to check if a triplet points to the disk hole 
-// and then retain only those that fulfil this requirement.
-// At the moment this feature is not fully (re)implemented.  
+  // N.B. this layout is not really included by default in the hits SoA
+  // This holds the needed parameters to activate (via ONLY_TRIPLETS_IN_HOLE) the
+  // calculations to check if a triplet points to the disk hole
+  // and then retain only those that fulfil this requirement.
+  // At the moment this feature is not fully (re)implemented.
 
   GENERATE_SOA_LAYOUT(AverageGeometryLayout,
                       SOA_COLUMN(float, ladderZ),
diff --git a/DataFormats/TrackingRecHitSoA/interface/alpaka/TrackingRecHitsSoACollection.h b/DataFormats/TrackingRecHitSoA/interface/alpaka/TrackingRecHitsSoACollection.h
@@ -60,13 +60,11 @@ namespace cms::alpakatools {
     static auto copyAsync(TQueue& queue, reco::TrackingRecHitHost const& hostData) {
       using TDevice = typename alpaka::trait::DevType<TQueue>::type;
 
-      auto hostHitView = hostData.template view<reco::TrackingRecHitSoA>();
-      auto moduleHitsView = hostData.template view<reco::HitModuleSoA>();
+      auto nHits = hostData.nHits();
 
-      reco::TrackingRecHitDevice<TDevice> deviceData(
-          queue, hostHitView.metadata().size(), moduleHitsView.metadata().size());
+      reco::TrackingRecHitDevice<TDevice> deviceData(queue, nHits, hostData.nModules());
 
-      if (hostHitView.metadata().size() == 0) {
+      if (nHits == 0) {
         std::memset(
             deviceData.buffer().data(),
             0,
diff --git a/DataFormats/TrackingRecHitSoA/src/alpaka/classes_cuda_def.xml b/DataFormats/TrackingRecHitSoA/src/alpaka/classes_cuda_def.xml
@@ -3,3 +3,4 @@
   <class name="edm::DeviceProduct<alpaka_cuda_async::reco::TrackingRecHitsSoACollection>" persistent="false"/>
   <class name="edm::Wrapper<edm::DeviceProduct<alpaka_cuda_async::reco::TrackingRecHitsSoACollection>>" persistent="false"/>
 </lcgdict>
+
diff --git a/DataFormats/TrackingRecHitSoA/src/alpaka/classes_rocm_def.xml b/DataFormats/TrackingRecHitSoA/src/alpaka/classes_rocm_def.xml
@@ -2,4 +2,5 @@
   <class name="alpaka_rocm_async::reco::TrackingRecHitsSoACollection" persistent="false"/>
   <class name="edm::DeviceProduct<alpaka_rocm_async::reco::TrackingRecHitsSoACollection>" persistent="false"/>
   <class name="edm::Wrapper<edm::DeviceProduct<alpaka_rocm_async::reco::TrackingRecHitsSoACollection>>" persistent="false"/>
-</lcgdict>
+</lcgdict>
+
diff --git a/DataFormats/TrackingRecHitSoA/src/classes_def.xml b/DataFormats/TrackingRecHitSoA/src/classes_def.xml
@@ -19,3 +19,4 @@
   <class name="portablecollection::CollectionLeaf<1, reco::HitModulesLayout<128, false>>"/>
   
 </lcgdict>
+
diff --git a/DataFormats/TrackingRecHitSoA/test/BuildFile.xml b/DataFormats/TrackingRecHitSoA/test/BuildFile.xml
@@ -13,4 +13,5 @@
   <use name="FWCore/Utilities"/>
 </library>
 
-<test name="testWriteAndReadHitSoA" command="testWriteAndReadHitSoA.sh" />
+<test name="testWriteAndReadHitSoA" command="testWriteAndReadHitSoA.sh" />
+
diff --git a/DataFormats/TrackingRecHitSoA/test/testReadHostHitSoA.py b/DataFormats/TrackingRecHitSoA/test/testReadHostHitSoA.py
@@ -16,4 +16,5 @@
 
 process.path = cms.Path(process.testReadHostHitSoA)
 
-process.endPath = cms.EndPath(process.out)
+process.endPath = cms.EndPath(process.out)
+
diff --git a/DataFormats/TrackingRecHitSoA/test/testWriteAndReadHitSoA.sh b/DataFormats/TrackingRecHitSoA/test/testWriteAndReadHitSoA.sh
@@ -19,3 +19,4 @@ if [ $? -ne 0 ]; then
 fi
 
 echo '>>>> Done! <<<<'
+
diff --git a/DataFormats/TrackingRecHitSoA/test/testWriteHostHitSoA.py b/DataFormats/TrackingRecHitSoA/test/testWriteHostHitSoA.py
@@ -16,4 +16,5 @@
 )
 
 process.path = cms.Path(process.hitSoA)
-process.endPath = cms.EndPath(process.out)
+process.endPath = cms.EndPath(process.out)
+
diff --git a/HLTrigger/Configuration/python/HLT_75e33/modules/hltPhase2PixelTracksSoA_cfi.py b/HLTrigger/Configuration/python/HLT_75e33/modules/hltPhase2PixelTracksSoA_cfi.py
@@ -43,3 +43,4 @@
 
 from Configuration.ProcessModifiers.singleIterPatatrack_cff import singleIterPatatrack
 singleIterPatatrack.toReplaceWith(hltPhase2PixelTracksSoA, _hltPhase2PixelTracksSoASingleIterPatatrack)
+
diff --git a/HLTrigger/Configuration/python/HLT_75e33_cff.py b/HLTrigger/Configuration/python/HLT_75e33_cff.py
@@ -376,3 +376,4 @@
     fragment.HLTriggerFinalPath,
     fragment.HLTAnalyzerEndpath,
 ])
+
diff --git a/HLTrigger/Configuration/python/HLT_75e33_timing_cff.py b/HLTrigger/Configuration/python/HLT_75e33_timing_cff.py
@@ -350,3 +350,4 @@
     fragment.HLTriggerFinalPath,
     fragment.HLTAnalyzerEndpath,
 ])
+
diff --git a/HLTrigger/Configuration/python/customizeHLTforCMSSW.py b/HLTrigger/Configuration/python/customizeHLTforCMSSW.py
@@ -223,3 +223,4 @@ def customizeHLTforCMSSW(process, menuType="GRun"):
     process = customizeHLTfor47611(process)
 
     return process
+
diff --git a/HeterogeneousCore/AlpakaInterface/interface/prefixScan.h b/HeterogeneousCore/AlpakaInterface/interface/prefixScan.h
@@ -188,13 +188,13 @@ namespace cms::alpakatools {
         psum[i] = (j < size) ? co[j] : T(0);
       }
       alpaka::syncBlockThreads(acc);
-      if (blocksPerGrid <= 1024)
+      if (blocksPerGrid <= warpSize * warpSize)
         blockPrefixScan(acc, psum, psum, blocksPerGrid, ws);
       else {
         auto off = 0u;
-        while (off + 1024 < blocksPerGrid) {
-          blockPrefixScan(acc, psum + off, psum + off, 1024, ws);
-          off = off + 1024 - 1;
+        while (off + warpSize * warpSize < blocksPerGrid) {
+          blockPrefixScan(acc, psum + off, warpSize * warpSize, ws);
+          off = off + warpSize * warpSize - 1;
           alpaka::syncBlockThreads(acc);
         }
         blockPrefixScan(acc, psum + off, psum + off, blocksPerGrid - off, ws);
diff --git a/RecoLocalTracker/ClusterParameterEstimator/BuildFile.xml b/RecoLocalTracker/ClusterParameterEstimator/BuildFile.xml
@@ -7,3 +7,4 @@
 <export>
   <lib name="1"/>
 </export>
+
diff --git a/RecoLocalTracker/SiPixelRecHits/plugins/alpaka/PixelRecHits.h b/RecoLocalTracker/SiPixelRecHits/plugins/alpaka/PixelRecHits.h
@@ -45,7 +45,6 @@ namespace ALPAKA_ACCELERATOR_NAMESPACE {
           // This is necessary only once - consider moving it somewhere else.
           // Copy the average geometry corrected by the beamspot.
           if (0 == module) {
-            // auto& agc = hits.averageGeometry();
             auto const& ag = cpeParams->averageGeometry();
             auto nLadders = TrackerTraits::numberOfLaddersInBarrel;
 
diff --git a/RecoLocalTracker/SiPixelRecHits/src/PixelCPEFastParamsHost.cc b/RecoLocalTracker/SiPixelRecHits/src/PixelCPEFastParamsHost.cc
@@ -255,17 +255,16 @@ void PixelCPEFastParamsHost<TrackerTraits>::fillParamsForDevice() {
 
 #ifdef ONLY_TRIPLETS_IN_HOLE
   // compute ladder baricenter (only in global z) for the barrel
-  //
 
   constexpr int numberOfModulesInLadder = TrackerTraits::numberOfModulesInLadder;
   constexpr int numberOfLaddersInBarrel = TrackerTraits::numberOfLaddersInBarrel;
   constexpr int numberOfModulesInBarrel = TrackerTraits::numberOfModulesInBarrel;
 
+  constexpr float ladderFactor = 1.f / float(numberOfModulesInLadder);
+
   constexpr int firstEndcapPos = TrackerTraits::firstEndcapPos;
   constexpr int firstEndcapNeg = TrackerTraits::firstEndcapNeg;
 
-  constexpr float ladderFactor = 1.f / float(numberOfModulesInLadder);
-
   auto& aveGeom = buffer_->averageGeometry();
   int il = 0;
   for (int im = 0, nm = numberOfModulesInBarrel; im < nm; ++im) {
diff --git a/RecoTracker/Configuration/python/customizePixelTracksForTriplets.py b/RecoTracker/Configuration/python/customizePixelTracksForTriplets.py
@@ -64,3 +64,4 @@ def customizePixelTracksForTriplets(process):
          break
 
    return process
+
diff --git a/RecoTracker/PixelSeeding/BuildFile.xml b/RecoTracker/PixelSeeding/BuildFile.xml
@@ -32,3 +32,4 @@
 </export>
 <use name="ofast-flag"/>
 <flags CXXFLAGS="-fno-math-errno"/>
+
diff --git a/RecoTracker/PixelSeeding/interface/alpaka/CACoupleSoACollection.h b/RecoTracker/PixelSeeding/interface/alpaka/CACoupleSoACollection.h
@@ -24,4 +24,3 @@ namespace ALPAKA_ACCELERATOR_NAMESPACE {
 ASSERT_DEVICE_MATCHES_HOST_COLLECTION(CACoupleSoACollection, ::caStructures::CACoupleHost);
 
 #endif  // RecoTracker_PixelSeeding_interface_CACoupleSoACollection_h
-
diff --git a/RecoTracker/PixelSeeding/plugins/alpaka/CAHitNtupletGenerator.cc b/RecoTracker/PixelSeeding/plugins/alpaka/CAHitNtupletGenerator.cc
@@ -54,15 +54,36 @@ namespace ALPAKA_ACCELERATOR_NAMESPACE {
           ->setComment("Cut on cluster size differences (in Y) for barrel-forward cells. Barrel-forward cells.");
 
       // Container sizes
-      desc.add<std::string>("maxNumberOfDoublets", std::to_string(pixelTopology::Phase1::maxNumberOfDoublets));
-      desc.add<std::string>("maxNumberOfTuples", std::to_string(pixelTopology::Phase1::maxNumberOfTuples));
-      desc.add<double>("avgHitsPerTrack", 5.0f);
-      desc.add<double>("avgCellsPerHit", 25.0f);
-      desc.add<double>("avgCellsPerCell", 2.0f);
-      desc.add<double>("avgTracksPerCell", 1.0f);
+      //
+      // maxNumberOfDoublets and maxNumberOfTuples may be defined at runtime depending on the number of hits.
+      // This is done via a TFormula expecting 'x' as nHits.
+      // e.g. : maxNumberOfDoublets = cms.string( '0.00022*pow(x,2)  + 0.53*x + 10000' )
+      // will compute maxNumberOfDoublets for each event as
+      //
+      //  	maxNumberOfDoublets = 2.2e-4 * nHits^2 + 0.53 * nHits + 10000
+      //
+      // this may also be simply a constant (as for the default parameters)
+      //
+      // 	 maxNumberOfDoublets = cms.string(str(512*1024))
+      //
+
+      desc.add<std::string>("maxNumberOfDoublets", std::to_string(pixelTopology::Phase1::maxNumberOfDoublets))
+          ->setComment(
+              "Max nummber of doublets (cells) as a string. The string will be parsed to a TFormula, depending on "
+              "nHits (labeled 'x'), \n and evaluated for each event. May also be a constant.");
+      desc.add<std::string>("maxNumberOfTuples", std::to_string(pixelTopology::Phase1::maxNumberOfTuples))
+          ->setComment("Max nummber of tuples as a string. Same behavior as maxNumberOfDoublets.");
+      desc.add<double>("avgHitsPerTrack", 5.0f)->setComment("Number of hits per track. Average per track.");
+      desc.add<double>("avgCellsPerHit", 25.0f)
+          ->setComment("Number of cells for which an hit is the outer hit. Average per hit.");
+      desc.add<double>("avgCellsPerCell", 2.0f)
+          ->setComment("Number of cells connected to another cell. Average per cell.");
+      desc.add<double>("avgTracksPerCell", 1.0f)
+          ->setComment("Number of tracks to which a cell belongs. Average per cell.");
 
       // nTuplet Cuts and Params
       desc.add<double>("ptmin", 0.9f)->setComment("Cut on minimum pt");
+      //// p [GeV/c] = B [T] * R [m] * 0.3 (factor from conversion from J to GeV and q = e = 1.6 * 10e-19 C)
       //// 87 cm/GeV = 1/(3.8T * 0.3)
       //// take less than radius given by the hardPtCut and reject everything below
       desc.add<double>("hardCurvCut", 1.f / (0.35 * 87.f))
diff --git a/RecoTracker/PixelSeeding/test/alpaka/CAGeometry_t.cpp b/RecoTracker/PixelSeeding/test/alpaka/CAGeometry_t.cpp
@@ -78,4 +78,4 @@ int main() {
   }
 
   return EXIT_SUCCESS;
-}
+}
diff --git a/RecoTracker/PixelSeeding/test/alpaka/CAGeometry_t.h b/RecoTracker/PixelSeeding/test/alpaka/CAGeometry_t.h
@@ -11,4 +11,3 @@ namespace ALPAKA_ACCELERATOR_NAMESPACE::testParamsSoA {
 }  // namespace ALPAKA_ACCELERATOR_NAMESPACE::testParamsSoA
 
 #endif  // RecoTracker_PixelSeeding_test_alpaka_CAGeometrySoA_test_h
-
diff --git a/RecoTracker/PixelTrackFitting/python/PixelTracks_cff.py b/RecoTracker/PixelTrackFitting/python/PixelTracks_cff.py
@@ -170,3 +170,4 @@ def _addCAGeometryESProducer(process):
     # Convert the pixel tracks from SoA to legacy format
     pixelTracks)
 )
+
diff --git a/RecoTracker/Record/BuildFile.xml b/RecoTracker/Record/BuildFile.xml
@@ -10,4 +10,5 @@
 <use name="clhep"/>
 <use name="boost"/>
 <use name="alpaka"/>
-<flags ALPAKA_BACKENDS="1"/>
+<flags ALPAKA_BACKENDS="1"/>
+
diff --git a/RecoTracker/Record/interface/CAGeometryHost.h b/RecoTracker/Record/interface/CAGeometryHost.h
@@ -12,4 +12,4 @@
 namespace reco {
   using CAGeometryHost = PortableHostMultiCollection<CALayersSoA, CAGraphSoA, CAModulesSoA>;
 }
-#endif  // RecoTracker_PixelSeeding_interface_CAGeometryHost_H
+#endif  // RecoTracker_PixelSeeding_interface_CAGeometryHost_H
diff --git a/RecoTracker/Record/src/ES_CAGeometry.cc b/RecoTracker/Record/src/ES_CAGeometry.cc
@@ -2,4 +2,3 @@
 #include "FWCore/Utilities/interface/typelookup.h"
 
 TYPELOOKUP_DATA_REG(reco::CAGeometryHost);
-
diff --git a/RecoVertex/Configuration/python/RecoPixelVertexing_cff.py b/RecoVertex/Configuration/python/RecoPixelVertexing_cff.py
@@ -48,3 +48,4 @@
     pixelVerticesTask
 )
 recopixelvertexing = cms.Sequence(recopixelvertexingTask)
+

Original file line number	Diff line number	Diff line change
`@@ -15,3 +15,4 @@`
`15`	`15`	`<class name="portablecollection::CollectionLeaf<1, reco::TrackHitsLayout<128, false>>"/>`
`16`	`16`
`17`	`17`	`</lcgdict>`
	`18`	`+`
Original file line number	Diff line number	Diff line change
`@@ -21,3 +21,4 @@`
`21`	`21`	`</library>`
`22`	`22`
`23`	`23`	`<test name="testWriteAndReadTrackSoA" command="testWriteAndReadTrackSoA.sh" />`
	`24`	`+`
Original file line number	Diff line number	Diff line change
`@@ -19,3 +19,4 @@ if [ $? -ne 0 ]; then`
`19`	`19`	`fi`
`20`	`20`
`21`	`21`	`echo '>>>> Done! <<<<'`
	`22`	`+`