changes to random number seeding of workers in MT mode #160
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Hi @goodenou,
which require these tests: build. |
|
@FNALbuild run build test |
|
⌛ The following tests have been triggered for ref 421b9c2: build |
kutschke
requested changes
Mar 22, 2020
Mu2eG4/fcl/g4test_pot.fcl
Outdated
| @@ -67,5 +67,8 @@ services.SeedService.maxUniqueEngines : 20 | |||
| # physics.producers.g4run.SDConfig.enableAllSDs : true | |||
| physics.producers.g4run.SDConfig.enableSD : [tracker, calorimeter, calorimeterRO, CRV, virtualdetector, stoppingtarget ] | |||
| physics.producers.g4run.physics.BirksConsts : { G4_POLYSTYRENE : 0.07943 } // {string : mm/MeV } | |||
|
|
|||
| services.GeometryService.simulatedDetector.tool_type : "Mu2e" | |||
|
|
|||
| physics.producers.g4run.physics.physicsListName : "QGSP_BERT" | |||
|
|
|||
There was a problem hiding this comment.
Lisa. Please check with Ray. Do we need to keep both
./Validation/fcl/potSimMT.fcl
./Validation/fcl/potSim.fcl
and
./Mu2eG4/fcl/g4test_potMT.fcl
./Mu2eG4/fcl/g4test_pot.fcl
If we can I would like to delete the latter two.
There was a problem hiding this comment.
I made /Mu2eG4/fcl/g4test_pot*fcl for my own testing. I can delete these if you want.
There was a problem hiding this comment.
If you no longer need them, delete them.
If you still need them, can you implement them by starting from #include the file from Validation?
Let me know what you plan to do.
There was a problem hiding this comment.
I have deleted them.
| @@ -185,6 +185,7 @@ Mu2eG4MT: { | |||
| g4run : { | |||
| @table::mu2eg4runDefaultSingleStage | |||
| module_type : "Mu2eG4MT" | |||
| initialSeed : 8 | |||
| } | |||
| } | |||
| } | |||
There was a problem hiding this comment.
We need to discuss this. I would like to drive 100% of the seeding from the SeedService unless there is a compelling reason not to. Does the master thread use the global instance of the CLHEP HepRandomEngine? If so then seeding it is already taken care of at the call to createEngine in the module c'tor. At least it used to be in the single threaded world.
There was a problem hiding this comment.
We can use the Seed Service, but we cannot use the Random Number Engine Service in Mu2eG4MT_module.cc because it is a Shared Module. I have altered the code to use the Seed Service. The MTRunManager now gets a seed from the Seed Service. It uses this seed in its MixMax random number generator to generate seeds for the worker random number generators.
There was a problem hiding this comment.
I don't see the change. Did you push it? The last commit I see is 3 days ago.
There was a problem hiding this comment.
I have just pushed the change.
|
Hi Rob,
I will answer the other questions later, but I want to let you know that we cannot use the Seed Service a shared module, which is what the MT module is. We must seed the engine ourselves. In light of that, do you have different questions for me?
Lisa
… On Mar 22, 2020, at 5:32 PM, Rob Kutschke ***@***.***> wrote:
@kutschke requested changes on this pull request.
In Mu2eG4/fcl/g4test_pot.fcl <#160 (comment)>:
> @@ -67,5 +67,8 @@ services.SeedService.maxUniqueEngines : 20
# physics.producers.g4run.SDConfig.enableAllSDs : true
physics.producers.g4run.SDConfig.enableSD : [tracker, calorimeter, calorimeterRO, CRV, virtualdetector, stoppingtarget ]
physics.producers.g4run.physics.BirksConsts : { G4_POLYSTYRENE : 0.07943 } // {string : mm/MeV }
+
+services.GeometryService.simulatedDetector.tool_type : "Mu2e"
+
physics.producers.g4run.physics.physicsListName : "QGSP_BERT"
Lisa. Please check with Ray. Do we need to keep both
./Validation/fcl/potSimMT.fcl
./Validation/fcl/potSim.fcl
and
./Mu2eG4/fcl/g4test_potMT.fcl
./Mu2eG4/fcl/g4test_pot.fcl
If we can I would like to delete the latter two.
In Mu2eG4/fcl/prolog.fcl <#160 (comment)>:
> @@ -185,6 +185,7 @@ Mu2eG4MT: {
g4run : {
@table::mu2eg4runDefaultSingleStage
module_type : "Mu2eG4MT"
+ initialSeed : 8
}
}
}
We need to discuss this. I would like to drive 100% of the seeding from the SeedService unless there is a compelling reason not to. Does the master thread use the global instance of the CLHEP HepRandomEngine? If so then seeding it is already taken care of at the call to createEngine in the module c'tor. At least it used to be in the single threaded world.
In Mu2eG4/inc/Mu2eG4Config.hh <#160 (comment)>:
> @@ -200,9 +200,9 @@ namespace mu2e {
fhicl::Atom<std::string> generatorModuleLabel {Name("generatorModuleLabel"), ""};
fhicl::Atom<bool> G4InteralFiltering {Name("G4InteralFiltering"), false};
- fhicl::Atom<int> maxEventsToSeed {Name("maxEventsToSeed"),
- Comment("Only used by Mu2eG4MT module."),
- 10000 };
+ fhicl::Atom<long> initialSeed {Name("initialSeed"),
+ Comment("Only used by Mu2eG4MTRunManager when running in MT mode"),
+ 8 };
See previous comment about using SeedService.
In Mu2eG4/inc/Mu2eG4MTRunManager.hh <#160 (comment)>:
> +
+ G4bool SetUpAnEvent(G4Event* evt,
+ long& s1, long& s2, long& s3,
+ G4bool reseedRequired);
For my own edification. Is this a standard G4 function or one that you added?
In Mu2eG4/inc/Mu2eG4WorkerRunManager.hh <#160 (comment)>:
> @@ -51,6 +51,7 @@ namespace mu2e {
void initializeUserActions(const G4ThreeVector& origin_in_world);
void initializeRun(art::Event* art_event);
void processEvent(art::Event*);
+ G4Event* generateEvt(G4int i_event);
Is this a standard G4 function or a Mu2e addition? The name seems weird to me - it sounds like it is a function that should run an event generator. So if we control the name, let's talk about a better name.
In Mu2eG4/src/Mu2eG4MTRunManager.cc <#160 (comment)>:
> @@ -102,10 +106,9 @@ namespace mu2e {
if(fSDM)
{ currentRun->SetHCtable(fSDM->GetHCtable()); }
std::ostringstream oss;
- //WHAT LIBRARY TO USE?
- //G4Random::saveFullState(oss);
- //randomNumberStatusForThisRun = oss.str();
- //currentRun->SetRandomNumberStatus(randomNumberStatusForThisRun);
+ G4Random::saveFullState(oss);
+ randomNumberStatusForThisRun = oss.str();
+ currentRun->SetRandomNumberStatus(randomNumberStatusForThisRun);
Please fill me in on the plan for storing random number engine state? The SeedService already has functionality to store the state at the start of each event and I prefer all engines to use it rather than rolling their own.
This brings up another wrinkle - we need to talk more about the management of the engines that are used in the worker threads. Ideally I would like to manage them with createEngine and the SeedService . For historical technical reasons, create engine can only be called from the c'tor of a module. So we would need to make them all in the c'tor, save pointers and pass out the pointers as needed. I don't know if this causes a big fight with G4 or not.
In Mu2eG4/src/Mu2eG4MTRunManager.cc <#160 (comment)>:
> -
- G4ExceptionDescription msgd;
- msgd << "Event modulo is reduced to " << eventModulo
- << " to distribute events to all threads.";
- G4Exception("G4MTRunManager::InitializeEventLoop()", "Run10035", JustWarning, msgd);
- }
- }
- else {
- eventModulo = int(std::sqrt(double(numberOfEventToBeProcessed/numworkers)));
- if(eventModulo<1) eventModulo = 1;
- }
-
- if ( InitializeSeeds(numevents) == false && numevents>0 ) {
+
+ //if we are using G4's seed filling scheme
+ if ( InitializeSeeds(seedbunchsize) == false && seedbunchsize>0 ) {
G4RNGHelper* helper = G4RNGHelper::GetInstance();
Is this thread safe?
In Mu2eG4/inc/Mu2eG4MTRunManager.hh <#160 (comment)>:
> @@ -61,9 +65,7 @@ namespace mu2e {
G4VUserPhysicsList* physicsList_;
int rmvlevel_;
-
- G4int maxNumEventstoSeed_;
-
+ long initialSeed_;
See previous comments about using the SeedService.
In Validation/fcl/potSimMT.fcl <#160 (comment)>:
> @@ -7,6 +7,7 @@
# set in the non MT file included above (which includes prolog files)
physics.producers.g4run.module_type : "Mu2eG4MT"
+physics.producers.g4run.initialSeed : 10
Another thing on the list if we can use the SeedService to seed the master engine.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#160 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AE44QBT5XHKFFVMX43JBX23RI2GYPANCNFSM4LQXDTZA>.
|
|
Hi Lisa,
We only access the SeedService in the c'tor when everything is still single threaded. Is there a type of service that can be called from a shared module? If so we *may* be able to just declare it to be that kind of service instead of a legacy service ( is legacy the proper name?). Or we could actually make it threadsafe; I *think* that all we need to do is to put a mutex at the entry and exit of:
seed_t getSeed( std::string const& instanceName );
Since this is a startup only thing, we can afford mutexes.
I also need to go back and look at your answers to a previous question and follow up on that.
Rob
… On Mar 22, 2020, at 6:21 PM, Lisa Goodenough ***@***.***> wrote:
Hi Rob,
I will answer the other questions later, but I want to let you know that we cannot use the Seed Service a shared module, which is what the MT module is. We must seed the engine ourselves. In light of that, do you have different questions for me?
Lisa
> On Mar 22, 2020, at 5:32 PM, Rob Kutschke ***@***.***> wrote:
>
>
> @kutschke requested changes on this pull request.
>
> In Mu2eG4/fcl/g4test_pot.fcl <#160 (comment)>:
>
> > @@ -67,5 +67,8 @@ services.SeedService.maxUniqueEngines : 20
> # physics.producers.g4run.SDConfig.enableAllSDs : true
> physics.producers.g4run.SDConfig.enableSD : [tracker, calorimeter, calorimeterRO, CRV, virtualdetector, stoppingtarget ]
> physics.producers.g4run.physics.BirksConsts : { G4_POLYSTYRENE : 0.07943 } // {string : mm/MeV }
> +
> +services.GeometryService.simulatedDetector.tool_type : "Mu2e"
> +
> physics.producers.g4run.physics.physicsListName : "QGSP_BERT"
>
> Lisa. Please check with Ray. Do we need to keep both
>
> ./Validation/fcl/potSimMT.fcl
> ./Validation/fcl/potSim.fcl
>
> and
> ./Mu2eG4/fcl/g4test_potMT.fcl
> ./Mu2eG4/fcl/g4test_pot.fcl
>
> If we can I would like to delete the latter two.
>
> In Mu2eG4/fcl/prolog.fcl <#160 (comment)>:
>
> > @@ -185,6 +185,7 @@ Mu2eG4MT: {
> g4run : {
> @table::mu2eg4runDefaultSingleStage
> module_type : "Mu2eG4MT"
> + initialSeed : 8
> }
> }
> }
> We need to discuss this. I would like to drive 100% of the seeding from the SeedService unless there is a compelling reason not to. Does the master thread use the global instance of the CLHEP HepRandomEngine? If so then seeding it is already taken care of at the call to createEngine in the module c'tor. At least it used to be in the single threaded world.
>
> In Mu2eG4/inc/Mu2eG4Config.hh <#160 (comment)>:
>
> > @@ -200,9 +200,9 @@ namespace mu2e {
> fhicl::Atom<std::string> generatorModuleLabel {Name("generatorModuleLabel"), ""};
>
> fhicl::Atom<bool> G4InteralFiltering {Name("G4InteralFiltering"), false};
> - fhicl::Atom<int> maxEventsToSeed {Name("maxEventsToSeed"),
> - Comment("Only used by Mu2eG4MT module."),
> - 10000 };
> + fhicl::Atom<long> initialSeed {Name("initialSeed"),
> + Comment("Only used by Mu2eG4MTRunManager when running in MT mode"),
> + 8 };
> See previous comment about using SeedService.
>
> In Mu2eG4/inc/Mu2eG4MTRunManager.hh <#160 (comment)>:
>
> > +
> + G4bool SetUpAnEvent(G4Event* evt,
> + long& s1, long& s2, long& s3,
> + G4bool reseedRequired);
> For my own edification. Is this a standard G4 function or one that you added?
>
> In Mu2eG4/inc/Mu2eG4WorkerRunManager.hh <#160 (comment)>:
>
> > @@ -51,6 +51,7 @@ namespace mu2e {
> void initializeUserActions(const G4ThreeVector& origin_in_world);
> void initializeRun(art::Event* art_event);
> void processEvent(art::Event*);
> + G4Event* generateEvt(G4int i_event);
> Is this a standard G4 function or a Mu2e addition? The name seems weird to me - it sounds like it is a function that should run an event generator. So if we control the name, let's talk about a better name.
>
> In Mu2eG4/src/Mu2eG4MTRunManager.cc <#160 (comment)>:
>
> > @@ -102,10 +106,9 @@ namespace mu2e {
> if(fSDM)
> { currentRun->SetHCtable(fSDM->GetHCtable()); }
> std::ostringstream oss;
> - //WHAT LIBRARY TO USE?
> - //G4Random::saveFullState(oss);
> - //randomNumberStatusForThisRun = oss.str();
> - //currentRun->SetRandomNumberStatus(randomNumberStatusForThisRun);
> + G4Random::saveFullState(oss);
> + randomNumberStatusForThisRun = oss.str();
> + currentRun->SetRandomNumberStatus(randomNumberStatusForThisRun);
>
> Please fill me in on the plan for storing random number engine state? The SeedService already has functionality to store the state at the start of each event and I prefer all engines to use it rather than rolling their own.
>
> This brings up another wrinkle - we need to talk more about the management of the engines that are used in the worker threads. Ideally I would like to manage them with createEngine and the SeedService . For historical technical reasons, create engine can only be called from the c'tor of a module. So we would need to make them all in the c'tor, save pointers and pass out the pointers as needed. I don't know if this causes a big fight with G4 or not.
>
> In Mu2eG4/src/Mu2eG4MTRunManager.cc <#160 (comment)>:
>
> > -
> - G4ExceptionDescription msgd;
> - msgd << "Event modulo is reduced to " << eventModulo
> - << " to distribute events to all threads.";
> - G4Exception("G4MTRunManager::InitializeEventLoop()", "Run10035", JustWarning, msgd);
> - }
> - }
> - else {
> - eventModulo = int(std::sqrt(double(numberOfEventToBeProcessed/numworkers)));
> - if(eventModulo<1) eventModulo = 1;
> - }
> -
> - if ( InitializeSeeds(numevents) == false && numevents>0 ) {
> +
> + //if we are using G4's seed filling scheme
> + if ( InitializeSeeds(seedbunchsize) == false && seedbunchsize>0 ) {
>
> G4RNGHelper* helper = G4RNGHelper::GetInstance();
> Is this thread safe?
>
> In Mu2eG4/inc/Mu2eG4MTRunManager.hh <#160 (comment)>:
>
> > @@ -61,9 +65,7 @@ namespace mu2e {
> G4VUserPhysicsList* physicsList_;
>
> int rmvlevel_;
> -
> - G4int maxNumEventstoSeed_;
> -
> + long initialSeed_;
> See previous comments about using the SeedService.
>
> In Validation/fcl/potSimMT.fcl <#160 (comment)>:
>
> > @@ -7,6 +7,7 @@
> # set in the non MT file included above (which includes prolog files)
>
> physics.producers.g4run.module_type : "Mu2eG4MT"
> +physics.producers.g4run.initialSeed : 10
> Another thing on the list if we can use the SeedService to seed the master engine.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub <#160 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AE44QBT5XHKFFVMX43JBX23RI2GYPANCNFSM4LQXDTZA>.
>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
|
Hi Lisa,
If you no longer need them, delete them.
If you still need them, put a comment at the top that that's what they are for. If it's reasonable, implement them starting from #include from Validation.
Let me know your plan.
Rob
… On Mar 23, 2020, at 4:36 PM, Lisa Goodenough ***@***.***> wrote:
@goodenou commented on this pull request.
In Mu2eG4/fcl/g4test_pot.fcl:
> @@ -67,5 +67,8 @@ services.SeedService.maxUniqueEngines : 20
# physics.producers.g4run.SDConfig.enableAllSDs : true
physics.producers.g4run.SDConfig.enableSD : [tracker, calorimeter, calorimeterRO, CRV, virtualdetector, stoppingtarget ]
physics.producers.g4run.physics.BirksConsts : { G4_POLYSTYRENE : 0.07943 } // {string : mm/MeV }
+
+services.GeometryService.simulatedDetector.tool_type : "Mu2e"
+
physics.producers.g4run.physics.physicsListName : "QGSP_BERT"
I made /Mu2eG4/fcl/g4test_pot*fcl for my own testing. I can delete these if you want.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
…anager random number engine
|
I did the following validation tests: 5k ceSimReco events and 1k potSim events. The agreement between the master branch and my branch was perfect for the non-MT versions of the code. For the MT versions, there were some differences that may be interesting. The pdfs showing the plots are at /mu2e/app/users/goodenou/MT_RNG2_plots/ |
|
@FNALbuild run build test |
kutschke
approved these changes
Mar 24, 2020
|
@FNALbuild run build test |
|
Hi Lisa,
Look in potSimMT_comp.pdf at the plot in the upper right on page 11 and at the plot in the lower right on page 12. It took me a long time convince myself that these are consistent with just random number differences. They are effectively very low stats and very likely represent very low energy electrons that get into the DS. The Y plots look Ok since they integrate over all X and Z so the Y plots are really telling you what's happening everywhere along the beam line.
Rob
… On Mar 24, 2020, at 9:55 AM, Lisa Goodenough ***@***.***> wrote:
I did the following validation tests: 5k ceSimReco events and 1k potSim events. The agreement between the master branch and my branch was perfect for the non-MT versions of the code. For the MT versions, there were some differences that may be interesting. The pdfs showing the plots are at /mu2e/app/users/goodenou/MT_RNG2_plots/
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
|
⌛ The following tests have been triggered for ref b532f22: build |
|
Hi Lisa,
I am ready to do the merge but I will hold off, probably until tomorrow. The issues is scheduled all of the merges in a way that the nightly validation helps us. If we do too many at once it's harder to untangle when something unexpected happens.
Rob
… On Mar 24, 2020, at 9:55 AM, Lisa Goodenough ***@***.***> wrote:
I did the following validation tests: 5k ceSimReco events and 1k potSim events. The agreement between the master branch and my branch was perfect for the non-MT versions of the code. For the MT versions, there were some differences that may be interesting. The pdfs showing the plots are at /mu2e/app/users/goodenou/MT_RNG2_plots/
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
|
OK. Thanks for the clarification. Are they error bars on the eg X plots there statistical? They appear to be less than Sort(N).
… On Mar 24, 2020, at 1:49 PM, Rob Kutschke ***@***.***> wrote:
Hi Lisa,
Look in potSimMT_comp.pdf at the plot in the upper right on page 11 and at the plot in the lower right on page 12. It took me a long time convince myself that these are consistent with just random number differences. They are effectively very low stats and very likely represent very low energy electrons that get into the DS. The Y plots look Ok since they integrate over all X and Z so the Y plots are really telling you what's happening everywhere along the beam line.
Rob
> On Mar 24, 2020, at 9:55 AM, Lisa Goodenough ***@***.***> wrote:
>
>
> I did the following validation tests: 5k ceSimReco events and 1k potSim events. The agreement between the master branch and my branch was perfect for the non-MT versions of the code. For the MT versions, there were some differences that may be interesting. The pdfs showing the plots are at /mu2e/app/users/goodenou/MT_RNG2_plots/
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub, or unsubscribe.
>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#160 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AE44QBWYDFVWYGELPZSPYWTRJD6D7ANCNFSM4LQXDTZA>.
|
|
That should say Sqrt(N), spell correct changed it.
… On Mar 24, 2020, at 2:34 PM, Lisa Goodenough ***@***.***> wrote:
OK. Thanks for the clarification. Are they error bars on the eg X plots there statistical? They appear to be less than Sort(N).
> On Mar 24, 2020, at 1:49 PM, Rob Kutschke ***@***.*** ***@***.***>> wrote:
>
>
> Hi Lisa,
>
> Look in potSimMT_comp.pdf at the plot in the upper right on page 11 and at the plot in the lower right on page 12. It took me a long time convince myself that these are consistent with just random number differences. They are effectively very low stats and very likely represent very low energy electrons that get into the DS. The Y plots look Ok since they integrate over all X and Z so the Y plots are really telling you what's happening everywhere along the beam line.
>
>
> Rob
>
>
> > On Mar 24, 2020, at 9:55 AM, Lisa Goodenough ***@***.*** ***@***.***>> wrote:
> >
> >
> > I did the following validation tests: 5k ceSimReco events and 1k potSim events. The agreement between the master branch and my branch was perfect for the non-MT versions of the code. For the MT versions, there were some differences that may be interesting. The pdfs showing the plots are at /mu2e/app/users/goodenou/MT_RNG2_plots/
> >
> > —
> > You are receiving this because you were mentioned.
> > Reply to this email directly, view it on GitHub, or unsubscribe.
> >
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub <#160 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AE44QBWYDFVWYGELPZSPYWTRJD6D7ANCNFSM4LQXDTZA>.
>
|
|
I am sure that wars will be started because of autocorrect. The error bars look like sqrt(N) to me. Note that there are only about 13000 entries inside the boundary of this plot. What gets into this plot? I bet that all of the SimParticles stopped or produced in this region are low energy electrons, positrons and photons. There might be 1 or 2 muons in there. Also remember that the 3 "start xyz DS" and the 3 "end xyz DS" plots are not cut on the other variables. If you still have the art files for these, define a DS region with cuts on all 3 x,y,z variables; use the ranges of xyz used for the plots. Then make a histograms of the number of StepPointMCs per event that start in that region. Then make a scatter plot of x vs z for values of y in the range. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The random number seeds for the worker run managers are seeded by the MT run manager. 10k seeds are created at a time by a MixMax engine. The initial seed for this generator is set by the fcl parameter 'initialSeed'. The number of seeds needed for the run, and thus the events that will be processed, does not need to be known at run start time. The seed algorithm will create seeds in batches of 10k seeds until there are no events left to process.