Comments to #43295

[AdrianoDee]

This issue keeps track of the comment addressed for the PRs https://github.com/cms-sw/cmssw/pull/43295

---

15 November 2023

• [x] https://github.com/cms-sw/cmssw/pull/43295#discussion_r1394560150 by @makortel https://github.com/PixelTracksAlpaka/cmssw/blob/783b7c9f3df721d43c03de30c47f850fdd79e0df/DataFormats/BeamSpot/BuildFile.xml#L10

  • Change with:

    <flags ALPAKA_BACKENDS="!serial"/>

    is preferred now

• [x] https://github.com/cms-sw/cmssw/pull/43295#discussion_r1394562904 by @makortel https://github.com/PixelTracksAlpaka/cmssw/blob/783b7c9f3df721d43c03de30c47f850fdd79e0df/DataFormats/BeamSpot/src/classes_def.xml#L8-L9

  • Change with: New class versions should start with 3

    <class name="BeamSpotPOD" ClassVersion="3">
    <version ClassVersion="3" checksum="280341519"/>

• [x] https://github.com/cms-sw/cmssw/pull/43295#discussion_r1394564386 by @makortel https://github.com/PixelTracksAlpaka/cmssw/blob/783b7c9f3df721d43c03de30c47f850fdd79e0df/DataFormats/Portable/interface/PortableDeviceProduct.h#L13 I don't see any relation to SoA. The class stores an object of type T in an Alpaka buffer.

• [x] https://github.com/cms-sw/cmssw/pull/43295#discussion_r1394574931 by @makortel https://github.com/PixelTracksAlpaka/cmssw/blob/783b7c9f3df721d43c03de30c47f850fdd79e0df/DataFormats/Portable/interface/PortableDeviceProduct.h#L15 What is the added value of this class compared to plain Alpaka buffer?

  I can think of

  • can be default-constructed
  • handles the data format const-requirements of the framework correctly

  Are these correct? Is something missing?

  (I'm mostly trying to understand the situation better, e.g. if we could eventually find a way without this level of indirection)

• [x] https://github.com/cms-sw/cmssw/pull/43295#discussion_r1394575456 by @makortel https://github.com/PixelTracksAlpaka/cmssw/blob/783b7c9f3df721d43c03de30c47f850fdd79e0df/DataFormats/Portable/interface/PortableHostProduct.h#L13 Seems to not have anything to do with SoA either.

• [x] https://github.com/cms-sw/cmssw/pull/43295#discussion_r1394576179 by @makortel https://github.com/PixelTracksAlpaka/cmssw/blob/783b7c9f3df721d43c03de30c47f850fdd79e0df/DataFormats/Portable/interface/PortableHostProduct.h#L68 Printouts need to be removed

• [x] https://github.com/cms-sw/cmssw/pull/43295#discussion_r1394578531 by @makortel https://github.com/PixelTracksAlpaka/cmssw/blob/783b7c9f3df721d43c03de30c47f850fdd79e0df/DataFormats/Portable/interface/PortableDeviceProduct.h#L64

  • Change with: Could this be

    Buffer& buffer() { return *buffer_; }

    (although I see the same pattern in PortableDeviceCollection)

• [x] https://github.com/cms-sw/cmssw/pull/43295#discussion_r1394582899 by @makortel https://github.com/PixelTracksAlpaka/cmssw/blob/783b7c9f3df721d43c03de30c47f850fdd79e0df/DataFormats/Portable/interface/PortableHostProduct.h#L15 How about PortableHostObject?

• [ ] https://github.com/cms-sw/cmssw/pull/43295#discussion_r1394587305 by @makortel https://github.com/PixelTracksAlpaka/cmssw/blob/783b7c9f3df721d43c03de30c47f850fdd79e0df/DataFormats/Portable/interface/PortableHostProduct.h#L66 Do I understand correctly this function is not currently called? Could you add a test where it is used?