Adding HeterogeneousCore/AlpakaUtilities and changes into HeterogeneousCore/AlpakaInterface #40932

borzari · 2023-03-02T14:15:50Z

Part 1/? of porting pixel track reconstruction code from CUDA to Alpaka. AlpakaUtilities have methods and tools to ease the work with Alpaka (based and modified from https://github.com/cms-patatrack/pixeltrack-standalone/tree/master/src/alpaka/AlpakaCore). AlpakaInterface changes comprise addition of functions for block/thread iteration inside kernels.

This PR is a suggestion from @makortel during 12th Patatrack Hackathon to split the work in several PRs to make incremental reviews.

cmsbuild · 2023-03-02T14:23:14Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40932/34416

This PR adds an extra 24KB to repository
Found files with invalid states:
- HeterogeneousCore/AlpakaUtilities/interface/initialise.h:
  - Added: db781df
  - Deleted: 8dbc953
- HeterogeneousCore/AlpakaUtilities/interface/demangle.h:
  - Added: db781df
  - Deleted: 8dbc953

cmsbuild · 2023-03-02T14:23:45Z

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

It involves the following packages:

HeterogeneousCore/AlpakaInterface (heterogeneous)
HeterogeneousCore/AlpakaUtilities (****)

The following packages do not have a category, yet:

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

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

cms-bot commands are listed here

cmsbuild · 2023-03-02T14:45:52Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40932/34417

This PR adds an extra 24KB to repository

cmsbuild · 2023-03-02T14:46:16Z

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

makortel · 2023-03-03T16:34:54Z

My first high-level question is if the code really warrants a new package HeterogeneousCore/AlpakaUtilities, or could just be placed in HeterogeneousCore/AlpakaInterface.

Last week we discussed with @fwyzard and @rovere about the placement of VecArray, and the outcome was to just place it/them in HeterogeneousCore/AlpakaInterface. @fwyzard Do you still agree with this decision?

fwyzard · 2023-03-03T16:36:58Z

In principle yes, I didn't look at the implementation in this PR.

From a quick look, this code needs a thorough review (for example, I'm convinced there is a lot of duplication), but I won't have much time until Thursday.

makortel

From first pass

makortel · 2023-03-03T16:36:19Z

HeterogeneousCore/AlpakaUtilities/BuildFile.xml

+<use name="rootcore"/>
+<use name="boost"/>
+<use name="eigen"/>
+<use name="DataFormats/Common"/>
+<use name="DataFormats/Portable"/>
+<use name="DataFormats/SoATemplate"/>


As far as I can tell, none of these dependencies are used. Or is there further code in the pipeline that would bring in some of these dependencies?

makortel · 2023-03-03T16:36:51Z

HeterogeneousCore/AlpakaUtilities/BuildFile.xml

+<use name="DataFormats/Portable"/>
+<use name="DataFormats/SoATemplate"/>
+<use name="HeterogeneousCore/AlpakaInterface"/>
+<flags ALPAKA_BACKENDS="1"/>


The ALPAKA_BACKENDS flag is not needed either as there is no code making use of ALPAKA_ACCELERATOR_NAMESPACE.

I wonder if that even does anything, for files that are just header files and are not inside the alpaka/ subdirectory ?

makortel · 2023-03-03T16:38:22Z

HeterogeneousCore/AlpakaUtilities/interface/HistoContainer.h

+#include "AtomicPairCounter.h"
+#include "alpakastdAlgorithm.h"
+#include "prefixScan.h"


Our convention is use full paths to the headers even if they are in the interface/ directory of the same package (only exceptions are cases where the source and header files are in src, plugins, or test).

makortel · 2023-03-03T16:44:50Z

HeterogeneousCore/AlpakaUtilities/interface/HistoContainer.h

+    };
+
+    template <typename TAcc, typename Histo, typename TQueue>
+    ALPAKA_FN_HOST ALPAKA_FN_INLINE __attribute__((always_inline)) void launchZero(Histo *__restrict__ h,


How necessary is ALPAKA_FN_HOST?

How necessary is ALPAKA_FN_INLINE given the __attribute__((always_inline))?

makortel · 2023-03-03T16:46:36Z

HeterogeneousCore/AlpakaUtilities/interface/HistoContainer.h

+      alpaka::enqueue(
+          queue,
+          alpaka::createTaskKernel<TAcc>(workDiv, multiBlockPrefixScanFirstStep<uint32_t>(), poff, poff, num_items));


How about

Suggested change

alpaka::enqueue(

queue,

alpaka::createTaskKernel<TAcc>(workDiv, multiBlockPrefixScanFirstStep<uint32_t>(), poff, poff, num_items));

alpaka::exec<TAcc>(

queue,

workDiv, multiBlockPrefixScanFirstStep<uint32_t>(), poff, poff, num_items);

instead? (also below)

makortel · 2023-03-03T16:47:50Z

HeterogeneousCore/AlpakaUtilities/interface/HistoContainer.h

+#include "prefixScan.h"
+
+#include "HeterogeneousCore/AlpakaInterface/interface/memory.h"
+#include "HeterogeneousCore/AlpakaInterface/interface/config.h"


On a cursory look the config.h looks like it would not be needed here. Is it needed?

makortel · 2023-03-03T16:49:09Z

HeterogeneousCore/AlpakaUtilities/interface/SimpleVector.h

+#include <type_traits>
+#include <utility>
+
+#include "HeterogeneousCore/AlpakaInterface/interface/config.h"


Can this be replaced as

Suggested change

#include "HeterogeneousCore/AlpakaInterface/interface/config.h"

#include <alpaka/alpaka.hpp>

?

makortel · 2023-03-03T16:49:31Z

HeterogeneousCore/AlpakaUtilities/interface/VecArray.h

+
+#include <utility>
+
+#include "HeterogeneousCore/AlpakaInterface/interface/config.h"


Can this be replaced as

Suggested change

#include "HeterogeneousCore/AlpakaInterface/interface/config.h"

#include <alpaka/alpaka.hpp>

?

makortel · 2023-03-03T16:53:39Z

HeterogeneousCore/AlpakaUtilities/interface/alpakastdAlgorithm.h

+
+  template <typename T = void>
+  struct less {
+    ALPAKA_FN_HOST_ACC constexpr bool operator()(const T &lhs, const T &rhs) const { return lhs < rhs; }


According to cppreference, std::less::operator() has been constexpr since C++14. Is a separate implementation still needed?

makortel · 2023-03-03T16:58:51Z

HeterogeneousCore/AlpakaUtilities/interface/radixSort.h

+#include <cstdint>
+#include <type_traits>
+
+#include "HeterogeneousCore/AlpakaInterface/interface/config.h"


Can this be replaced as

Suggested change

#include "HeterogeneousCore/AlpakaInterface/interface/config.h"

#include <alpaka/alpaka.hpp>

?

fwyzard · 2023-03-03T18:20:06Z

HeterogeneousCore/AlpakaInterface/interface/workdivision.h

@@ -302,6 +302,237 @@ namespace cms::alpakatools {
    const Vec extent_;
  };

+  /*********************************************


@borzari @nothingface0 could you point me to the use cases for

element_index_range_in_block element_index_range_in_block_truncated element_index_range_in_grid element_index_range_in_grid_truncated

and

for_each_element_in_block for_each_element_in_block_strided for_each_element_in_grid_strided

?

Regarding the latter, do you prefer the lambda approach (pass a function object or lambda to the for_each_element_in_... call) or the range loop approach (get the index via a range loop, as with elements_with_stride) ?

element_index_range_in_block

workdivision.h

here and here. It's then used by element_index_range_in_block_truncated and element_index_range_in_grid.

gpuClusterChargeCut.h

here

gpuPixelRecHits.h

here

gpuFishbone.h

here

gpuPixelDoubletsAlgos.h

here

element_index_range_in_block_truncated

workdivision.h

here. See for_each_element_in_block for implicit uses.

element_index_range_in_grid

workdivision.h

Inside for_each_element_in_grid_strided here.

gpuFishbone.h

here

gpuPixelDoubletsAlgos.h

here

element_index_range_in_grid_truncated

workdivision.h

Only used from overloaded function here. Not used anywhere else, can be deleted.

for_each_element_in_block

workdivision.h

Call from overloaded function here.

radixSort.h

here, here, here, here, here.

for_each_element_in_block_strided

workdivision.h

Call from overloaded function here.

radixSort.h

here, here, here, here, here, here, here, here, here, here, here, and here.

for_each_element_in_grid_strided

workdivision.h

Call from overloaded function here.

HistoContainer.h

here, here and here.

Comments

Since we didn't have the time to go in-depth in the patatrack-standalone algorithms, we did not try to change different ways of looping over the elements so that they're more uniform.
That said, personally speaking, the range approach (elements_with_stride) seems more intuitive, and it's cleaner in the code, too.

A visualized overview of the functions that depend on those functions can be seen below for easier(?) comprehension:

fwyzard · 2023-03-03T18:22:42Z

HeterogeneousCore/AlpakaUtilities/interface/AtomicPairCounter.h

+
+    static constexpr c_type incr = 1UL << 32;
+
+    ALPAKA_FN_HOST_ACC Counters get() const { return counter.counters; }


Does this need to be ALPAKA_FN_HOST_ACC or would it be enough to be ALPAKA_FN_ACC ?

fwyzard · 2023-03-03T18:29:25Z

HeterogeneousCore/AlpakaUtilities/interface/alpakastdAlgorithm.h

+// reimplementation of std algorithms able to compile with Alpaka,
+// mostly by declaring them constexpr
+
+namespace alpaka_std {


@makortel a technical question: would it work (and be allowed) to declare this as namespace alpaka::std ?

If we do that, any code inside namespace alpaka { ... } that uses std::lower_bound would pick which version ?

I'd be wary in using namespace std even inside another namespace.

If we do that, any code inside namespace alpaka { ... } that uses std::lower_bound would pick which version ?

Based on our early attempts to use cms::alpaka namespace, and alpaka::<X> types there, I guess std::lower_bound inside namespace alpaka { ... } would resolve to alpaka::std::lower_bound, if the header defining it is #included, and otherwise to ::std::lower_bound. I also guess #includeng a header defining anything in alpaka::std namespace would cause code in alpaka namespace to need to use ::std:: for all std:: names that are not found in alpaka::std.

But within CMSSW we are not going to add names in alpaka namespace, are we?

But within CMSSW we are not going to add names in alpaka namespace, are we?

No, but I'm wondering in case we push these upstream into Alpaka.

cmsbuild · 2023-03-05T12:48:12Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40932/34443

This PR adds an extra 24KB to repository

smuzaffar · 2023-11-06T16:49:48Z

Milestone for this pull request has been moved to CMSSW_14_0_X.Please open a backport if it should also go in to CMSSW_13_3_X.

cmsbuild · 2024-02-06T10:10:00Z

Milestone for this pull request has been moved to CMSSW_14_1_X. Please open a backport if it should also go in to CMSSW_14_0_X.

fwyzard · 2024-02-06T11:11:09Z

@borzari am I right that these changes have been merged as part of the large Alpaka migration ?

If so, can you close this PR ?

borzari · 2024-02-06T15:57:34Z

@borzari am I right that these changes have been merged as part of the large Alpaka migration ?

If so, can you close this PR ?

Hi @fwyzard. Yes, it was all included in the Alpaka migration. I am closing this.

cmsbuild added this to the CMSSW_13_1_X milestone Mar 2, 2023

cmsbuild added pending-signatures tests-pending orp-pending new-package-pending code-checks-pending heterogeneous-pending labels Mar 2, 2023

cmsbuild added code-checks-approved and removed code-checks-pending labels Mar 2, 2023

Adding AlpakaUtilities and changes to Interface

7712e91

nothingface0 force-pushed the AlpakaUtilities branch from 8dbc953 to 7712e91 Compare March 2, 2023 14:39

cmsbuild added code-checks-pending and removed code-checks-approved labels Mar 2, 2023

cmsbuild added code-checks-approved and removed code-checks-pending labels Mar 2, 2023

makortel reviewed Mar 3, 2023

View reviewed changes

fwyzard reviewed Mar 3, 2023

View reviewed changes

Fixed attribute list order

9640e15

cmsbuild added code-checks-pending and removed code-checks-approved labels Mar 5, 2023

cmsbuild removed the code-checks-pending label Mar 5, 2023

cmsbuild mentioned this pull request Aug 13, 2023

Fix elements_with_stride_nd when the index is outside the extent #42558

Merged

cmsbuild mentioned this pull request Aug 29, 2023

[HGCAL trigger] Module splitting and updated seeding normalization #42246

Merged

cmsbuild mentioned this pull request Oct 19, 2023

Ports prefixScan, OneToManyAssoc and HistoContainer from CUDAUtilities. #43064

Merged

smuzaffar modified the milestones: CMSSW_13_3_X, CMSSW_14_0_X Nov 6, 2023

cmsbuild modified the milestones: CMSSW_14_0_X, CMSSW_13_3_X Nov 6, 2023

cmsbuild mentioned this pull request Nov 6, 2023

Introduce new utilities for writing Alpaka kernels #43205

Merged

cmsbuild modified the milestones: CMSSW_13_3_X, CMSSW_14_0_X Nov 6, 2023

This was referenced Nov 14, 2023

ECAL unpacker and ECAL multifit algorithm migration to alpaka #43257

Merged

Add Alpaka Implementation of PFClusterProducer #43130

Merged

This was referenced Dec 21, 2023

Implement independent_groups and independent_group_elements for Alpaka kernels #43624

Merged

Improve uniform_groups-related functions #43632

Merged

cmsbuild modified the milestones: CMSSW_14_0_X, CMSSW_14_1_X Feb 6, 2024

borzari closed this Feb 6, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding HeterogeneousCore/AlpakaUtilities and changes into HeterogeneousCore/AlpakaInterface #40932

Adding HeterogeneousCore/AlpakaUtilities and changes into HeterogeneousCore/AlpakaInterface #40932

borzari commented Mar 2, 2023

cmsbuild commented Mar 2, 2023

cmsbuild commented Mar 2, 2023 •

edited

Loading

cmsbuild commented Mar 2, 2023

cmsbuild commented Mar 2, 2023

makortel commented Mar 3, 2023

fwyzard commented Mar 3, 2023

makortel left a comment

makortel Mar 3, 2023

makortel Mar 3, 2023

fwyzard Mar 3, 2023

makortel Mar 3, 2023

makortel Mar 3, 2023

makortel Mar 3, 2023

makortel Mar 3, 2023

makortel Mar 3, 2023

makortel Mar 3, 2023

makortel Mar 3, 2023

makortel Mar 3, 2023

fwyzard Mar 3, 2023

nothingface0 Mar 6, 2023 •

edited

Loading

fwyzard Mar 3, 2023

fwyzard Mar 3, 2023

makortel Mar 3, 2023

fwyzard Mar 3, 2023

cmsbuild commented Mar 5, 2023

smuzaffar commented Nov 6, 2023

cmsbuild commented Feb 6, 2024

fwyzard commented Feb 6, 2024

borzari commented Feb 6, 2024

	#include "HeterogeneousCore/AlpakaInterface/interface/config.h"
	#include <alpaka/alpaka.hpp>


		#include <utility>

		#include "HeterogeneousCore/AlpakaInterface/interface/config.h"


		static constexpr c_type incr = 1UL << 32;

		ALPAKA_FN_HOST_ACC Counters get() const { return counter.counters; }

Adding HeterogeneousCore/AlpakaUtilities and changes into HeterogeneousCore/AlpakaInterface #40932

Adding HeterogeneousCore/AlpakaUtilities and changes into HeterogeneousCore/AlpakaInterface #40932

Conversation

borzari commented Mar 2, 2023

cmsbuild commented Mar 2, 2023

cmsbuild commented Mar 2, 2023 • edited Loading

cmsbuild commented Mar 2, 2023

cmsbuild commented Mar 2, 2023

makortel commented Mar 3, 2023

fwyzard commented Mar 3, 2023

makortel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nothingface0 Mar 6, 2023 • edited Loading

Choose a reason for hiding this comment

element_index_range_in_block

workdivision.h

gpuClusterChargeCut.h

gpuPixelRecHits.h

gpuFishbone.h

gpuPixelDoubletsAlgos.h

element_index_range_in_block_truncated

workdivision.h

element_index_range_in_grid

workdivision.h

gpuFishbone.h

gpuPixelDoubletsAlgos.h

element_index_range_in_grid_truncated

workdivision.h

for_each_element_in_block

workdivision.h

radixSort.h

for_each_element_in_block_strided

workdivision.h

radixSort.h

for_each_element_in_grid_strided

workdivision.h

HistoContainer.h

Comments

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmsbuild commented Mar 5, 2023

smuzaffar commented Nov 6, 2023

cmsbuild commented Feb 6, 2024

fwyzard commented Feb 6, 2024

borzari commented Feb 6, 2024

cmsbuild commented Mar 2, 2023 •

edited

Loading

nothingface0 Mar 6, 2023 •

edited

Loading

`element_index_range_in_block`

`workdivision.h`

`gpuClusterChargeCut.h`

`gpuPixelRecHits.h`

`gpuFishbone.h`

`gpuPixelDoubletsAlgos.h`

`element_index_range_in_block_truncated`

`workdivision.h`

`element_index_range_in_grid`

`workdivision.h`

`gpuFishbone.h`

`gpuPixelDoubletsAlgos.h`

`element_index_range_in_grid_truncated`

`workdivision.h`

`for_each_element_in_block`

`workdivision.h`

`radixSort.h`

`for_each_element_in_block_strided`

`workdivision.h`

`radixSort.h`

`for_each_element_in_grid_strided`

`workdivision.h`

`HistoContainer.h`