-
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
Use of ESGetToken in Oscar producer #34338
Conversation
assign core |
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34338/23706 ERROR: Build errors found during clang-tidy run.
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34338/23707
|
New categories assigned: core @Dr15Jones,@smuzaffar,@makortel you have been requested to review this Pull request/Issue and eventually sign? Thanks |
A new Pull Request was created by @civanch (Vladimir Ivantchenko) for master. It involves the following packages: SimG4Core/Application @smuzaffar, @civanch, @Dr15Jones, @makortel, @mdhildreth, @cmsbuild can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
please test |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-5c6882/16476/summary.html Comparison SummarySummary:
|
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-5c6882/16714/summary.html Comparison SummarySummary:
|
+1 |
Issue #34448 is created. This PR can be merged. |
@makortel , would you agree to sign it? The issue with Run-1 WFs may be resolved in other PR. |
+1 The differences in Run 1 workflows appear to be caused by not-yet-known reason (appears to be related to the execution order magnetic field and geometry ESProducers) that is not caused by this PR, even if this PR acts as an example of bringing the issue up. Merging this PR should not make things worse as they are already. |
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. @silviodonato, @dpiparo, @qliphy, @perrotta (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
I think this PR has broken some addOn tests: |
Indeed, the es.getData() call for the MagneticField should have gone to RunManagerMTWorker::beginRun() (sorry for missing it) |
The problem occurs randomly, depending if (for whatever reason) the
The problem is that the |
Here is a fix #34479 |
PR description:
Migration to ESGetToken of the OscarMTProducer required some reorganization of Geant4 initialization. If this PR is correct then no change should be at SIM and other steps.
PR validation:
private
if this PR is a backport please specify the original PR and why you need to backport that PR: no