Implement heterogeneous Association Map based on SoA blocks by sbaldu · Pull Request #49995 · cms-sw/cmssw

sbaldu · 2026-02-01T14:30:48Z

PR description:

This PR implements an association map based on SoA blocks. In order to provide the accessors to the map's properties and content, the PR also enables the definition of SoA view methods.

PR validation:

The PR includes a test of the construction and fill of the map and checks the correctness of the content and the accessor methods.

FYI @felicepantaleo

cmsbuild · 2026-02-01T14:31:13Z

cms-bot internal usage

cmsbuild · 2026-02-01T14:33:01Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-49995/47797

cmsbuild · 2026-02-01T14:33:27Z

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

It involves the following packages:

DataFormats/AssociationMap (****)
DataFormats/SoATemplate (heterogeneous)

The following packages do not have a category, yet:

DataFormats/AssociationMap
Please create a PR for https://github.com/cms-sw/cms-bot/blob/master/categories_map.py to assign category

@cmsbuild, @fwyzard, @makortel can you please review it and eventually sign? Thanks.
@missirol, @mmusich, @rovere this is something you requested to watch as well.
@ftenchini, @mandrenguyen, @sextonkennedy you are the release manager for this.

cms-bot commands are listed here

fwyzard · 2026-02-02T08:53:35Z

type ngt

makortel · 2026-02-02T14:25:03Z

+#include <cstdint>
+#include <type_traits>
+
+namespace ticl {


Is this utility specific to TICL or intended for more general usage?

As of now the main usage is TICL

Would it be feasible to include this code in a package that already uses TICL?

Or, the motivation to place this code in its own package is not clear.

Thanks for the suggestion. I've now put the map under a new DataFormats/TICL package

With @felicepantaleo we decided to add a new package for TICL data formats and we'll start moving the structures specific for TICL there

With @felicepantaleo we decided to add a new package for TICL data formats and we'll start moving the structures specific for TICL there

Ok. I guess at least @cms-sw/reconstruction-l2 should sign it? What about @cms-sw/heterogeneous-l2?

What about @cms-sw/heterogeneous-l2?

This too, I guess 🤷🏻‍♂️

cmsbuild · 2026-02-02T15:59:58Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-49995/47813

cmsbuild · 2026-02-02T16:00:18Z

Pull request #49995 was updated. @cmsbuild, @fwyzard, @makortel can you please check and sign again.

cmsbuild · 2026-02-03T11:23:28Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-49995/47834

cmsbuild · 2026-02-03T11:23:56Z

Pull request #49995 was updated. @cmsbuild, @fwyzard, @makortel can you please check and sign again.

Moanwar · 2026-08-17T12:50:06Z

+1

Parsifal-2045 · 2026-08-18T09:33:50Z

I think that fixing the previous off-by-one issues has introduced some new unwanted behaviour (that escaped the current testing). Now the OffsetsLayout correctly stores the requested nkeys + 1 so that the count() method works as expected, then AssociationMap just returns size() - 1 to compensate (AssociationMap.h:62-64).
The issue is that now the number passed by the caller and what the block holds are different and the value stored in the SoABlocks depends on which constructor was used:

The constructor relying on the user-provided layout (link) copies what the caller passed to the constructor (let's say nkeys)
The constructor using per-block views asks each view how many elements it actually contains (which would be nkeys + 1).

The override of the SoABlocks sizes in AssociationMap makes the two disagree so that building a map with nkeys = 2 one would get:

map.view().metadata().size()[1] == 2
map.view().offsets().metadata().size() == 3
AssociationMap<>::const_cast_View(map.const_view()).metadata().size()[1] == 3
which can be easily checked via something like

CHECK( map.view().metadata().size()[1] == map.view().offsets().metadata().size() )

I think this "disagreement" might have multiple repercussions (e.g. probably in the deepCopy check) so I would suggest to drop the OffsetsLayout override, prefer building always with nkeys + 1 and having keys() stay size() - 1 for consistent size reporting. Note that dropping the override would most likely need to go together with #49995 (comment) to not break fill()

cmsbuild · 2026-08-18T10:15:59Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-49995/50648

There are other open Pull requests which might conflict with changes you have proposed:
- File DataFormats/SoATemplate/README.md modified in PR(s): [Draft] Allow users to set a valid data range for the SoA View #49204, Introspection for SoABlocks #51670
- File DataFormats/SoATemplate/interface/SoABlocks.h modified in PR(s): Introspection for SoABlocks #51670

cmsbuild · 2026-08-18T10:16:20Z

Pull request #49995 was updated. @Moanwar, @cmsbuild, @fwyzard, @jfernan2, @makortel, @mandrenguyen, @srimanob can you please check and sign again.

cmsbuild · 2026-08-19T13:36:37Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-49995/50664

There are other open Pull requests which might conflict with changes you have proposed:
- File DataFormats/SoATemplate/README.md modified in PR(s): [Draft] Allow users to set a valid data range for the SoA View #49204, Introspection for SoABlocks #51670
- File DataFormats/SoATemplate/interface/SoABlocks.h modified in PR(s): Introspection for SoABlocks #51670

cmsbuild · 2026-08-19T13:37:07Z

Pull request #49995 was updated. @Moanwar, @cmsbuild, @fwyzard, @jfernan2, @makortel, @mandrenguyen, @srimanob can you please check and sign again.

felicepantaleo · 2026-08-19T17:48:09Z

please test

cmsbuild · 2026-08-19T21:16:26Z

-1

Failed Tests: RelVals-AMD_MI300X
Size: This PR adds an extra 24KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-9aee20/55469/summary.html
COMMIT: 9b6e60a
CMSSW: CMSSW_20_1_X_2026-08-19-1100/el9_amd64_gcc14
Additional Tests: GPU,AMD_MI300X,AMD_W7900,NVIDIA_H100,NVIDIA_L40S,NVIDIA_T4
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/49995/55469/install.sh to create a dev area with all the needed externals and cmssw changes.

DAS Queries: The DAS query tests failed, see the summary page for details.

Failed RelVals-AMD_MI300X

37634.40237634.402_TTbar_14TeV+Run4D127_Patatrack_PixelOnlyAlpaka/step2_TTbar_14TeV+Run4D127_Patatrack_PixelOnlyAlpaka.log
37634.40437634.404_TTbar_14TeV+Run4D127_Patatrack_PixelOnlyAlpaka_Profiling/step2_TTbar_14TeV+Run4D127_Patatrack_PixelOnlyAlpaka_Profiling.log
37634.750337634.7503_TTbar_14TeV+Run4D127_HLTHeterogeneousValid/step2_TTbar_14TeV+Run4D127_HLTHeterogeneousValid.log

Expand to see more relval errors ...

37634.751

Comparison Summary

Summary:

You potentially removed 1 lines from the logs
Reco comparison results: 4 differences found in the comparisons
DQMHistoTests: Total files compared: 45
DQMHistoTests: Total histograms compared: 3592269
DQMHistoTests: Total failures: 3
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3592248
DQMHistoTests: Total skipped: 18
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 44 files compared)
Checked 195 log files, 163 edm output root files, 45 DQM output files
TriggerResults: no differences found

NVIDIA_H100 Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 83 differences found in the comparisons
DQMHistoTests: Total files compared: 7
DQMHistoTests: Total histograms compared: 173739
DQMHistoTests: Total failures: 8632
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 165107
DQMHistoTests: Total skipped: 0
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 6 files compared)
Checked 25 log files, 20 edm output root files, 7 DQM output files
TriggerResults: no differences found

NVIDIA_L40S Comparison Summary

Summary:

You potentially removed 1 lines from the logs
Reco comparison results: 116 differences found in the comparisons
DQMHistoTests: Total files compared: 7
DQMHistoTests: Total histograms compared: 173739
DQMHistoTests: Total failures: 9292
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 164447
DQMHistoTests: Total skipped: 0
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 6 files compared)
Checked 25 log files, 20 edm output root files, 7 DQM output files
TriggerResults: no differences found

NVIDIA_T4 Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 86 differences found in the comparisons
DQMHistoTests: Total files compared: 7
DQMHistoTests: Total histograms compared: 173739
DQMHistoTests: Total failures: 8970
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 164769
DQMHistoTests: Total skipped: 0
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 6 files compared)
Checked 25 log files, 20 edm output root files, 7 DQM output files
TriggerResults: no differences found

Max Memory Comparisons exceeding threshold NVIDIA_H100

@cms-sw/core-l2 , I found 1 workflow step(s) with memory usage exceeding the error threshold:

Expand to see workflows ...

Error: Workflow 37634.7503_TTbar_14TeV+Run4D127_HLTHeterogeneousValid step2 max memory diff -200.1 exceeds +/- 30.0 MiB

Max Memory Comparisons exceeding threshold NVIDIA_T4

@cms-sw/core-l2 , I found 1 workflow step(s) with memory usage exceeding the error threshold:

Expand to see workflows ...

Error: Workflow 37634.7503_TTbar_14TeV+Run4D127_HLTHeterogeneousValid step2 max memory diff 30.1 exceeds +/- 30.0 MiB

fwyzard · 2026-08-20T07:56:35Z

ignore tests-rejected with ib-failure

sbaldu · 2026-08-20T08:16:25Z

I think that fixing the previous off-by-one issues has introduced some new unwanted behaviour (that escaped the current testing). Now the OffsetsLayout correctly stores the requested nkeys + 1 so that the count() method works as expected, then AssociationMap just returns size() - 1 to compensate (AssociationMap.h:62-64). The issue is that now the number passed by the caller and what the block holds are different and the value stored in the SoABlocks depends on which constructor was used:

The constructor relying on the user-provided layout (link) copies what the caller passed to the constructor (let's say nkeys)

The constructor using per-block views asks each view how many elements it actually contains (which would be nkeys + 1).

The override of the SoABlocks sizes in AssociationMap makes the two disagree so that building a map with nkeys = 2 one would get:

map.view().metadata().size()[1] == 2

map.view().offsets().metadata().size() == 3

AssociationMap<>::const_cast_View(map.const_view()).metadata().size()[1] == 3
which can be easily checked via something like
CHECK( map.view().metadata().size()[1] == map.view().offsets().metadata().size() )
I think this "disagreement" might have multiple repercussions (e.g. probably in the deepCopy check) so I would suggest to drop the OffsetsLayout override, prefer building always with nkeys + 1 and having keys() stay size() - 1 for consistent size reporting. Note that dropping the override would most likely need to go together with #49995 (comment) to not break fill()

I added a test specifically for the deepCopy and it works as expected. With @Electricks94 we also tested reading the map from ROOT files, and that also works correctly, producing a map with the correct content and sizes.
Regarding the different sizes, I agree that it could be confusing, but that's also why we provided the keys and size methods for the View, so in this case that would be the intended interface rather than using metadata().size(), also because the offsets layout and its actual size are implementation details.

fwyzard · 2026-08-20T09:24:26Z

+                                    std::span<const uint32_t>{device_associations.data(), nvalues},
+                                    std::span<const uint32_t>{device_values.data(), nvalues});


I'm curious if explicitly building a std::span is really needed, given that an alpaka buffer should be automatically convertible to std::span after alpaka-group/alpaka#2579 ?

I tried but it doesn't compile. I think it's because associator::fill is a template and templates are deduced before the implicit conversion operators are checked

Ah... OK, I still have to figure out what can and cannot (yet) be done with concepts and templates.

Can you try with a simpler constructor or a static_cast ?

Suggested change

std::span<const uint32_t>{device_associations.data(), nvalues},

std::span<const uint32_t>{device_values.data(), nvalues});

std::span<const uint32_t>{device_associations},

std::span<const uint32_t>{device_values});

Yes that works, I'll add it together with the additions to the tests discussed below

cmsbuild · 2026-08-20T09:47:44Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-49995/50673

There are other open Pull requests which might conflict with changes you have proposed:
- File DataFormats/SoATemplate/README.md modified in PR(s): [Draft] Allow users to set a valid data range for the SoA View #49204, Introspection for SoABlocks #51670
- File DataFormats/SoATemplate/interface/SoABlocks.h modified in PR(s): Introspection for SoABlocks #51670

cmsbuild · 2026-08-20T09:48:07Z

Pull request #49995 was updated. @Moanwar, @cmsbuild, @fwyzard, @jfernan2, @makortel, @mandrenguyen, @srimanob can you please check and sign again.

fwyzard · 2026-08-20T09:50:43Z

Can you add or modify the tests to check some more cases:

the map is not completely full (can it happen for the keys, or only for the values ?)

the map is exactly full (I think all current tests assume this)

the keys overflow (should assert ?)

the values overflow (should assert ?)

the number of keys is 30

the number of keys is 31 (so n+1 is 32 which is on the alignment boundary)

the number of keys is 32

different keys have different number of elements

In the effort to increase the robustness of the tests I would also opt to add a ROOT streamer test that stores and reads a map. There we rely on the interplay between SoABlocks and the layouts it contains. Everything is correct as it is but we should make sure that future developements don't destroy anything here. Especially, since we plan to support RNTuple soon things might change and I think it will be important to test that these changes don't affect anything that is developed in this PR

I have written a test case for this a while ago here. I think it just needs to be added to right place in this PR if found useful

cmsbuild · 2026-08-20T10:06:13Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-49995/50674

There are other open Pull requests which might conflict with changes you have proposed:
- File DataFormats/SoATemplate/README.md modified in PR(s): [Draft] Allow users to set a valid data range for the SoA View #49204, Introspection for SoABlocks #51670
- File DataFormats/SoATemplate/interface/SoABlocks.h modified in PR(s): Introspection for SoABlocks #51670

cmsbuild · 2026-08-20T10:06:38Z

Pull request #49995 was updated. @Moanwar, @cmsbuild, @fwyzard, @jfernan2, @makortel, @mandrenguyen, @srimanob can you please check and sign again.

cmsbuild · 2026-08-20T10:11:38Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-49995/50675

There are other open Pull requests which might conflict with changes you have proposed:
- File DataFormats/SoATemplate/README.md modified in PR(s): [Draft] Allow users to set a valid data range for the SoA View #49204, Introspection for SoABlocks #51670
- File DataFormats/SoATemplate/interface/SoABlocks.h modified in PR(s): Introspection for SoABlocks #51670

cmsbuild · 2026-08-20T10:12:02Z

Pull request #49995 was updated. @Moanwar, @cmsbuild, @fwyzard, @jfernan2, @makortel, @mandrenguyen, @srimanob can you please check and sign again.

fwyzard · 2026-08-20T10:35:32Z

I am lost among the discussion in #49995 (comment) and #49995 (comment) .

I agree with @Parsifal-2045 that having an potentially public inconsistent interface is Bad.

I understand that we need nkeys + 1 entries in the buffer.
(well, maybe not, maybe we could rely on the fact that the first offset is always 0 and store one less offset? but the code would be come more complicated and somewhat less efficient).

I would like to understand what code would produce inconsistent results, and discuss how to address the issue.

cmsbuild added this to the CMSSW_16_1_X milestone Feb 1, 2026

cmsbuild added pending-signatures tests-pending orp-pending new-package-pending code-checks-pending heterogeneous-pending labels Feb 1, 2026

cmsbuild added code-checks-approved and removed code-checks-pending labels Feb 1, 2026

cmsbuild added the ngt label Feb 2, 2026

makortel reviewed Feb 2, 2026

View reviewed changes

cmsbuild added code-checks-pending and removed code-checks-approved labels Feb 2, 2026

cmsbuild added code-checks-approved and removed code-checks-pending labels Feb 2, 2026

fwyzard reviewed Feb 2, 2026

View reviewed changes

Comment thread DataFormats/AssociationMap/interface/FillAssociator.h Outdated

fwyzard reviewed Feb 2, 2026

View reviewed changes

Comment thread DataFormats/AssociationMap/interface/FillAssociator.h Outdated

sbaldu force-pushed the feature/heterogeneous-associator branch from a9ddafa to 9b406b0 Compare February 3, 2026 11:22

cmsbuild added code-checks-pending and removed code-checks-approved labels Feb 3, 2026

cmsbuild added code-checks-approved and removed code-checks-pending labels Feb 3, 2026

Parsifal-2045 reviewed Aug 18, 2026

View reviewed changes

Comment thread DataFormats/TICL/interface/detail/FillAssociator.h Outdated

Add definition of view methods in SoABlocks

fba32a2

Parsifal-2045 reviewed Aug 18, 2026

View reviewed changes

Comment thread DataFormats/TICL/interface/detail/FillAssociator.h Outdated