Skip to content
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

Reset random state on seed change #2130

Merged
merged 8 commits into from
Oct 1, 2021
Merged

Conversation

jvesely
Copy link
Collaborator

@jvesely jvesely commented Sep 29, 2021

The original semantics was to reset random state once per context with the most up-to-date value of the seed.
The new approach is to reset random state every time the seed value changes, whether through direct assignment or modulation.

The practical change in semantics is subtle and helps match results between virtual executions and the simulated composition (see the comments in 'test_modulation_of_random_state' for details).

…function instead of RandomState

Results change since the 'seed' parameter is converted to list before use.
Signed-off-by: Jan Vesely <[email protected]>
Drop custom in-flight function check.
Remove any cost calculation and use generated random value as objective metric.

Signed-off-by: Jan Vesely <[email protected]>
…ialize parameters.random_state

Fix function copy workaround to reset the seed parameter,
the state will be adjusted by the seed setter function.
Use DEFAULT_SEED to avoid advancing global seed.

Signed-off-by: Jan Vesely <[email protected]>
@jvesely jvesely requested a review from kmantel September 29, 2021 06:06
@github-actions
Copy link

This PR causes the following changes to the html docs (ubuntu-latest-3.7-x64):

No differences!

...

See CI logs for the full diff.

@lgtm-com
Copy link
Contributor

lgtm-com bot commented Sep 29, 2021

This pull request fixes 1 alert when merging b0898ce into aea0621 - view on LGTM.com

fixed alerts:

  • 1 for Except block handles 'BaseException'

Instead of resetting the PRNG state once per context,
the new approach compares the originally used seed with the new value
(either new setting or modulation).

This allows for better match in multi-trial executions with OCM:
Simulations will only reset PRNG if the modulated seed is different
from the one in the current execution context.
The result of evaluation will thus match the result of trial even
if OCM selects the same seed as originally used by the execution.

The PRNG state is also reset on direct seed modification to preserve
the old behaviour in situations like;
Func1.seed = n
Func2.seed = n
# Both Func1 and Func2 now follow the same random sequence irrespective
# of their original seed

Signed-off-by: Jan Vesely <[email protected]>
…ials

Construct the expected values in the test instead of using constants
to verify the semantics of reusing random seeds.

Signed-off-by: Jan Vesely <[email protected]>
@github-actions
Copy link

This PR causes the following changes to the html docs (ubuntu-latest-3.7-x64):

No differences!

...

See CI logs for the full diff.

@lgtm-com
Copy link
Contributor

lgtm-com bot commented Sep 29, 2021

This pull request fixes 1 alert when merging f32638b into aea0621 - view on LGTM.com

fixed alerts:

  • 1 for Except block handles 'BaseException'

@github-actions
Copy link

github-actions bot commented Oct 1, 2021

This PR causes the following changes to the html docs (ubuntu-latest-3.7-x64):

No differences!

...

See CI logs for the full diff.

@lgtm-com
Copy link
Contributor

lgtm-com bot commented Oct 1, 2021

This pull request fixes 1 alert when merging 429ffea into 2c3a429 - view on LGTM.com

fixed alerts:

  • 1 for Except block handles 'BaseException'

@jvesely jvesely merged commit 5405ac9 into PrincetonUniversity:devel Oct 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants