Addressed ESProducer comments

RecoTracker/LST/plugins/alpaka/LSTModulesDevESProducer.cc

@@ -15,41 +15,18 @@ namespace ALPAKA_ACCELERATOR_NAMESPACE {
 
   class LSTModulesDevESProducer : public ESProducer {
   public:
-    LSTModulesDevESProducer(const edm::ParameterSet& iConfig);
+    LSTModulesDevESProducer(edm::ParameterSet const& iConfig) : ESProducer(iConfig) { setWhatProduced(this); }
 
-    static void fillDescriptions(edm::ConfigurationDescriptions& descriptions);
+    static void fillDescriptions(edm::ConfigurationDescriptions& descriptions) {
+      edm::ParameterSetDescription desc;
+      descriptions.addWithDefaultLabel(desc);
+    }
 
-    std::unique_ptr<SDL::LSTESHostData<SDL::Dev>> produceHost(TrackerRecoGeometryRecord const& iRecord);
-    std::unique_ptr<SDL::LSTESDeviceData<SDL::Dev>> produceDevice(
-        device::Record<TrackerRecoGeometryRecord> const& iRecord);
-
-  private:
-    edm::ESGetToken<SDL::LSTESHostData<SDL::Dev>, TrackerRecoGeometryRecord> lstESHostToken_;
+    std::unique_ptr<SDL::LSTESData<SDL::DevHost>> produce(TrackerRecoGeometryRecord const& iRecord) {
+      return SDL::loadAndFillESHost();
+    }
   };
 
-  LSTModulesDevESProducer::LSTModulesDevESProducer(const edm::ParameterSet& iConfig) : ESProducer(iConfig) {
-    setWhatProduced(this, &LSTModulesDevESProducer::produceHost);
-    auto cc = setWhatProduced(this, &LSTModulesDevESProducer::produceDevice);
-    lstESHostToken_ = cc.consumes();
-  }
-
-  void LSTModulesDevESProducer::fillDescriptions(edm::ConfigurationDescriptions& descriptions) {
-    edm::ParameterSetDescription desc;
-    descriptions.addWithDefaultLabel(desc);
-  }
-
-  std::unique_ptr<SDL::LSTESHostData<SDL::Dev>> LSTModulesDevESProducer::produceHost(
-      TrackerRecoGeometryRecord const& iRecord) {
-    return SDL::loadAndFillESHost();
-  }
-
-  std::unique_ptr<SDL::LSTESDeviceData<SDL::Dev>> LSTModulesDevESProducer::produceDevice(
-      device::Record<TrackerRecoGeometryRecord> const& iRecord) {
-    auto const& lstESHostData = iRecord.get(lstESHostToken_);
-    SDL::QueueAcc& queue = iRecord.queue();
-    return SDL::loadAndFillESDevice(queue, &lstESHostData);
-  }
-
 }  // namespace ALPAKA_ACCELERATOR_NAMESPACE
 
 DEFINE_FWK_EVENTSETUP_ALPAKA_MODULE(LSTModulesDevESProducer);

RecoTracker/LST/plugins/alpaka/LSTProducer.cc

@@ -81,7 +81,7 @@ namespace ALPAKA_ACCELERATOR_NAMESPACE {
   private:
     edm::EDGetTokenT<LSTPixelSeedInput> lstPixelSeedInputToken_;
     edm::EDGetTokenT<LSTPhase2OTHitsInput> lstPhase2OTHitsInputToken_;
-    device::ESGetToken<SDL::LSTESDeviceData<SDL::Dev>, TrackerRecoGeometryRecord> lstESToken_;
+    device::ESGetToken<SDL::LSTESData<Device>, TrackerRecoGeometryRecord> lstESToken_;
     const int verbose_;
     edm::EDPutTokenT<LSTOutput> lstOutputToken_;
 

RecoTracker/LST/src/alpaka/ES_ModulesDev.cc

@@ -3,8 +3,5 @@
 
 // Temporary hack: The DevHost instantiation is needed in both CPU and GPU plugins,
 // whereas the (non-host-)Device instantiation only in the GPU plugin
-TYPELOOKUP_DATA_REG(SDL::LSTESHostData<SDL::Dev>);
-TYPELOOKUP_DATA_REG(SDL::LSTESDeviceData<SDL::DevHost>);
-TYPELOOKUP_DATA_REG(ALPAKA_ACCELERATOR_NAMESPACE::ESDeviceProduct<std::unique_ptr<SDL::LSTESHostData<SDL::Dev>>>);
-TYPELOOKUP_ALPAKA_TEMPLATED_DATA_REG(SDL::LSTESHostData);
-TYPELOOKUP_ALPAKA_TEMPLATED_DATA_REG(SDL::LSTESDeviceData);
+TYPELOOKUP_DATA_REG(SDL::LSTESData<SDL::DevHost>);
+TYPELOOKUP_ALPAKA_TEMPLATED_DATA_REG(SDL::LSTESData);

RecoTracker/LSTCore/interface/alpaka/LST.h

@@ -28,7 +28,7 @@ namespace SDL {
 
     void run(QueueAcc& queue,
              bool verbose,
-             const LSTESDeviceData<Dev>* deviceESData,
+             const LSTESData<Dev>* deviceESData,
              const std::vector<float> see_px,
              const std::vector<float> see_py,
              const std::vector<float> see_pz,

RecoTracker/LSTCore/interface/alpaka/LSTESData.h

@@ -16,58 +16,27 @@ namespace SDL {
 
   struct pixelMap;
 
-  template <typename>
-  class TiltedGeometry;
-
-  template <typename>
-  class ModuleConnectionMap;
-  using MapPLStoLayer = std::array<std::array<ModuleConnectionMap<Dev>, 4>, 3>;
-
   template <typename>
   struct modulesBuffer;
 
-  template <typename>
-  class EndcapGeometryHost;
-
   template <typename>
   class EndcapGeometry;
 
   template <typename TDev>
-  struct LSTESHostData;
-
-  // FIXME: This shouldn't be a templated struct
-  template <>
-  struct LSTESHostData<Dev> {
-    std::shared_ptr<const MapPLStoLayer> mapPLStoLayer;
-    std::shared_ptr<const EndcapGeometryHost<Dev>> endcapGeometry;
-    std::shared_ptr<const TiltedGeometry<Dev>> tiltedGeometry;
-    std::shared_ptr<const ModuleConnectionMap<Dev>> moduleConnectionMap;
-
-    LSTESHostData(std::shared_ptr<MapPLStoLayer> mapPLStoLayerIn,
-                  std::shared_ptr<EndcapGeometryHost<Dev>> endcapGeometryIn,
-                  std::shared_ptr<TiltedGeometry<Dev>> tiltedGeometryIn,
-                  std::shared_ptr<ModuleConnectionMap<Dev>> moduleConnectionMapIn)
-        : mapPLStoLayer(mapPLStoLayerIn),
-          endcapGeometry(endcapGeometryIn),
-          tiltedGeometry(tiltedGeometryIn),
-          moduleConnectionMap(moduleConnectionMapIn) {}
-  };
-
-  template <typename TDev>
-  struct LSTESDeviceData {
+  struct LSTESData {
     uint16_t nModules;
     uint16_t nLowerModules;
     unsigned int nPixels;
     std::shared_ptr<const modulesBuffer<TDev>> modulesBuffers;
     std::shared_ptr<const EndcapGeometry<TDev>> endcapGeometry;
     std::shared_ptr<const pixelMap> pixelMapping;
 
-    LSTESDeviceData(uint16_t nModulesIn,
-                    uint16_t nLowerModulesIn,
-                    unsigned int nPixelsIn,
-                    std::shared_ptr<modulesBuffer<TDev>> modulesBuffersIn,
-                    std::shared_ptr<EndcapGeometry<TDev>> endcapGeometryIn,
-                    std::shared_ptr<pixelMap> pixelMappingIn)
+    LSTESData(uint16_t const& nModulesIn,
+              uint16_t const& nLowerModulesIn,
+              unsigned int const& nPixelsIn,
+              std::shared_ptr<const modulesBuffer<TDev>> const& modulesBuffersIn,
+              std::shared_ptr<const EndcapGeometry<TDev>> const& endcapGeometryIn,
+              std::shared_ptr<const pixelMap> const& pixelMappingIn)
         : nModules(nModulesIn),
           nLowerModules(nLowerModulesIn),
           nPixels(nPixelsIn),
@@ -76,18 +45,15 @@ namespace SDL {
           pixelMapping(pixelMappingIn) {}
   };
 
-  std::unique_ptr<LSTESHostData<Dev>> loadAndFillESHost();
-  std::unique_ptr<LSTESDeviceData<Dev>> loadAndFillESDevice(SDL::QueueAcc& queue, const LSTESHostData<Dev>* hostData);
+  std::unique_ptr<LSTESData<SDL::DevHost>> loadAndFillESHost();
 
 }  // namespace SDL
 
 namespace cms::alpakatools {
   template <>
-  struct CopyToDevice<SDL::LSTESHostData<SDL::Dev>> {
+  struct CopyToDevice<SDL::LSTESData<SDL::DevHost>> {
     template <typename TQueue>
-    static auto copyAsync(TQueue& queue, SDL::LSTESHostData<SDL::Dev> const& hostData) {
-      return std::make_unique<SDL::LSTESHostData<SDL::Dev>>(hostData);
-    }
+    static SDL::LSTESData<alpaka::Dev<TQueue>> copyAsync(TQueue& queue, SDL::LSTESData<SDL::DevHost> const& srcData);
   };
 }  // namespace cms::alpakatools
 

RecoTracker/LSTCore/src/alpaka/EndcapGeometry.dev.cc

@@ -1,16 +1,23 @@
 #include "EndcapGeometry.h"
 
-SDL::EndcapGeometry<SDL::Dev>::EndcapGeometry(SDL::Dev const& devAccIn,
-                                              SDL::QueueAcc& queue,
-                                              SDL::EndcapGeometryHost<SDL::Dev> const& endcapGeometryIn)
-    : geoMapDetId_buf(allocBufWrapper<unsigned int>(devAccIn, endcapGeometryIn.centroid_phis_.size())),
-      geoMapPhi_buf(allocBufWrapper<float>(devAccIn, endcapGeometryIn.centroid_phis_.size())) {
-  dxdy_slope_ = endcapGeometryIn.dxdy_slope_;
-  centroid_phis_ = endcapGeometryIn.centroid_phis_;
-  fillGeoMapArraysExplicit(queue);
+template <typename TDev>
+SDL::EndcapGeometry<TDev>::EndcapGeometry(TDev const& devAccIn)
+    : geoMapDetId_buf(allocBufWrapper<unsigned int>(devAccIn, 0)), geoMapPhi_buf(allocBufWrapper<float>(devAccIn, 0)) {}
+
+template <typename TDev>
+template <typename TQueue>
+SDL::EndcapGeometry<TDev>::EndcapGeometry(TQueue& queue, SDL::EndcapGeometry<SDL::DevHost> const& endcapGeometrySrc)
+    : dxdy_slope_(endcapGeometrySrc.dxdy_slope_),
+      centroid_phis_(endcapGeometrySrc.centroid_phis_),
+      geoMapDetId_buf(allocBufWrapper<unsigned int>(alpaka::getDev(queue), endcapGeometrySrc.nEndCapMap)),
+      geoMapPhi_buf(allocBufWrapper<float>(alpaka::getDev(queue), endcapGeometrySrc.nEndCapMap)),
+      nEndCapMap(endcapGeometrySrc.nEndCapMap) {
+  alpaka::memcpy(queue, geoMapPhi_buf, endcapGeometrySrc.geoMapPhi_buf);
+  alpaka::memcpy(queue, geoMapDetId_buf, endcapGeometrySrc.geoMapDetId_buf);
 }
 
-void SDL::EndcapGeometryHost<SDL::Dev>::load(std::string filename) {
+template <typename TDev>
+void SDL::EndcapGeometry<TDev>::load(std::string filename) {
   dxdy_slope_.clear();
   centroid_phis_.clear();
 
@@ -38,19 +45,22 @@ void SDL::EndcapGeometryHost<SDL::Dev>::load(std::string filename) {
       }
     }
   }
+
+  fillGeoMapArraysExplicitHost();
 }
 
-void SDL::EndcapGeometry<SDL::Dev>::fillGeoMapArraysExplicit(SDL::QueueAcc& queue) {
+template <typename TDev>
+void SDL::EndcapGeometry<TDev>::fillGeoMapArraysExplicitHost() {
   unsigned int phi_size = centroid_phis_.size();
 
   // Allocate buffers on host
   SDL::DevHost const& devHost = cms::alpakatools::host();
-  auto mapPhi_host_buf = allocBufWrapper<float>(devHost, phi_size);
-  auto mapDetId_host_buf = allocBufWrapper<unsigned int>(devHost, phi_size);
+  geoMapPhi_buf = allocBufWrapper<float>(devHost, phi_size);
+  geoMapDetId_buf = allocBufWrapper<unsigned int>(devHost, phi_size);
 
   // Access the raw pointers of the buffers
-  float* mapPhi = alpaka::getPtrNative(mapPhi_host_buf);
-  unsigned int* mapDetId = alpaka::getPtrNative(mapDetId_host_buf);
+  float* mapPhi = alpaka::getPtrNative(geoMapPhi_buf);
+  unsigned int* mapDetId = alpaka::getPtrNative(geoMapDetId_buf);
 
   unsigned int counter = 0;
   for (auto it = centroid_phis_.begin(); it != centroid_phis_.end(); ++it) {
@@ -62,24 +72,18 @@ void SDL::EndcapGeometry<SDL::Dev>::fillGeoMapArraysExplicit(SDL::QueueAcc& queu
   }
 
   nEndCapMap = counter;
-
-  // Copy data from host to device buffers
-  alpaka::memcpy(queue, geoMapPhi_buf, mapPhi_host_buf);
-  alpaka::memcpy(queue, geoMapDetId_buf, mapDetId_host_buf);
-  alpaka::wait(queue);
 }
 
-float SDL::EndcapGeometry<SDL::Dev>::getdxdy_slope(unsigned int detid) const {
-  if (dxdy_slope_.find(detid) != dxdy_slope_.end()) {
-    return dxdy_slope_.at(detid);
-  } else {
-    return 0;
-  }
-}
-float SDL::EndcapGeometryHost<SDL::Dev>::getdxdy_slope(unsigned int detid) const {
+template <typename TDev>
+float SDL::EndcapGeometry<TDev>::getdxdy_slope(unsigned int detid) const {
   if (dxdy_slope_.find(detid) != dxdy_slope_.end()) {
     return dxdy_slope_.at(detid);
   } else {
     return 0;
   }
 }
+
+// We need to instantiate these to ensure that the compiler generates them
+template class SDL::EndcapGeometry<SDL::DevHost>;
+template SDL::EndcapGeometry<SDL::Dev>::EndcapGeometry(SDL::QueueAcc& queue,
+                                                       SDL::EndcapGeometry<SDL::DevHost> const& endcapGeometrySrc);

RecoTracker/LSTCore/src/alpaka/EndcapGeometry.h

@@ -19,43 +19,29 @@
 
 namespace SDL {
 
-  // FIXME: Need to separate this better into host and device classes
-  // This is only needed for host, but we template it to avoid symbol conflicts
   template <typename TDev>
-  class EndcapGeometryHost;
-
-  template <>
-  class EndcapGeometryHost<Dev> {
-  public:
-    std::map<unsigned int, float> dxdy_slope_;     // dx/dy slope
-    std::map<unsigned int, float> centroid_phis_;  // centroid phi
-
-    EndcapGeometryHost() = default;
-    ~EndcapGeometryHost() = default;
-
-    void load(std::string);
-    float getdxdy_slope(unsigned int detid) const;
-  };
-
-  template <typename TDev>
-  class EndcapGeometry;
-
-  template <>
-  class EndcapGeometry<Dev> {
+  class EndcapGeometry {
   private:
     std::map<unsigned int, float> dxdy_slope_;     // dx/dy slope
     std::map<unsigned int, float> centroid_phis_;  // centroid phi
 
+    // Friend all other instantiations of this template
+    template <typename OtherTDev>
+    friend class EndcapGeometry;
+
   public:
-    Buf<SDL::Dev, unsigned int> geoMapDetId_buf;
-    Buf<SDL::Dev, float> geoMapPhi_buf;
+    Buf<TDev, unsigned int> geoMapDetId_buf;
+    Buf<TDev, float> geoMapPhi_buf;
 
     unsigned int nEndCapMap;
 
-    EndcapGeometry(Dev const& devAccIn, QueueAcc& queue, SDL::EndcapGeometryHost<Dev> const& endcapGeometryIn);
+    EndcapGeometry(TDev const& devAccIn);
+    template <typename TQueue>
+    EndcapGeometry(TQueue& queue, EndcapGeometry<DevHost> const& endcapGeometrySrc);
     ~EndcapGeometry() = default;
 
-    void fillGeoMapArraysExplicit(QueueAcc& queue);
+    void load(std::string);
+    void fillGeoMapArraysExplicitHost();
     float getdxdy_slope(unsigned int detid) const;
   };
 }  // namespace SDL

RecoTracker/LSTCore/src/alpaka/Event.h

@@ -97,7 +97,7 @@ namespace SDL {
   public:
     // Constructor used for CMSSW integration. Uses an external queue.
     template <typename TQueue>
-    Event(bool verbose, TQueue const& q, const LSTESDeviceData<Dev>* deviceESData)
+    Event(bool verbose, TQueue const& q, const LSTESData<Dev>* deviceESData)
         : queue(q),
           devAcc(alpaka::getDev(q)),
           devHost(cms::alpakatools::host()),

RecoTracker/LSTCore/src/alpaka/LST.dev.cc

@@ -11,7 +11,7 @@ using XYZVector = ROOT::Math::XYZVector;
 
 void SDL::LST<SDL::Acc>::run(SDL::QueueAcc& queue,
                              bool verbose,
-                             const LSTESDeviceData<SDL::Dev>* deviceESData,
+                             const LSTESData<SDL::Dev>* deviceESData,
                              const std::vector<float> see_px,
                              const std::vector<float> see_py,
                              const std::vector<float> see_pz,