Add a `zeroInitialise()` method to portable objects and collections #45861

fwyzard · 2024-09-02T15:24:54Z

PR description:

Add a zeroInitialise() method to the PortableObject and PortableCollection templates that erases the data in the Buffer by writing zeros (bytes containing '\0') to it:

on the host, zeroInitialise() can be called without arguments to execute immediately, or with a queue to be run asynchronously;
on the device, zeroInitialise() requires a queue and can only be run asynchronously.

Extend the tests in DataFormats/PortableTestObjects and HeterogeneousCore/AlpakaTest to validate the new functionality.

PR validation:

The update unit tests pass on CPU and NVIDIA GPUs.

cmsbuild · 2024-09-02T15:25:17Z

cms-bot internal usage

fwyzard · 2024-09-02T15:28:46Z

enable gpu

fwyzard · 2024-09-02T15:28:49Z

please test

fwyzard · 2024-09-02T15:28:59Z

@makortel could you review this ?

cmsbuild · 2024-09-02T15:30:03Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-45861/41602

There are other open Pull requests which might conflict with changes you have proposed:
- File DataFormats/Portable/interface/PortableDeviceCollection.h modified in PR(s): [RFC] Add tests and rudimentary protections for default-constructed PortableCollections #44844
- File DataFormats/Portable/interface/PortableHostCollection.h modified in PR(s): [RFC] Add tests and rudimentary protections for default-constructed PortableCollections #44844
- File DataFormats/Portable/interface/PortableHostObject.h modified in PR(s): [RFC] Add {Copy,Move}ToDeviceCache<T> class templates and moveToDeviceAsync function template #43969
- File HeterogeneousCore/AlpakaTest/plugins/alpaka/TestAlgo.dev.cc modified in PR(s): [RFC] Add {Copy,Move}ToDeviceCache<T> class templates and moveToDeviceAsync function template #43969
- File HeterogeneousCore/AlpakaTest/plugins/alpaka/TestAlgo.h modified in PR(s): [RFC] Add {Copy,Move}ToDeviceCache<T> class templates and moveToDeviceAsync function template #43969

cmsbuild · 2024-09-02T15:30:27Z

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

It involves the following packages:

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

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

cms-bot commands are listed here

cmsbuild · 2024-09-02T19:12:48Z

+1

Size: This PR adds an extra 32KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-32f48a/41229/summary.html
COMMIT: 2c4f848
CMSSW: CMSSW_14_2_X_2024-09-02-1100/el8_amd64_gcc12
Additional Tests: GPU
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/45861/41229/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially removed 3 lines from the logs
Reco comparison results: 6 differences found in the comparisons
DQMHistoTests: Total files compared: 44
DQMHistoTests: Total histograms compared: 3328315
DQMHistoTests: Total failures: 3
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3328292
DQMHistoTests: Total skipped: 20
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 43 files compared)
Checked 193 log files, 163 edm output root files, 44 DQM output files
TriggerResults: no differences found

GPU Comparison Summary

Summary:

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

fwyzard · 2024-09-04T05:58:25Z

please test

cmsbuild · 2024-09-04T05:58:31Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-45861/41623

There are other open Pull requests which might conflict with changes you have proposed:
- File DataFormats/Portable/interface/PortableDeviceCollection.h modified in PR(s): [RFC] Add tests and rudimentary protections for default-constructed PortableCollections #44844
- File DataFormats/Portable/interface/PortableHostCollection.h modified in PR(s): [RFC] Add tests and rudimentary protections for default-constructed PortableCollections #44844
- File DataFormats/Portable/interface/PortableHostObject.h modified in PR(s): [RFC] Add {Copy,Move}ToDeviceCache<T> class templates and moveToDeviceAsync function template #43969
- File HeterogeneousCore/AlpakaTest/plugins/alpaka/TestAlgo.dev.cc modified in PR(s): [RFC] Add {Copy,Move}ToDeviceCache<T> class templates and moveToDeviceAsync function template #43969
- File HeterogeneousCore/AlpakaTest/plugins/alpaka/TestAlgo.h modified in PR(s): [RFC] Add {Copy,Move}ToDeviceCache<T> class templates and moveToDeviceAsync function template #43969

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-45861/41623/code-format.patch
e.g. curl -k https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-45861/41623/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

fwyzard · 2024-09-04T06:00:39Z

please test

cmsbuild · 2024-09-04T06:04:39Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-45861/41624

There are other open Pull requests which might conflict with changes you have proposed:
- File DataFormats/Portable/interface/PortableDeviceCollection.h modified in PR(s): [RFC] Add tests and rudimentary protections for default-constructed PortableCollections #44844
- File DataFormats/Portable/interface/PortableHostCollection.h modified in PR(s): [RFC] Add tests and rudimentary protections for default-constructed PortableCollections #44844
- File DataFormats/Portable/interface/PortableHostObject.h modified in PR(s): [RFC] Add {Copy,Move}ToDeviceCache<T> class templates and moveToDeviceAsync function template #43969
- File HeterogeneousCore/AlpakaTest/plugins/alpaka/TestAlgo.dev.cc modified in PR(s): [RFC] Add {Copy,Move}ToDeviceCache<T> class templates and moveToDeviceAsync function template #43969
- File HeterogeneousCore/AlpakaTest/plugins/alpaka/TestAlgo.h modified in PR(s): [RFC] Add {Copy,Move}ToDeviceCache<T> class templates and moveToDeviceAsync function template #43969

cmsbuild · 2024-09-04T06:05:04Z

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

cmsbuild · 2024-09-04T09:44:52Z

+1

Size: This PR adds an extra 28KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-32f48a/41268/summary.html
COMMIT: 146b187
CMSSW: CMSSW_14_2_X_2024-09-03-2300/el8_amd64_gcc12
Additional Tests: GPU
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/45861/41268/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: 15 differences found in the comparisons
DQMHistoTests: Total files compared: 44
DQMHistoTests: Total histograms compared: 3328501
DQMHistoTests: Total failures: 17
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3328464
DQMHistoTests: Total skipped: 20
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 43 files compared)
Checked 193 log files, 163 edm output root files, 44 DQM output files
TriggerResults: no differences found

GPU Comparison Summary

Summary:

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

makortel

Looks good to me

makortel · 2024-09-05T18:15:59Z

DataFormats/Portable/interface/PortableDeviceCollection.h

@@ -70,6 +70,12 @@ class PortableDeviceCollection {
  ConstBuffer buffer() const { return *buffer_; }
  ConstBuffer const_buffer() const { return *buffer_; }

+  // erases the data in the Buffer by writing zeros (bytes containing '\0') to it
+  template <typename TQueue, typename = std::enable_if_t<alpaka::isQueue<TQueue>>>
+  void zeroInitialise(TQueue&& queue) {


Not for this PR, but should the constructor overload(s) be updated to use forwarding reference for the TDev and TQueue?

fwyzard · 2024-09-07T07:54:58Z

+heterogeneous

cmsbuild · 2024-09-07T07:55:23Z

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

mandrenguyen · 2024-09-08T08:04:37Z

+1

cmsbuild added this to the CMSSW_14_2_X milestone Sep 2, 2024

cmsbuild added pending-signatures tests-pending orp-pending code-checks-pending heterogeneous-pending labels Sep 2, 2024

fwyzard changed the title ~~git log -n2Portable collection zero memory~~ Add an erase() method to the Portable objects and collections Sep 2, 2024

fwyzard force-pushed the PortableCollection_zero_memory branch from 5904f72 to 2c4f848 Compare September 2, 2024 15:26

cmsbuild added tests-started and removed tests-pending labels Sep 2, 2024

cmsbuild added code-checks-approved and removed code-checks-pending labels Sep 2, 2024

cmsbuild added tests-approved and removed tests-started labels Sep 2, 2024

fwyzard force-pushed the PortableCollection_zero_memory branch from 2c4f848 to 4f051dd Compare September 4, 2024 05:56

cmsbuild added tests-pending code-checks-pending and removed tests-approved code-checks-approved labels Sep 4, 2024

fwyzard changed the title ~~Add an erase() method to the Portable objects and collections~~ Add a method to zeroInitialise() Portable objects and collections Sep 4, 2024

fwyzard changed the title ~~Add a method to zeroInitialise() Portable objects and collections~~ Add a zeroInitialise() method to portable objects and collections Sep 4, 2024

Add tests for the zeroInitialise() method

146b187

fwyzard force-pushed the PortableCollection_zero_memory branch from 4f051dd to 146b187 Compare September 4, 2024 06:00

cmsbuild added tests-pending code-checks-pending and removed tests-started code-checks-rejected labels Sep 4, 2024

cmsbuild added tests-started and removed tests-pending labels Sep 4, 2024

cmsbuild added code-checks-approved and removed code-checks-pending labels Sep 4, 2024

cmsbuild added tests-approved and removed tests-started labels Sep 4, 2024

makortel reviewed Sep 5, 2024

View reviewed changes

cmsbuild added fully-signed heterogeneous-approved and removed pending-signatures heterogeneous-pending labels Sep 7, 2024

cmsbuild added orp-approved and removed orp-pending labels Sep 8, 2024

cmsbuild merged commit 82dc60c into cms-sw:master Sep 8, 2024
14 checks passed

fwyzard mentioned this pull request Sep 9, 2024

CMSSW Integration of LST #45117

Open

3 tasks

cmsbuild mentioned this pull request Sep 9, 2024

[TF] Update TF v2.16.1 (without libfft) cms-sw/cmsdist#9388

Closed

fwyzard deleted the PortableCollection_zero_memory branch September 13, 2024 16:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a `zeroInitialise()` method to portable objects and collections #45861

Add a `zeroInitialise()` method to portable objects and collections #45861

fwyzard commented Sep 2, 2024 •

edited

Loading

cmsbuild commented Sep 2, 2024 •

edited

Loading

fwyzard commented Sep 2, 2024

fwyzard commented Sep 2, 2024

fwyzard commented Sep 2, 2024

cmsbuild commented Sep 2, 2024

cmsbuild commented Sep 2, 2024

cmsbuild commented Sep 2, 2024

fwyzard commented Sep 4, 2024

cmsbuild commented Sep 4, 2024

fwyzard commented Sep 4, 2024

cmsbuild commented Sep 4, 2024

cmsbuild commented Sep 4, 2024

cmsbuild commented Sep 4, 2024

makortel left a comment

makortel Sep 5, 2024

fwyzard commented Sep 7, 2024

cmsbuild commented Sep 7, 2024

mandrenguyen commented Sep 8, 2024

Add a zeroInitialise() method to portable objects and collections #45861

Add a zeroInitialise() method to portable objects and collections #45861

Conversation

fwyzard commented Sep 2, 2024 • edited Loading

PR description:

PR validation:

cmsbuild commented Sep 2, 2024 • edited Loading

fwyzard commented Sep 2, 2024

fwyzard commented Sep 2, 2024

fwyzard commented Sep 2, 2024

cmsbuild commented Sep 2, 2024

cmsbuild commented Sep 2, 2024

cmsbuild commented Sep 2, 2024

Comparison Summary

GPU Comparison Summary

fwyzard commented Sep 4, 2024

cmsbuild commented Sep 4, 2024

fwyzard commented Sep 4, 2024

cmsbuild commented Sep 4, 2024

cmsbuild commented Sep 4, 2024

cmsbuild commented Sep 4, 2024

Comparison Summary

GPU Comparison Summary

makortel left a comment

Choose a reason for hiding this comment

makortel Sep 5, 2024

Choose a reason for hiding this comment

fwyzard commented Sep 7, 2024

cmsbuild commented Sep 7, 2024

mandrenguyen commented Sep 8, 2024

Add a `zeroInitialise()` method to portable objects and collections #45861

Add a `zeroInitialise()` method to portable objects and collections #45861

fwyzard commented Sep 2, 2024 •

edited

Loading

cmsbuild commented Sep 2, 2024 •

edited

Loading