Alpaka implementation of Hcal Local Reconstruction (Mahi)

[kakwok]

PR description:

This PR port the CUDA implementation of Hcal Local Reconstruction (Mahi) to using Alpaka.
Custom SoA data structure used in CUDA for HCAL condition data and rechits are replaced with PortableCollection.
The current Alpaka implementation aims at reproducing the results from the CUDA implementation, no algorithmic changes are made.

There are 4 main pieces involved in the migration:

• digiConverter(hcalDigisProducerPortable): Convert CPU digis into SoA format
• Produce HCAL condition data in SoA format (Multiple producers)
• Mahi kernels (Mahi.dev.cc)
• Convert rechits from SoA to legacy format (HcalRecHitSoAToLegacy)

More details on code design are presented in the recent HLT GPU development meetings:
https://indico.cern.ch/event/1350955/
https://indico.cern.ch/event/1350953/
https://indico.cern.ch/event/1350952/
https://indico.cern.ch/event/1230377/
https://indico.cern.ch/event/1230374/

PR validation:

Alpaka GPU and CPU rechit energies are validated with slightly modified workflow 12434.523 (ttbar simulation).
More validation comparisons are being worked on.

[cmsbuild]

cms-bot internal usage

[cmsbuild]

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44910/40188

• This PR adds an extra 248KB to repository

Code check has found code style and quality issues which could be resolved by applying following patch(s)

• code-format:
  https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44910/40188/code-format.patch
  e.g. curl -k https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44910/40188/code-format.patch | patch -p1
  You can also run scram build code-format to apply code format directly

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44910/40189

• This PR adds an extra 248KB to repository

[cmsbuild]

A new Pull Request was created by @kakwok for master.

It involves the following packages:

• CondFormats/DataRecord (db, alca)
• CondFormats/HcalObjects (db, alca)
• DQM/HcalTasks (dqm)
• DataFormats/HcalDigi (simulation)
• DataFormats/HcalRecHit (reconstruction)
• EventFilter/HcalRawToDigi (reconstruction)
• HeterogeneousCore/AlpakaInterface (heterogeneous)
• RecoLocalCalo/Configuration (reconstruction)
• RecoLocalCalo/HcalRecAlgos (reconstruction)
• RecoLocalCalo/HcalRecProducers (reconstruction)

@syuvivida, @francescobrivio, @makortel, @rvenditti, @mandrenguyen, @fwyzard, @tjavaid, @mdhildreth, @saumyaphor4252, @perrotta, @consuegs, @jfernan2, @cmsbuild, @civanch, @nothingface0, @antoniovagnerini can you please review it and eventually sign? Thanks.
@argiro, @DryRun, @rsreds, @Martin-Grunewald, @PonIlya, @JanChyczynski, @rovere, @makortel, @missirol, @yuanchao, @tocheng, @bsunanda, @apsallid, @mmusich, @ReyerBand, @wang0jin, @sameasy, @thomreis, @mariadalfonso, @youyingli, @abdoulline, @rchatter, @seemasharmafnal this is something you requested to watch as well.
@sextonkennedy, @antoniovilela, @rappoccio you are the release manager for this.

cms-bot commands are listed here

[fwyzard]

type hcal

[fwyzard]

Well, given the #ifdef, the function is defined both for the host and the device, so it makes sense that it doesn't compile as a pure __device__ function.

The canonical alpaka approach would be to have different specialisation for each accelerator - but we can clean this up later.

[mmusich]

@cms-sw/pdmv-l2 @cms-sw/reconstruction-l2 kind ping.

[mandrenguyen]

+1
No changes to reco comparisons.
Visual inspection of code seems ok.

[fwyzard]

@Dr15Jones as far as I can see the @cms-sw/core-l2 signature is required because of the changes in DataFormats/Common/src/classes_def.xml:

 <class name="edm::Wrapper<std::map<edm::ParameterSetID,edm::ParameterSetBlob>>"/>

 <class name="edm::StdArray<short, 4>"/>
+<class name="edm::StdArray<unsigned short, 11>"/>
</lcgdict>

that are needed do store

  using QIE11dataArray = edm::StdArray<uint16_t, QIE11DigiCollection::MAXSAMPLES + Flavor1::HEADER_WORDS>;
  using QIE10dataArray = edm::StdArray<uint16_t, HBHEDataFrame::MAXSAMPLES + Flavor5::HEADER_WORDS>;

defined in DataFormats/HcalDigi/interface/HcalDigiSoA.h (MAXSAMPLES plus HEADER_WORDS is 11 in both cases).

Could you comment if you see any issues with that, or could you sign for @cms-sw/core-l2 this PR and its 14.0.x backport (#45324) ?

[antoniovilela]

@cms-sw/core-l2 @cms-sw/pdmv-l2
Please review & sign.

[antoniovilela]

@cms-sw/core-l2 @cms-sw/pdmv-l2 Please review & sign.

@smuzaffar
Hi Shahzad,
In case you are signing for Core.
Thanks.

[smuzaffar]

+core

core chages in DataFormats/Common/src/classes_def.xml i.e adding new dict for edm::StdArray<unsigned short, 11> look good

[antoniovilela]

+core

core chages in DataFormats/Common/src/classes_def.xml i.e adding new dict for edm::StdArray<unsigned short, 11> look good

Thanks Shahzad.

[antoniovilela]

@cms-sw/pdmv-l2
We will bypass the signature for the master PR, but please have a look and let us know.

[antoniovilela]

+1

[antoniovilela]

merge

[mmusich]

@kakwok @fwyzard seems we missed to protect here the cases where these modules do not exist (I.e. the other tables that are not GRun). This leads to crashes in the integration tests when trying to integrate this in the combined table. Can you provide a fix for this real quick?

[fwyzard]

@mmusich does #45484 work for you ?

[mmusich]

@fwyzard thank you.

does #45484 work for you ?

I think so. Martin was also reporting problems related to the Hcal local reco part of the customization concerning hltTowerMakerForAllSerialSync. Apparently the check at the top

cmssw/HLTrigger/Configuration/python/customizeHLTforAlpaka.py

Line 1106 in b08e6a8

if not hasattr(process, 'HLTDoLocalHcalSequence'):

does not cover all the use cases.

[fwyzard]

But why do we care about these customisations ?
Can we simply stop calling them once HCAL has been migrated ?

[Martin-Grunewald]

I agree and this will be the case!

But before I was just checking & testing things by applying the ALPAKA customisation to ALL subtables, and it ONLY worked with the GRun one, not with any of the others (HIon,PRef, ...)

[mmusich]

But why do we care about these customisations ?
Can we simply stop calling them once HCAL has been migrated ?

In the long term yes, but:
a) we need a customization that allows a correct migration (integration tests functionally working after migration for all the subtables)
b) similarly to what done for pixel and ecal I think we want to keep the customization around to be able to apply it to older menus for private studies.

[fwyzard]

a) we need a customization that allows a correct migration (integration tests functionally working after migration for all the subtables)

/users/fwyzard/CMSSW_14_0_0/from_HLT_v183/HLT/V23 should be fully migrated.

b) similarly to what done for pixel and ecal I think we want to keep the customization around to be able to apply it to older menus for private studies.

Old versions of the GRun menu should already work, though ?

[mmusich]

versions of the GRun menu should already work, though ?

Yes, but only those.

[fwyzard]

Well, I don't see any reason why we should want to apply a customisation intended for the GRun physics menu to configurations that are 1. obsolete, and 2. not physics menus.

If STORM has any specific example of why it would be needed, please explain it.

Otherwise, as far as I'm concerned there is not need to maintain or modify these customisations any further.

[mmusich]

If STORM has any specific example of why it would be needed, please explain it.

Nothing directly comes to mind, but a use case might appear in future. Also I think it's dangerous to keep potentially broken customizations accessible to users

Otherwise, as far as I'm concerned there is not need to maintain or modify these customisations any further.

I appreciate your lack of will to amend this any further. We'll take care of that ourselves.