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

Deprecate jumpahead in get_random_state #711

Closed
atrettin opened this issue Nov 24, 2022 · 5 comments · Fixed by #745
Closed

Deprecate jumpahead in get_random_state #711

atrettin opened this issue Nov 24, 2022 · 5 comments · Fixed by #745
Milestone

Comments

@atrettin
Copy link
Contributor

The jumpahead argument in pisa.utils.random_numbers.get_random_state does not result in independent sequences of output numbers if the random state is used to generate random numbers. It doesn't even reliably move the sequence of output numbers ahead by the given number, because calling rand does not mean that the internal Mersenne twister is really moved by one step (and really, we don't have access to numpy internals and shouldn't make such assumptions anyway).

A function that minimally implements the behavior of the PISA method is:

def get_poisson_sequence(size, seed=0, jumpahead=0):
    rs = np.random.RandomState(seed=seed)
    rs.rand(jumpahead)
    
    return rs.poisson(size=size)

With the right settings for jumpahead, we can repeat sequences exactly after a few initially different numbers:

get_poisson_sequence(15, seed=0, jumpahead=1)
>>> array([2, 1, 3, 2, 1, 0, 0, 5, 1, 1, 2, 0, 1, 1, 2])
get_poisson_sequence(15, seed=0, jumpahead=3)
>>> array([1, 1, 2, 2, 1, 0, 0, 5, 1, 1, 2, 0, 1, 1, 2])

We should deprecate jumpahead, as people can be mislead and use it to generate trials that they think are independent. My suggestion is to raise an Exception (no one cares about warnings) with instructions about how to fix the problem (by using a different seed instead of jumpahead).

@philippeller
Copy link
Collaborator

good catch! Let's do what you suggest and raise an exception if anyone uses this feature and disable it

@LeanderFischer LeanderFischer added this to the PISA 4.2 milestone Jul 14, 2023
@LeanderFischer
Copy link
Collaborator

LeanderFischer commented Jul 14, 2023

This hasn't been implemented, right? edit: no and once it gets implemented, the unit test should probably check for it.

@atrettin
Copy link
Contributor Author

Yeah this can trip people up when they make pseudo-data trials and they wonder why their trials don't look so independent. If this is still a thing then yes it should definitely get fixed! 😅

@LeanderFischer LeanderFischer linked a pull request Jul 14, 2023 that will close this issue
@LeanderFischer
Copy link
Collaborator

Interestingly, the OG version of the unit test, using jumpahead worked 🤔

@atrettin
Copy link
Contributor Author

It doesn't always fail to produce independent series. That's the terrible thing about it. :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants