-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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.
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
Replace configuration-copying Alpaka ESProducers with MoveToDeviceCache #47047
Replace configuration-copying Alpaka ESProducers with MoveToDeviceCache #47047
Conversation
cms-bot internal usage |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-47047/43186
|
A new Pull Request was created by @makortel for master. It involves the following packages:
@Martin-Grunewald, @atpathak, @cmsbuild, @consuegs, @francescobrivio, @jfernan2, @mandrenguyen, @mmusich, @perrotta can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
be750b4
to
bac91f6
Compare
@cms-sw/heterogeneous-l2 |
enable gpu |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-47047/43187
|
@cmsbuild, please test |
Pull request #47047 was updated. @Martin-Grunewald, @atpathak, @consuegs, @francescobrivio, @jfernan2, @mandrenguyen, @mmusich, @perrotta can you please check and sign again. |
+1 Size: This PR adds an extra 140KB to repository Comparison SummarySummary:
GPU Comparison SummarySummary:
|
+1 |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-47047/43276
|
@cmsbuild, please test |
+heterogeneous |
That is correct. |
+1 Size: This PR adds an extra 28KB to repository Comparison SummarySummary:
GPU Comparison SummarySummary:
|
+1 |
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. @antoniovilela, @sextonkennedy, @rappoccio, @mandrenguyen (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
delattr(process, esProducer.label()) | ||
|
||
for prod in producers_by_type(process, "HBHERecHitProducerPortable@alpaka", "alpaka_serial_sync::HBHERecHitProducerPortable"): | ||
pulseOffsetLabel = prod.mahiPulseOffSets.getModuleLabel() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I realize only now, that this could have been written better. If one tries to run this customization function on a menu that has already the customization re-absorbed (e.g. /dev/CMSSW_15_0_0/GRun/V4
) one incurs in runtime errors such as: 'EDProducer' object has no attribute 'mahiPulseOffSets'
.
@Sam-Harper FYI
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If one tries to run this customization function on a menu that has already the customization re-absorbed
Is this situation something these customize functions should generally work? I'm sorry I didn't think about that (I though the customize function would be discarded after the menu itself has been updated).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this situation something these customize functions should generally work?
when we are transitioning between release cycles, yes, it happens commonly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR description:
Before #43969 a common pattern to copy data from configuration to all devices was to use the EventSetup system. While that works, it adds more pieces to be managed and disconnects the configuration parameters of a module (in addition to slight abuse of EventSetup system as "a job" is not a real IOV). This PR proposes to replace such Alpaka ESProducers with the
MoveToDeviceCache
introduced in #43969.Resolves cms-sw/framework-team#1129
PR validation:
Code compiles, matrix workflows 12834.{402,412,422} and 145.604 run with and without an NVIDIA GPU.