Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integration of alpaka and macro-based SoA in CMSSW #38855

Merged
merged 11 commits into from Jul 26, 2022

Conversation

fwyzard
Copy link
Contributor

@fwyzard fwyzard commented Jul 25, 2022

PR description:

This PR is a re-posting of #37716 to fix the bot labels.
It introduces examples with the integration of alpaka in CMSSW, and the use of macro-based SoA data structres.

PR validation:

cmsRun HeterogeneousCore/AlpakaTest/test/writer.py
cmsRun HeterogeneousCore/AlpakaTest/test/reader.py

run successfully, both with and without a GPU, using a SwitchProducerCUDA to choose the modules to run.

In both cases, the test.root file created by the writer.py configuration contains the same collections:

$ edmDumpEventContent --all test.root 
Type                                  Module                 Label     Process         Full Name
------------------------------------------------------------------------------------------------
PortableHostCollection<portabletest::TestSoALayout<128,false> >    "testProducer"         ""        "Writer"        128falseportabletestTestSoALayoutPortableHostCollection_testProducer__Writer
PortableHostCollection<portabletest::TestSoALayout<128,false> >    "testProducerSerial"   ""        "Writer"        128falseportabletestTestSoALayoutPortableHostCollection_testProducerSerial__Writer

Open issues, to be followed up in future PRs

fwyzard and others added 11 commits July 22, 2022 08:46
General notes:
  - code in the alpaka subdirectories is compiled multiple times, once
    for each backend; this is controlled by the ALPAKA_BACKENDS flag in
    each package or plugins BuildFile.xml;
  - code in .cc files is always compiled by the host compiler;
  - code in .dev.cc files is compiled the respective device compiler:
    nvcc for CUDA, hipcc for HIP, gcc for the serial or tbb backends.

The HeterogeneousCore/AlpakaInterface package provides an interface to
the header-only alpaka external library.
It provides a definition of the common types and macros used in CMSSW,
of the host types, and of the accelertor devices, queues and events for
each backend.
It also provides a free function to access the common host object.
This package can safely be used by DataFormats packages.

The DataFormats/Portable package implements template wrappers for
persistent host-only collections, transient device-only collections, and
common interface for both.
See DataFormats/Portable/README.md for more information.

The DataFormats/PortableTestObjects package implements portable SoA-based
data formats used for testing the alpaka build rules and the SoA
templates.
The HeterogeneousCore/AlpakaTest package implements modules for
producing, transferring, and analysing those objects.
See HeterogeneousCore/AlpakaTest/test/ for unit tests for writing and
reading SoA data structures.

The HeterogeneousCore/AlpakaServices provides simple services for
checking the availability of devices for each alpaka backend.
Implement a ConstBuffer interface for non-mutable buffers, based on
alpaka::ViewConst<Buffer> .

Remove the LogDebug messages from the constructors and destructors.
This lets the compiler automatically convert a View to a ConstView when
passed by pointer or by reference, emulating the usual conversion rules
for mutable/const pointers and references.

Various improvements:
  - fix const correctness in View::Metadata and SoAColumnAccessorsImpl;
  - make SOA_INLINE always inline also on the host;
  - simplify the Matryoshka templates;
  - use the __trap() intrinsic in gpu code;
  - replace typedef with using;
  - add missing type aliases;
  - add defaulted copy/move ctors and dtors;
  - make various methods constexpr;
  - add and update comments, remove unused methods and macros;
  - improve spacing and indentations;
  - apply code formatting.
This is limited to view trivially deducted from layouts, not the manually defined ones.
@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38855/31256

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @fwyzard (Andrea Bocci) for master.

It involves the following packages:

  • DataFormats/Portable (heterogeneous)
  • DataFormats/PortableTestObjects (heterogeneous)
  • DataFormats/SoATemplate (heterogeneous)
  • HeterogeneousCore/AlpakaCore (heterogeneous)
  • HeterogeneousCore/AlpakaInterface (heterogeneous)
  • HeterogeneousCore/AlpakaServices (****)
  • HeterogeneousCore/AlpakaTest (heterogeneous)

The following packages do not have a category, yet:

HeterogeneousCore/AlpakaServices
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, @rovere this is something you requested to watch as well.
@perrotta, @dpiparo, @qliphy, @rappoccio you are the release manager for this.

cms-bot commands are listed here

@fwyzard
Copy link
Contributor Author

fwyzard commented Jul 25, 2022

please test

@fwyzard
Copy link
Contributor Author

fwyzard commented Jul 25, 2022

+heterogeneous

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next master IBs after it passes the integration tests. This pull request will now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @qliphy, @rappoccio (and backports should be raised in the release meeting by the corresponding L2)

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-d1321e/26442/summary.html
COMMIT: 866bf1d
CMSSW: CMSSW_12_5_X_2022-07-25-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/38855/26442/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 4 differences found in the comparisons
  • DQMHistoTests: Total files compared: 51
  • DQMHistoTests: Total histograms compared: 3667670
  • DQMHistoTests: Total failures: 8
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3667640
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 50 files compared)
  • Checked 210 log files, 47 edm output root files, 51 DQM output files
  • TriggerResults: no differences found

Copy link
Contributor

@perrotta perrotta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fwyzard @makortel I think these "broken rules" can be actually safely broken here, but maybe you want to comment now before merging

namespace portabletest {

// import the top-level portabletest namespace
using namespace ::portabletest;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rule1 : https://raw.githubusercontent.com/cms-sw/cmssw/master/Utilities/ReleaseScripts/python/cmsCodeRules/config.py
Search for "using namespace" or "using std::" in header files

/DataFormats/PortableTestObjects/interface/alpaka/TestDeviceCollection.h
[13]
/AlpakaInterface/interface/config.h
[43, 69, 95, 121]

even though in this context it may probably make sense

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @perrotta for checking. The main reason for this code rule check is to avoid using namespace X to add names into the global namespace. The relevant C++ Core Guideline is https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#sf7-dont-write-using-namespace-at-global-scope-in-a-header-file. This line adds the names into another namespace, which is ok (one could argue our checker is too eager to warn).


#ifdef ALPAKA_ACC_GPU_CUDA_ENABLED
namespace alpaka_cuda_async {
using namespace alpaka_common;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These also probably make sense...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This case is similar, and is ok.

@perrotta
Copy link
Contributor

+1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants