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
Switch to CLHEP 2.4.0.0 #3616
Switch to CLHEP 2.4.0.0 #3616
Conversation
A new Pull Request was created by @civanch (Vladimir Ivantchenko) for branch IB/CMSSW_10_0_X/gcc630. @cmsbuild, @smuzaffar, @gudrutis, @mrodozov can you please review it and eventually sign? Thanks. |
please test |
The tests are being triggered in jenkins. |
Comparison job queued. |
Comparison is ready @slava77 comparisons for the following workflows were not done due to missing matrix map:
Comparison Summary:
|
It is strange, that the difference is two WFs only: 5.1 and 135.4 |
@davidlange6 , any objection getting this one in? |
hi @civanch - but we don't expect different random number sequences unless there is a change. Lets check that its reproducible - otherwise is there something in the release notes that suggests a bug fix that might affect fast sim? |
please test |
The tests are being triggered in jenkins. |
@smuzaffar , @davidlange6 the difference in two FWs means that there are places were where CMS Service for random numbers do not used, instead CLHEP random engine is called directly. CLHEP random default was changed but this does not affect CMS RandomService which is explicit per producer. The suspect is also generators, in these WFs they likely from run-1. |
Comparison job queued. |
Comparison is ready @slava77 comparisons for the following workflows were not done due to missing matrix map:
Comparison Summary:
|
hi @civanch - it can not be gen - as the generators are the same here and in full sim. (both run1/run2) i notice that the HepRandom defaults have changed to mixmax - do we just discover that someone is using the default (that probably shouldn't be) |
Hi, @davidlange6 , I agree with you, that it is the FastSim feature - we discover that somewhere in FastSim the default CLHEP generator is used. Also FastSim is tightly coupled with TRandom3 - I see inside FastSim even checks this... We need discuss this with FastSim experts but this should not stop from merge of this CLHEP PR - when FatsSim will be fully understood likely a CMSSW PR will be needed but should we wait for that? |
@ssekmen - we see a problem in FastSim WFs. |
the question is why are we now ready to use minmax in production? I don't think so. so should we not just control its usage?
i don't actually see something in fast sim creating a HepRandom object - do you know where to look?
… On Dec 12, 2017, at 2:07 PM, Vladimir Ivantchenko ***@***.***> wrote:
Hi, @davidlange6 , I agree with you, that it is the FastSim feature - we discover that somewhere in FastSim the default CLHEP generator is used. Also FastSim is tightly coupled with TRandom3 - I see inside FastSim even checks this... We need discuss this with FastSim experts but this should not stop from merge of this CLHEP PR - when FatsSim will be fully understood likely a CMSSW PR will be needed but should we wait for that?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
The sequence is long. I grep all places in FastSim without full understanding of the situation. |
what should i search for? I don't see much from looking at HepRandom.
… On Dec 12, 2017, at 2:35 PM, Vladimir Ivantchenko ***@***.***> wrote:
The sequence is long. I grep all places in FastSim without full understanding of the situation.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
there may search for G4UniformRand() it is only one idea. |
G4UniformRand seems to be used in lots of places (but almost no place in fast sim) - I conclude we must have initialized it correctly.
… On Dec 12, 2017, at 2:44 PM, Vladimir Ivantchenko ***@***.***> wrote:
there may search for G4UniformRand() it is only one idea.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
@civanch , checking. This could be happening in the calorimetry code. |
G4UniformRand seems to be used in fastsim HF showers: FastSimulation/ShowerDevelopment/src/FastHFShowerLibrary.cc . |
Also FastSimulation/CTPPSRecHitProducer in several places use directly CLHEP for Gauss. |
I hate guessing games - can you find the actual constructor that uses the "default"?
Eg, the syntax in FastSim.CTPPS is no different from that in the rest of CMS (and its TRandom3 it would seem, strangely)
https://cmssdt.cern.ch/lxr/source/FastSimulation/CTPPSRecHitProducer/plugins/CTPPSRecHitProducer.cc#0151
… On Dec 12, 2017, at 4:48 PM, Vladimir Ivantchenko ***@***.***> wrote:
Also FastSimulation/CTPPSRecHitProducer in several places use directly CLHEP for Gauss.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
Main modification - improved MixMax random number generator and make it default. The test should show no difference with the baseline, because in CMSSW explicit random number generators are defined per producer, so the old ones will be used.