Read SoA scalars and Eigen columns from a ROOT streamer #39532

fwyzard · 2022-09-29T09:21:48Z

PR description:

Fix the ROOT streamers for the template-based SoA to read back the scalar variables, and to write and read back Eigen objects.

Update the TestAlpakaAnalyzer and CUDA TestPortableAnalyzer to check that scalars and matrices are read back correctly, and print a more verbose description of the SoA being read.

Update the test scripts so that the unit tests actually fail (exit with a non-zero value) if any of the cmsRun jobs fails.

PR validation:

The improved unit tests pass.

If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:

To be backported to CMSSW_12_5_X for the Alpaka migration.

fwyzard · 2022-09-29T09:22:03Z

type bugfix

fwyzard · 2022-09-29T09:22:12Z

enable gpu

fwyzard · 2022-09-29T09:22:16Z

please test

cmsbuild · 2022-09-29T09:28:17Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-39532/32305

This PR adds an extra 24KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File HeterogeneousCore/AlpakaCore/interface/chooseDevice.h modified in PR(s): Require the AlpakaService to be enabled before running on a Device #39498

cmsbuild · 2022-09-29T09:28:39Z

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

It involves the following packages:

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

@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 · 2022-09-29T13:39:01Z

-1

Failed Tests: UnitTests
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-474471/27840/summary.html
COMMIT: 848368c
CMSSW: CMSSW_12_6_X_2022-09-28-2300/el8_amd64_gcc10
Additional Tests: GPU
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/39532/27840/install.sh to create a dev area with all the needed externals and cmssw changes.

Unit Tests

I found errors in the following unit tests:

---> test testHeterogeneousCoreAlpakaTestWriteRead had ERRORS

Comparison Summary

@slava77 comparisons for the following workflows were not done due to missing matrix map:

/data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-474471/41834.0_TTbar_14TeV+2026D94+TTbar_14TeV_TuneCP5_GenSimHLBeamSpot14+DigiTrigger+RecoGlobal+HARVESTGlobal

Summary:

No significant changes to the logs found
Reco comparison results: 4 differences found in the comparisons
DQMHistoTests: Total files compared: 49
DQMHistoTests: Total histograms compared: 3433154
DQMHistoTests: Total failures: 3
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3433129
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
Checked 204 log files, 49 edm output root files, 49 DQM output files
TriggerResults: no differences found

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: 74
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 19802
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

makortel

The first commit (e5736f4) is already part of the master branch. Can it be dropped?

makortel · 2022-09-29T13:41:43Z

DataFormats/SoATemplate/interface/SoALayout.h

      ,                                                                                                                \
      /* Column */                                                                                                     \
      memcpy(BOOST_PP_CAT(NAME, _), onfile.BOOST_PP_CAT(NAME, _), sizeof(CPP_TYPE) * onfile.elements_);                \
      ,                                                                                                                \
      /* Eigen column */                                                                                               \
-      /* TODO: implement*/                                                                                             \
+      std::cerr << "Warning: reading an Eigen column from a ROOT file is not implemented" << std::endl;                \


Would an exception make sense? (or does it really matter for now?)

@ericcano should have an implementation of it later today, I'll add it here and update the PR description.

fwyzard · 2022-09-29T14:01:00Z

Unit Tests

I found errors in the following unit tests:

---> test testHeterogeneousCoreAlpakaTestWriteRead had ERRORS

The error is

cmsRun: /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_6_X_2022-09-28-2300/src/HeterogeneousCore/AlpakaTest/plugins/TestAlpakaAnalyzer.cc:86: virtual void TestAlpakaAnalyzer::analyze(const edm::Event&, const edm::EventSetup&): Assertion `view.r() == 1' failed.

and I've had it locally as well.

Looks like SCRAM is not checking out and recompiling all the dependencies (or we failed to declare some of them ?).

cmsbuild · 2022-09-29T21:52:36Z

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

fwyzard · 2022-09-30T04:05:23Z

please test with cms-sw/cmsdist#8107

cmsbuild · 2022-09-30T08:31:39Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-474471/27868/summary.html
COMMIT: 954edbc
CMSSW: CMSSW_12_6_X_2022-09-29-2300/el8_amd64_gcc10
Additional Tests: GPU
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/39532/27868/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

@slava77 comparisons for the following workflows were not done due to missing matrix map:

/pool/condor/dir_268586/jenkins/workspace/compare-root-files-short-matrix/data/PR-474471/41834.0_TTbar_14TeV+2026D94+TTbar_14TeV_TuneCP5_GenSimHLBeamSpot14+DigiTrigger+RecoGlobal+HARVESTGlobal

Summary:

No significant changes to the logs found
Reco comparison results: 19 differences found in the comparisons
DQMHistoTests: Total files compared: 49
DQMHistoTests: Total histograms compared: 3433154
DQMHistoTests: Total failures: 114
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3433018
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
Checked 204 log files, 49 edm output root files, 49 DQM output files
TriggerResults: no differences found

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

fwyzard · 2022-09-30T08:34:02Z

@makortel do you have any other concerns, or is it ok to merge this ?

makortel · 2022-09-30T14:10:52Z

@makortel do you have any other concerns, or is it ok to merge this ?

Looks good, although I would still prefer to drop the first commit (e5736f4) since it is already part of the master branch (even if this time git/GitHub is able to see it doesn't cause conflict).

fwyzard · 2022-09-30T14:18:31Z

That's the trick I found (by chance) so that all future PRs based on this same branch will merge without conflicts in both 12.5.x and 12.6.x.

fwyzard · 2022-09-30T14:18:40Z

+heterogeneous

cmsbuild · 2022-09-30T14:18:53Z

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

perrotta · 2022-10-01T05:42:41Z

@fwyzard you tested this lately together with cms-sw/cmsdist#8107, but neither here nor in the description of that cmsdist PR is ever written that the two havo to be merged together: please confirm that this PR can be merged independently from cms-sw/cmsdist#8107, or provide some different direction otherwise.

fwyzard · 2022-10-01T07:35:10Z

@perrotta, technically this and 8107 are independent.

8107 is a fix that can go in by itself.

This PR implements changes that are independent from 8107, and (also) makes the unit test report any failures instead of ignoring them.

Some random failures are already happening, but without this PR they are ignored: 8107 fixes them.

perrotta · 2022-10-01T07:39:06Z

+1

perrotta · 2022-10-02T06:19:49Z

@fwyzard perhaps you already noticed it, but after the merge of this PR the unit test in HeterogeneousCore/AlpakaTest/test/testHeterogeneousCoreAlpakaTestWriteRead.sh started failing, see e.g. https://cmssdt.cern.ch/SDT/cgi-bin/logreader/el8_amd64_gcc10/CMSSW_12_6_X_2022-10-01-1100/unitTestLogs/HeterogeneousCore/AlpakaTest#/

fwyzard · 2022-10-02T06:32:53Z

Yes - the fix is cms-sw/cmsdist#8107 .

fwyzard · 2022-10-02T06:50:45Z

What I meant earlier is that the underlying tests has been failing for a while (and cms-sw/cmsdist#8107 is the fix).

However, the failures were going unreported due to an annoying interplay between how scram run the tests and how bash inherits an environment from a parent process running within a conditional statement. This PR fixes the scripts to actually report the errors - so the pre-existing failures are now apparent.

Add missing includes

e5736f4

cmsbuild added this to the CMSSW_12_6_X milestone Sep 29, 2022

cmsbuild added bug-fix code-checks-pending heterogeneous-pending orp-pending pending-signatures tests-pending labels Sep 29, 2022

fwyzard mentioned this pull request Sep 29, 2022

Read SoA scalars and Eigen columns from a ROOT streamer [12.5.x] #39533

Merged

cmsbuild added tests-started and removed tests-pending labels Sep 29, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels Sep 29, 2022

cmsbuild added tests-rejected and removed tests-started labels Sep 29, 2022

makortel reviewed Sep 29, 2022

View reviewed changes

fwyzard force-pushed the alpaka_framework branch from 848368c to 6fb6335 Compare September 29, 2022 14:30

cmsbuild added code-checks-pending tests-pending tests-started and removed tests-rejected code-checks-approved tests-pending labels Sep 29, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels Sep 29, 2022

fwyzard mentioned this pull request Sep 29, 2022

Add helper functions for alpaka kernels #39546

Merged

fwyzard changed the title ~~Implement reading back SoA scalars from a ROOT streamer~~ Implement reading SoA scalars and Eigen columns from a ROOT streamer Sep 29, 2022

fwyzard changed the title ~~Implement reading SoA scalars and Eigen columns from a ROOT streamer~~ Read SoA scalars and Eigen columns from a ROOT streamer Sep 29, 2022

cmsbuild added the requires-external label Sep 30, 2022

cmsbuild added tests-approved and removed tests-started labels Sep 30, 2022

fwyzard mentioned this pull request Sep 30, 2022

Set ALPAKA_DEFAULT_HOST_MEMORY_ALIGNMENT=128 cms-sw/cmsdist#8107

Merged

cmsbuild added fully-signed heterogeneous-approved and removed pending-signatures heterogeneous-pending labels Sep 30, 2022

cmsbuild added orp-approved and removed orp-pending labels Oct 1, 2022

cmsbuild merged commit a89eb4c into cms-sw:master Oct 1, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Read SoA scalars and Eigen columns from a ROOT streamer #39532

Read SoA scalars and Eigen columns from a ROOT streamer #39532

fwyzard commented Sep 29, 2022 •

edited

fwyzard commented Sep 29, 2022

fwyzard commented Sep 29, 2022

fwyzard commented Sep 29, 2022

cmsbuild commented Sep 29, 2022

cmsbuild commented Sep 29, 2022

cmsbuild commented Sep 29, 2022

makortel left a comment

makortel Sep 29, 2022

fwyzard Sep 29, 2022

fwyzard commented Sep 29, 2022

Unit Tests

cmsbuild commented Sep 29, 2022

fwyzard commented Sep 30, 2022 •

edited

cmsbuild commented Sep 30, 2022

fwyzard commented Sep 30, 2022

makortel commented Sep 30, 2022

fwyzard commented Sep 30, 2022

fwyzard commented Sep 30, 2022

cmsbuild commented Sep 30, 2022

perrotta commented Oct 1, 2022

fwyzard commented Oct 1, 2022

perrotta commented Oct 1, 2022

perrotta commented Oct 2, 2022

fwyzard commented Oct 2, 2022

fwyzard commented Oct 2, 2022

Read SoA scalars and Eigen columns from a ROOT streamer #39532

Read SoA scalars and Eigen columns from a ROOT streamer #39532

Conversation

fwyzard commented Sep 29, 2022 • edited

PR description:

PR validation:

If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:

fwyzard commented Sep 29, 2022

fwyzard commented Sep 29, 2022

fwyzard commented Sep 29, 2022

cmsbuild commented Sep 29, 2022

cmsbuild commented Sep 29, 2022

cmsbuild commented Sep 29, 2022

Unit Tests

Comparison Summary

GPU Comparison Summary

makortel left a comment

Choose a reason for hiding this comment

makortel Sep 29, 2022

Choose a reason for hiding this comment

fwyzard Sep 29, 2022

Choose a reason for hiding this comment

fwyzard commented Sep 29, 2022

Unit Tests

cmsbuild commented Sep 29, 2022

fwyzard commented Sep 30, 2022 • edited

cmsbuild commented Sep 30, 2022

Comparison Summary

GPU Comparison Summary

fwyzard commented Sep 30, 2022

makortel commented Sep 30, 2022

fwyzard commented Sep 30, 2022

fwyzard commented Sep 30, 2022

cmsbuild commented Sep 30, 2022

perrotta commented Oct 1, 2022

fwyzard commented Oct 1, 2022

perrotta commented Oct 1, 2022

perrotta commented Oct 2, 2022

fwyzard commented Oct 2, 2022

fwyzard commented Oct 2, 2022

fwyzard commented Sep 29, 2022 •

edited

fwyzard commented Sep 30, 2022 •

edited