Revived being able to run modules inside a mixing module #40133
Conversation
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40133/33124
|
|
A new Pull Request was created by @Dr15Jones (Chris Jones) for master. It involves the following packages:
@smuzaffar, @civanch, @Dr15Jones, @makortel, @mdhildreth, @cmsbuild can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
| @@ -383,6 +383,8 @@ namespace edm { | |||
| ModuleCallingContext moduleCallingContext(&moduleDescription()); | |||
| ModuleContextSentry moduleContextSentry(&moduleCallingContext, parentContext); | |||
|
|
|||
| setupPileUpEvent(setup); | |||
There was a problem hiding this comment.
Just to note that as far as I can tell, the necessity of this call has gone unnoticed because before we have not used the ability to run modules (that would consume EventSetup products) as part of the MixingModule. Would you agree?
There was a problem hiding this comment.
I think you are probably correct.
This does not modify the old token, just allows the new token to have a different service for the same activity as the old token.
Uses the mixing modules assigned random engine as the basis for the engines given to the modules internal to the mixing module.
Share the implementation across RandomNumberGenerators
Dr15Jones
force-pushed
the
reviveModulesInsideMixing
branch
from
6971e19 to
cbdb75c
Compare
|
please test |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40133/33274
|
|
Pull request #40133 was updated. @makortel, @smuzaffar, @civanch, @Dr15Jones, @mdhildreth can you please check and sign again. |
|
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-9199ce/29435/summary.html Comparison SummarySummary:
|
|
The differences occur only in 11634.7 (which is the "all tracking iterations using mkFit" variant). Seems that similar spurious differences are visible in some other recent master PRs as well #39803 (comment). |
|
+core |
|
+1 |
cmsbuild
added
fully-signed
simulation-approved
and removed
simulation-pending
pending-signatures
labels
|
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) |
|
+1 |
PR description:
PR validation:
Tested generating pileup 'on the fly' by embedding the generator and OscarMTProducer within the mixing module. [This requires a further change to OscarMTProducer].