Skip to content

Conversation

@PetrilloAtWork
Copy link
Member

@PetrilloAtWork PetrilloAtWork commented Sep 10, 2022

The policy now uses random as fallback policy for per-event seeds.

This addresses issue #243.

The background: LArSoft provides a policy for providing seeds to the random number generators which ensures that the same event will always receive the same seeds, guaranteeing job reproducibility. Unfortunately that policy does not define what happens when seeds are required outside the event boundaries; a typical case is the LArSoft implementation of CORSIKA interface, CORSIKAGen, which at beginning of the job decides randomly which pool of pre-generated events to use in the job. This choice happens at a time when no event is being processed (no input file, in fact, has been opened yet). The undefined behaviour of the per-event policy was in fact to always use 0 as a seed, which would cause the same pool of cosmic ray events to be used for every job.

While LArSoft offers no real solution to this issue, it now provide a workaround in allowing to control which seeds are provided at that time. My choice here is to use a random policy, which does not guarantee reproducibility (e.g. rerunning a job on a certain input will pick a different CORSIKA event pool) but at least behaves as a random generator.

This pull request defines this new configuration, defines also an alias icarus_default_NuRandomService supposed to define the "default" random seed service configuration for ICARUS, and then it uses the latter wherever there is the need to configure that service.

This may be a breaking change for the simulation, since different random streams may be generated for some of the modules, but it will likely not be noticed by the integration tests, which fix the random seeds overriding the standard configuration settings.

This pull request has been tested running the unit tests, a prodcorsika_genie_standard_icarus.fcl job, a standard_g4_icarus.fcl job and a standard_detsim_icarus.fcl job.

The policy now uses random as fallback policy for pre-event seeds.
This addresses issue #243.
@PetrilloAtWork PetrilloAtWork added the breaking this change will break backward compatibility in some way label Sep 10, 2022
Copy link
Contributor

@SFBayLaser SFBayLaser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is a good change to make. Thank you!

@rennney
Copy link
Contributor

rennney commented Sep 26, 2022

trigger build

@FNALbuild
Copy link
Collaborator

✔️ CI build for LArSoft Succeeded on slf7 for c7:prof -- details available through the CI dashboard

@FNALbuild
Copy link
Collaborator

✔️ CI build for LArSoft Succeeded on slf7 for e20:prof -- details available through the CI dashboard

@FNALbuild
Copy link
Collaborator

⚠️ CI build for ICARUS Warning at phase ci_tests ICARUS on slf7 for c7:prof - ignored warnings for build -- details available through the CI dashboard

🚨 For more details about the warning phase, check the ci_tests ICARUS phase logs

parent CI build details are available through the CI dashboard

@FNALbuild
Copy link
Collaborator

⚠️ CI build for ICARUS Warning at phase ci_tests ICARUS on slf7 for e20:prof - ignored warnings for build -- details available through the CI dashboard

🚨 For more details about the warning phase, check the ci_tests ICARUS phase logs

parent CI build details are available through the CI dashboard

@rennney rennney merged commit da08113 into develop Sep 28, 2022
@rennney rennney deleted the feature/gp_issue243 branch September 28, 2022 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking this change will break backward compatibility in some way

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants