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

Use SeedSequence in RandomStream #939

Conversation

brandonwillard
Copy link
Member

@brandonwillard brandonwillard commented Apr 29, 2022

This PR closes #936 and removes the old/deprecated attributes that RandomStream adds to the variables it generates (e.g. .rng, .update).

@brandonwillard brandonwillard self-assigned this Apr 29, 2022
@brandonwillard brandonwillard added enhancement New feature or request random variables Involves random variables and/or sampling important labels Apr 29, 2022
Copy link
Contributor

@ricardoV94 ricardoV94 left a comment

Choose a reason for hiding this comment

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

Looks good. Is there a reason why we keep supporting RandomState instead of RandomGenerator alone?

@brandonwillard
Copy link
Member Author

Looks good. Is there a reason why we keep supporting RandomState instead of RandomGenerator alone?

Mostly just for backward compatibility, but, now, it's useful for the Numba backend as well—at least until something like numba/numba#7900 goes through.

@brandonwillard brandonwillard force-pushed the use-SeedSequence-in-RandomStream branch 2 times, most recently from 43e4dc9 to aa6d2b9 Compare April 29, 2022 05:20
@brandonwillard brandonwillard force-pushed the use-SeedSequence-in-RandomStream branch from aa6d2b9 to b6dc523 Compare April 29, 2022 05:24
@codecov
Copy link

codecov bot commented Apr 29, 2022

Codecov Report

Merging #939 (b6dc523) into main (c19ac79) will increase coverage by 0.00%.
The diff coverage is 95.45%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #939   +/-   ##
=======================================
  Coverage   78.91%   78.91%           
=======================================
  Files         152      152           
  Lines       47657    47660    +3     
  Branches    10852    10853    +1     
=======================================
+ Hits        37609    37612    +3     
  Misses       7548     7548           
  Partials     2500     2500           
Impacted Files Coverage Δ
aesara/sandbox/rng_mrg.py 84.11% <ø> (-0.04%) ⬇️
aesara/compile/nanguardmode.py 62.37% <50.00%> (+0.37%) ⬆️
aesara/tensor/random/utils.py 100.00% <100.00%> (ø)

@brandonwillard brandonwillard merged commit 3e42c5c into aesara-devs:main Apr 29, 2022
@brandonwillard brandonwillard deleted the use-SeedSequence-in-RandomStream branch April 29, 2022 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request important random variables Involves random variables and/or sampling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use SeedSequence in RandomStream
2 participants