Random generator service for FastSim #21714
Conversation
|
The code-checks are being triggered in jenkins. |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-21714/2590 |
|
A new Pull Request was created by @civanch (Vladimir Ivantchenko) for master. It involves the following packages: FastSimulation/Utilities @cmsbuild, @ssekmen, @civanch, @mdhildreth, @lveldere can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
please test |
|
The tests are being triggered in jenkins. |
|
Hi, @Dr15Jones, you may be want to comment on this PR? |
|
this doesn't make sense- we don't need to go through G4 to get random numbers in CMS.. |
|
@davidlange6 It appears to me that with this pull request, the random numbers come from CMS via the RandomNumberGenerator service. The G4 part is just passing the CMS random engine to G4 to use since, under the hood, fastsim is calling G4 components which need a random number. |
|
@davidlange6 , internally, HF Shower library use G4 methods and FastSim FTF nuclear generator in tracker use Geant4 model. Potentially, there may be other consumers of Geant4. |
|
Indeed, this is the G4 usage we have now (although the FTP nuclear interactions are currently not actively used). |
|
Indeed - I missed that point - so then we should ensure in either a more central, or more targeted location that g4 is initialized in fastsim jobs- buried in a class within FastSimulation/Utilities hardly seems like the place for that (as this class is _not_ used by G4 and does not currently use G4) That way we control dependencies
… On Dec 14, 2017, at 4:03 PM, Chris Jones ***@***.***> wrote:
@davidlange6 It appears to me that with this pull request, the random numbers come from CMS via the RandomNumberGenerator service. The G4 part is just passing the CMS random engine to G4 to use since, under the hood, fastsim is calling G4 components which need a random number.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
What about FamosProducer? The same line of code may be moved there. This may work but I am not 100% sure. The choice of Utility was done for reason, that other Producers may use the same FastSimUtility , so it is more safe (number of plugins in FastSim sequence is big enough and may be some also use random numbers). A solution would be to include the same line in all producers we have for today (which use random numbers). |
|
how about in a trivial new plugin living in SimG4Core?
… On Dec 14, 2017, at 4:24 PM, Vladimir Ivantchenko ***@***.***> wrote:
What about FamosProducer? The same line of code may be moved there.
This may work but I am not 100% sure. The choice of Utility was done for reason, that other Producers may use the same FastSimUtility , so it is more safe (number of plugins in FastSim sequence is big enough and may be some also use random numbers). A solution would be to include the same line in all producers we have for today (which use random numbers).
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
I don't think FamosProducer is more central compared to FastSimulation/Utilities, which is already used by many fastsim modules. |
|
@Dr15Jones , a basic question: are we sure that all FastSim plugins are in the same streamID? If not, how we should handle this? This is a general question also for FullSim where we in one plugin smear interaction point and in Oscar generate hits. |
|
Plugins called from the same module instance for the same event will be on the same thread and will therefore have the same stream ID. As long as G4 is using a thread_local to hold the random engine, everything will be fine. |
|
In that case, may be FamosProducer is a correct solution? Making extra empty producer in SimG4Core not a problem, but at the same time, may be easily forgotten by FastSim developers inside their unit tests and other evolutions. |
cmsbuild
added
fastsim-approved
fully-signed
and removed
fastsim-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. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2) |
|
Comparison is ready @slava77 comparisons for the following workflows were not done due to missing matrix map:
Comparison Summary:
|
|
@civanch There was also a residual comment about the MaterialEffects package, where the geant4 dependency is still there. Could you please comment? |
|
please test workflow 135.1 |
|
The tests are being triggered in jenkins. |
cmsbuild
added
comparison-pending
tests-pending
tests-started
and removed
comparison-available
tests-approved
tests-pending
labels
|
Comparison job queued. |
|
@fabiocos , since long we have an alternative FastSim hadronic model for nuclear interaction in tracker, which based on FTF Geant4 model. For that we introduced FastSim dependence on Geant4. From my point of view, this model is technically much better than the default FastSim model. I have in mind adding more Geant4 based models for FastSim. |
|
Comparison is ready @slava77 comparisons for the following workflows were not done due to missing matrix map:
Comparison Summary:
|
|
+1 |
Recently it was discovered, that some of components of FastSim calls CLHEP default directly instead of access to the generator via CMSSW Service. Potentially, this may provide data race and/or non-reproducibility of results in MT mode, because the default CLHEP engine is static. Due to this problem we stack with migration to the recent CLHEP 4.2.0.0
Currently, known places in FastSim where CLHEP or Geant4 calls to engine are used : HF shower library, FTF nuclear interaction, CTTPS forward simulation.
FastSim has utility which access CMSSW Service correctly. In this clean-up a thread save method to initialise random engine is added using FullSim approach, so in all sampling, even in Geant4 components, the same random generator will be used. Current default for FastSim is TRandom3 but any other may be configured. Additionally, the inline methods of this utility are modified to reduce number of "if" and to use common interface to Gauss and Poisson sampling.
The payment for this fix - direct dependence of FastSim/Utilities on Geant4. Note, that FastSim already depends on Geant4 directly and indirectly, I do not think that this dependence make a problem. Also random number sequence will be different, so many differences should be seen in FastSim WFs.