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.

Sign up for GitHub

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

Jump to bottom

Disable ROOT automatic class parsing in ProductRegistryHelper::addToRegistry #47397

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Disable ROOT automatic class parsing in ProductRegistryHelper::addToRegistry #47397

wants to merge 2 commits into from
+102 −1

Conversation

fwyzard
Copy link
Contributor

@fwyzard fwyzard commented Feb 19, 2025

PR description:

Disable ROOT automatic class parsing while registering products in ProductRegistryHelper::addToRegistry().

This done to avoid trying to build missing dictionaries for edm::Wrapper<T> when the dictionary for T itself is present and the relevant classes.h file includes a CUDA-related header, as that causes ROOT to fail before we can issue the proper error message about missing dictionaries.

Add a new test to check that the framework handles such corner case correctly.

PR validation:

The new unit test passes, while it would fail without these changes.

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 15.0.x

@fwyzard
Disable ROOT automatic class parsing while registering products
21bea8b 
This done to avoid trying to build missing dictionaries for edm::Wrapper<T>
when the dictionary for T itself is present and the relevant classes.h file
includes a CUDA-related header, as that causes ROOT to fail before we can
issue the proper error message about missing dictionaries.
@fwyzard
Add a test for missing dictionaries with CUDA headers
072f250 
Add a test to check that the framework handles correctly the case of
edm::Wrapper<T> where:
  - T has a dictionary
  - edm::Wrapper<T> does not have a dictionary
  - the corresponding classes.h file includes CUDA headers
@cmsbuild cmsbuild added this to the CMSSW_15_1_X milestone Feb 19, 2025
@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 19, 2025 •
edited
Loading

cms-bot internal usage

@fwyzard
Copy link
Contributor Author

fwyzard commented Feb 19, 2025

enable gpu

@fwyzard
Copy link
Contributor Author

fwyzard commented Feb 19, 2025

please test

@fwyzard fwyzard mentioned this pull request Feb 19, 2025
Open
@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-47397/43780

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

  • FWCore/Framework (core)
  • HeterogeneousCore/CUDATest (heterogeneous)

@Dr15Jones, @fwyzard, @makortel, @smuzaffar can you please review it and eventually sign? Thanks.
@makortel, @missirol, @rovere, @wddgit 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

@fwyzard
Copy link
Contributor Author

fwyzard commented Feb 19, 2025

Assertion `used_concurrently_but_shouldnt' failed.

Ah :-/

@cmsbuild
Copy link
Contributor

-1

Failed Tests: UnitTests RelVals RelVals-GPU RelVals-INPUT AddOn
Size: This PR adds an extra 32KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-7b07a4/44500/summary.html
COMMIT: 072f250
CMSSW: CMSSW_15_1_X_2025-02-18-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/47397/44500/install.sh to create a dev area with all the needed externals and cmssw changes.

Unit Tests

I found 3 errors in the following unit tests:

---> test TestFWCoreFramework had ERRORS
---> test test_MC_22_crosscheck had ERRORS
---> test test_MC_23_crosscheck had ERRORS

RelVals

  • 135.4A fatal system signal has occurred: abort signal
  • 1306.0A fatal system signal has occurred: abort signal
  • 7.3A fatal system signal has occurred: abort signal
Expand to see more relval errors ...

RelVals-GPU

  • 12834.41212834.412_TTbar_14TeV+2024_Patatrack_ECALOnlyAlpaka/step3_TTbar_14TeV+2024_Patatrack_ECALOnlyAlpaka.log

RelVals-INPUT

  • 138.5138.5_ExpressCollisions2021/step2_ExpressCollisions2021.log
  • 138.4138.4_PromptCollisions2021/step2_PromptCollisions2021.log
  • 138.2138.2_ExpressCosmics2021/step2_ExpressCosmics2021.log
Expand to see more relval errors ...

AddOn Tests

A fatal system signal has occurred: abort signal

A fatal system signal has occurred: abort signal

A fatal system signal has occurred: abort signal
Expand to see more addon errors ...

@fwyzard
Copy link
Contributor Author

fwyzard commented Feb 19, 2025

After more digging I think I found what is the underlying issue: #47400 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
changes-dataformats code-checks-approved core-pending heterogeneous-pending orp-pending pending-signatures tests-rejected
Projects
None yet
Milestone
CMSSW_15_1_X
Development

Successfully merging this pull request may close these issues.
2 participants
@fwyzard @cmsbuild