Add scalar to macro-based SoA example #38877

fwyzard · 2022-07-28T03:38:57Z

PR description:

This PR is a follow up to #38855.
It fixes the return types used in some of the View and ConstView accessors, and adds an example of using scalars in a SoA.

PR validation:

All unit tests pass.

DataFormats/PortableTestObjects/src/classes_def.xml

fwyzard · 2022-07-28T03:41:43Z

DataFormats/PortableTestObjects/src/classes_def.xml

@@ -6,7 +6,7 @@
    <field name="y_" comment="[nElements_]"/>
    <field name="z_" comment="[nElements_]"/>
    <field name="id_" comment="[nElements_]"/>


@ericcano @makortel
I am thinking about renaming nElements_ to elements_, as a parallel to scalar_.
What do you think ?

I don't have strong feelings either way.

As long as the name is clear (and both are), I don't have a strong opinion either.

fwyzard · 2022-07-28T03:43:28Z

DataFormats/PortableTestObjects/src/classes_def.xml

@@ -6,7 +6,7 @@
    <field name="y_" comment="[nElements_]"/>
    <field name="z_" comment="[nElements_]"/>
    <field name="id_" comment="[nElements_]"/>
-    <field name="metadata()" comment="!"/>
+    <field name="r_" comment="[scalar_]"/>


This is the only way I found to make ROOT store a single element owned via a pointer.
The scant documentation seems to suggest that -> should be used for that, but I couldn't get it to work.

This is the only way I found to make ROOT store a single element owned via a pointer.
The scant documentation seems to suggest that -> should be used for that, but I couldn't get it to work.

@pcanal, any thoughts?

The object pointeed to by a pointer to a class are stored without any annotation. For pointer to a class, the annotation '->' means that the pointer is guaranteed to never be a nullptr (i.e. the default constructor allocates the object).

For numerical type, it is assumed (because a pointer to a single value is longer than just the value) that it points to an array of numerical elements and indeed, you need to give a size.

I see.

So if I wrap the "scalar" type in a struct (or union ?) I could use the -> syntax ?

With a struct and if you initialize the pointer to non-zero in the default constructor, yes you can use -> (it wont work for a union).

fwyzard · 2022-07-28T03:43:59Z

enable gpu

fwyzard · 2022-07-28T03:44:04Z

please test

cmsbuild · 2022-07-28T03:45:57Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38877/31301

This PR adds an extra 32KB 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-38877/31301/code-format.patch
e.g. curl -k https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38877/31301/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

fwyzard · 2022-07-28T04:04:21Z

enable gpu

fwyzard · 2022-07-28T04:04:24Z

please test

cmsbuild · 2022-07-28T04:10:29Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38877/31302

This PR adds an extra 32KB to repository

cmsbuild · 2022-07-28T04:10:57Z

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

It involves the following packages:

DataFormats/PortableTestObjects (heterogeneous)
DataFormats/SoATemplate (heterogeneous)
HeterogeneousCore/AlpakaTest (heterogeneous)

@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

cmsbuild · 2022-07-28T07:53:46Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-82d89f/26500/summary.html
COMMIT: 22a63e1
CMSSW: CMSSW_12_5_X_2022-07-27-2300/el8_amd64_gcc10
Additional Tests: GPU
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/38877/26500/install.sh to create a dev area with all the needed externals and cmssw changes.

GPU Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
Reco comparison had 3 failed jobs
DQMHistoTests: Total files compared: 4
DQMHistoTests: Total histograms compared: 19876
DQMHistoTests: Total failures: 8
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 19868
DQMHistoTests: Total skipped: 0
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 3 files compared)
Checked 12 log files, 9 edm output root files, 4 DQM output files
TriggerResults: no differences found

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 51
DQMHistoTests: Total histograms compared: 3667670
DQMHistoTests: Total failures: 2
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3667646
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

makortel · 2022-07-28T18:42:01Z

DataFormats/SoATemplate/interface/SoALayout.h

@@ -525,6 +525,7 @@
    /* data members */                                                                                                 \
    std::byte* mem_;                                                                                                   \
    size_type nElements_;                                                                                              \
+    size_type const scalar_ = 1;                                                                                       \


Is the scalar_ needed only for the iorule? Could it be constexpr or static?

Is the scalar_ needed only for the iorule?

Yes.

Could it be constexpr or static?

No, that doesn't work :-/

cmsbuild · 2022-08-16T12:18:18Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38877/31584

This PR adds an extra 40KB to repository

cmsbuild · 2022-08-16T12:18:48Z

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

cmsbuild · 2022-08-16T16:11:21Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-82d89f/26850/summary.html
COMMIT: 3cbc0ec
CMSSW: CMSSW_12_5_X_2022-08-16-1100/el8_amd64_gcc10
Additional Tests: GPU
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/38877/26850/install.sh to create a dev area with all the needed externals and cmssw changes.

GPU Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
Reco comparison had 3 failed jobs
DQMHistoTests: Total files compared: 4
DQMHistoTests: Total histograms compared: 19876
DQMHistoTests: Total failures: 8
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 19868
DQMHistoTests: Total skipped: 0
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 3 files compared)
Checked 12 log files, 9 edm output root files, 4 DQM output files
TriggerResults: found differences in 2 / 3 workflows

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: 3692500
DQMHistoTests: Total failures: 8
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3692470
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 50 files compared)
Checked 212 log files, 49 edm output root files, 51 DQM output files
TriggerResults: no differences found

fwyzard · 2022-08-23T10:59:11Z

+heterogeneous

fwyzard · 2022-08-23T10:59:33Z

let's move forward like this, and postpone any further improvements to future PRs

cmsbuild · 2022-08-23T10:59:40Z

This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). 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)

perrotta · 2022-08-23T12:21:38Z

@fwyzard are the changes observed in wf 11634.506 expected as a consequence of the fixes integrated here? e.g. for SiPixelHeterogeneous_PixelTrackSoA:

There are apparently reco differences also for the wfs 11634.512 and 11634.522, but the plot of the differences do not show up in https://cmssdt.cern.ch/SDT/jenkins-artifacts/baseLineComparisonsGPU/CMSSW_12_5_X_2022-08-16-1100+82d89f/52160/validateJR.html,

fwyzard · 2022-08-23T12:23:39Z

hi @perrotta,
I don't think so: this PR touches only code that is not yet used anywhere, so it shouldn't affect any workflow.

fwyzard · 2022-08-23T15:47:08Z

please test

mmusich · 2022-08-23T17:03:52Z

@perrotta

to clarify on top of the question asked at the ORP meeting of Aug 23rd.
edup stands for "early duplicates" and denotes a set of tracks which is not even fitted and therefore is not used for any downstream reconstruction, and which is known to be very sensitive to the order of operations executed.
I think the fluctuation you see is the somewhat "normal" irreproducibility in the tracking GPU workflows. As long as it doesn't affect anything "loose and above" we don't need to worry.
Moreover as Andrea said this PR is just adding new code that is not used anywhere yet.

cmsbuild · 2022-08-23T19:41:38Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-82d89f/27020/summary.html
COMMIT: 3cbc0ec
CMSSW: CMSSW_12_5_X_2022-08-23-1100/el8_amd64_gcc10
Additional Tests: GPU
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/38877/27020/install.sh to create a dev area with all the needed externals and cmssw changes.

GPU Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
Reco comparison had 2 failed jobs
DQMHistoTests: Total files compared: 4
DQMHistoTests: Total histograms compared: 19876
DQMHistoTests: Total failures: 8
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 19868
DQMHistoTests: Total skipped: 0
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 3 files compared)
Checked 12 log files, 9 edm output root files, 4 DQM output files
TriggerResults: found differences in 3 / 3 workflows

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: 3693084
DQMHistoTests: Total failures: 8
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3693054
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 50 files compared)
Checked 212 log files, 49 edm output root files, 51 DQM output files
TriggerResults: no differences found

perrotta · 2022-08-23T20:18:32Z

Definitely not reprodicible:

perrotta · 2022-08-23T20:19:18Z

+1

cmsbuild added this to the CMSSW_12_5_X milestone Jul 28, 2022

cmsbuild added code-checks-pending heterogeneous-pending orp-pending pending-signatures tests-pending labels Jul 28, 2022

fwyzard commented Jul 28, 2022

View reviewed changes

DataFormats/PortableTestObjects/src/classes_def.xml Show resolved Hide resolved

fwyzard commented Jul 28, 2022

View reviewed changes

cmsbuild added tests-started and removed tests-pending labels Jul 28, 2022

cmsbuild added code-checks-rejected and removed code-checks-pending labels Jul 28, 2022

fwyzard force-pushed the alpaka_and_SoA_data_formats branch from ff39dce to 22a63e1 Compare July 28, 2022 04:04

cmsbuild added code-checks-pending and removed code-checks-rejected labels Jul 28, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels Jul 28, 2022

cmsbuild added tests-approved and removed tests-started labels Jul 28, 2022

makortel reviewed Jul 28, 2022

View reviewed changes

cmsbuild mentioned this pull request Jul 31, 2022

Migration of input collection parameters to untracked for CTPPS and DTMonitorModule #38854

Closed

cmsbuild added code-checks-approved and removed code-checks-pending labels Aug 16, 2022

cmsbuild added tests-approved and removed tests-started labels Aug 16, 2022

cmsbuild added fully-signed heterogeneous-approved and removed pending-signatures heterogeneous-pending labels Aug 23, 2022

cmsbuild added tests-started and removed tests-approved labels Aug 23, 2022

cmsbuild added tests-approved and removed tests-started labels Aug 23, 2022

cmsbuild added orp-approved and removed orp-pending labels Aug 23, 2022

cmsbuild merged commit e336eae into cms-sw:master Aug 23, 2022

fwyzard deleted the alpaka_and_SoA_data_formats branch August 23, 2022 20:20

cmsbuild mentioned this pull request Aug 24, 2022

[BuildRules] testing LTO rules cms-sw/cmsdist#8038

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add scalar to macro-based SoA example #38877

Add scalar to macro-based SoA example #38877

fwyzard commented Jul 28, 2022

fwyzard Jul 28, 2022

makortel Jul 28, 2022

ericcano Aug 16, 2022 •

edited

Loading

fwyzard Jul 28, 2022

makortel Jul 28, 2022

pcanal Jul 28, 2022

fwyzard Jul 28, 2022

pcanal Jul 28, 2022

fwyzard commented Jul 28, 2022

fwyzard commented Jul 28, 2022

cmsbuild commented Jul 28, 2022

fwyzard commented Jul 28, 2022

fwyzard commented Jul 28, 2022

cmsbuild commented Jul 28, 2022

cmsbuild commented Jul 28, 2022

cmsbuild commented Jul 28, 2022

makortel Jul 28, 2022

fwyzard Jul 28, 2022

cmsbuild commented Aug 16, 2022

cmsbuild commented Aug 16, 2022

cmsbuild commented Aug 16, 2022

fwyzard commented Aug 23, 2022

fwyzard commented Aug 23, 2022

cmsbuild commented Aug 23, 2022

perrotta commented Aug 23, 2022

fwyzard commented Aug 23, 2022

fwyzard commented Aug 23, 2022

mmusich commented Aug 23, 2022

cmsbuild commented Aug 23, 2022

perrotta commented Aug 23, 2022

perrotta commented Aug 23, 2022

Add scalar to macro-based SoA example #38877

Add scalar to macro-based SoA example #38877

Conversation

fwyzard commented Jul 28, 2022

PR description:

PR validation:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ericcano Aug 16, 2022 • edited Loading

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

fwyzard commented Jul 28, 2022

fwyzard commented Jul 28, 2022

cmsbuild commented Jul 28, 2022

fwyzard commented Jul 28, 2022

fwyzard commented Jul 28, 2022

cmsbuild commented Jul 28, 2022

cmsbuild commented Jul 28, 2022

cmsbuild commented Jul 28, 2022

GPU Comparison Summary

Comparison Summary

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmsbuild commented Aug 16, 2022

cmsbuild commented Aug 16, 2022

cmsbuild commented Aug 16, 2022

GPU Comparison Summary

Comparison Summary

fwyzard commented Aug 23, 2022

fwyzard commented Aug 23, 2022

cmsbuild commented Aug 23, 2022

perrotta commented Aug 23, 2022

fwyzard commented Aug 23, 2022

fwyzard commented Aug 23, 2022

mmusich commented Aug 23, 2022

cmsbuild commented Aug 23, 2022

GPU Comparison Summary

Comparison Summary

perrotta commented Aug 23, 2022

perrotta commented Aug 23, 2022

ericcano Aug 16, 2022 •

edited

Loading