Skip to content

Conversation

@Electricks94
Copy link
Contributor

PR description:

This PR modifies the test SoACustomizedMethods_t by adding SOA_HOST_DEVICE to all element methods. Doing so, the test is also extended as a CUDA, HIP and Alpaka version to show consistency. Furthermore, the documentation is extended to hint to users of the SoA to use SOA_HOST_DEVICE to ensure that element methods can be used in device kernels.

A similar idea was promoted previously in #49203 by using constexpr to ensure that functions can run in device kernels. However, as shown, this method is not feasible for us due to a bug in the nvcc compiler. Hence, with this PR #49203 can be closed.

@felicepantaleo fyi

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 18, 2025

cms-bot internal usage

@Electricks94
Copy link
Contributor Author

type ngt

@cmsbuild cmsbuild added the ngt label Dec 18, 2025
@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

  • DataFormats/Portable (heterogeneous)
  • DataFormats/SoATemplate (heterogeneous)

@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

@Electricks94
Copy link
Contributor Author

ping

@makortel
Copy link
Contributor

@cmsbuild, please test

@cmsbuild
Copy link
Contributor

+1

Size: This PR adds an extra 16KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-b98190/50756/summary.html
COMMIT: f39007e
CMSSW: CMSSW_16_1_X_2026-01-20-1100/el8_amd64_gcc13
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/49671/50756/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: 52
  • DQMHistoTests: Total histograms compared: 4025536
  • DQMHistoTests: Total failures: 3
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 4025513
  • DQMHistoTests: Total skipped: 20
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 51 files compared)
  • Checked 222 log files, 193 edm output root files, 52 DQM output files
  • TriggerResults: no differences found

Comment on lines 8 to 51
GENERATE_SOA_LAYOUT(SoATemplate,
SOA_COLUMN(float, x),
SOA_COLUMN(float, y),
SOA_COLUMN(float, z),
SOA_COLUMN(double, v_x),
SOA_COLUMN(double, v_y),
SOA_COLUMN(double, v_z),

SOA_ELEMENT_METHODS(

SOA_HOST_DEVICE void normalise() {
float norm_position = square_norm_position();
if (norm_position > 0.0f) {
x() /= norm_position;
y() /= norm_position;
z() /= norm_position;
};
double norm_velocity = square_norm_velocity();
if (norm_velocity > 0.0f) {
v_x() /= norm_velocity;
v_y() /= norm_velocity;
v_z() /= norm_velocity;
};
}),

SOA_CONST_ELEMENT_METHODS(
SOA_HOST_DEVICE float square_norm_position()
const { return sqrt(x() * x() + y() * y() + z() * z()); };

SOA_HOST_DEVICE double square_norm_velocity()
const { return sqrt(v_x() * v_x() + v_y() * v_y() + v_z() * v_z()); };

template <typename T1, typename T2>
SOA_HOST_DEVICE static auto time(T1 pos, T2 vel) {
if (!(vel == 0))
return pos / vel;
return 0.;
}),

SOA_SCALAR(int, detectorType))

using SoA = SoATemplate<>;
using SoAView = SoA::View;
using SoAConstView = SoA::ConstView;
Copy link
Contributor

@fwyzard fwyzard Jan 24, 2026

Choose a reason for hiding this comment

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

Can you move the SoA to a separate header file, and reuse it for all tests (CPU, CUDA, ROCm, and alpaka) ?

Comment on lines 40 to 45
<bin file="SoACustomizedMethods_t.cu" name="SoACustomizedMethodsCuda">
<use name="boost"/>
<use name="catch2"/>
<use name="cuda"/>
<use name="DataFormats/SoATemplate"/>
</bin>
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be wrapped in a <iftool name="cuda"> block.

Comment on lines 47 to 52
<bin file="SoACustomizedMethods_t.hip.cc" name="SoACustomizedMethodsHip">
<use name="boost"/>
<use name="catch2"/>
<use name="rocm"/>
<use name="DataFormats/SoATemplate"/>
</bin>
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be wrapped in a <iftool name="rocm"> block

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-49671/47694

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

@cmsbuild
Copy link
Contributor

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

@@ -0,0 +1,46 @@
#include "DataFormats/SoATemplate/interface/SoALayout.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add

#include <cmath>

for the use of sqrt() ?


template <typename T1, typename T2>
SOA_HOST_DEVICE static auto time(T1 pos, T2 vel) {
if (not(vel == 0))
Copy link
Contributor

Choose a reason for hiding this comment

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

why not more simply

Suggested change
if (not(vel == 0))
if (vel != 0)

?

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-49671/47698

@cmsbuild
Copy link
Contributor

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

@fwyzard
Copy link
Contributor

fwyzard commented Jan 26, 2026

please test

@fwyzard
Copy link
Contributor

fwyzard commented Jan 26, 2026

+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. @ftenchini, @sextonkennedy, @mandrenguyen (and backports should be raised in the release meeting by the corresponding L2)

@cmsbuild
Copy link
Contributor

+1

Size: This PR adds an extra 24KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-b98190/50892/summary.html
COMMIT: 30d94e9
CMSSW: CMSSW_16_1_X_2026-01-26-1100/el8_amd64_gcc13
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/49671/50892/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

@mandrenguyen
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit d678a0a into cms-sw:master Jan 27, 2026
10 checks passed
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.

5 participants