Skip to content

Use np.RansdomState in tests - #1312

Merged
nden merged 3 commits into
astropy:masterfrom
nden:use-random-state
Aug 21, 2013
Merged

nden merged 3 commits into
astropy:masterfrom
nden:use-random-state

Conversation

@nden

@nden nden commented Aug 1, 2013

Copy link
Copy Markdown
Contributor

This is relevant mostly for tests of iterative algorithms.

@mdboom

mdboom commented Aug 2, 2013

Copy link
Copy Markdown
Contributor

I've restarted the build to hopefully fix the other test that was randomly broken in master. Once that goes though, I'm 💯 on this getting merged.

@nden nden mentioned this pull request Aug 5, 2013
@astrofrog

Copy link
Copy Markdown
Member

@nden - there is still a failure - is there a test for which the random state is still not being set?

@astrofrog

Copy link
Copy Markdown
Member

@nden - since the test is still failing but should be deterministic, maybe this is just a case of changing the random seed?

@nden

nden commented Aug 17, 2013

Copy link
Copy Markdown
Contributor Author

@astrofrog This was my thought too but shouldn't this be also deterministic across machines. The tests pass for me locally. I changed it we'll see what happens now. (Sorry for not being very responsive. I have to finish something else before I leave for a month at the end of August. I'll also try to do as much as I can for the 0.3 release but it may not be much.)

@astrofrog

Copy link
Copy Markdown
Member

The tests pass, but I'm going to restart Travis to double check.

@nden

nden commented Aug 21, 2013

Copy link
Copy Markdown
Contributor Author

@astrofrog Tests passed again. There's nothing controversial about this so I am going to merge it. If we see this test failing again we'll revisit

nden added a commit that referenced this pull request Aug 21, 2013
@nden
nden merged commit a3953c5 into astropy:master Aug 21, 2013
@astrofrog

Copy link
Copy Markdown
Member

@nden - sounds good!

@eteq

eteq commented Aug 21, 2013

Copy link
Copy Markdown
Member

@nden - this is perfectly fine as it was merged, but I just wanted to point out astropy.utils.misc.NumpyRNGContext - you can see an example of its use at https://github.com/astropy/astropy/blob/master/astropy/stats/tests/test_funcs.py . It's basically a decorator that sets the numpy RNG seed so you can just use the regular np.random.* functions but have them be deterministic.

RandomState should do essentially the same thing, so whichever is easier in a given case is fine.

@nden

nden commented Aug 21, 2013

Copy link
Copy Markdown
Contributor Author

@eteq Thanks for pointing this out, I didn't know about it.

@nden
nden deleted the use-random-state branch April 30, 2018 12:17
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.

4 participants