Skip to content

UserWarning: You set iterator_valid__shuffle=True after upgrading to 0.12.0, but it is False #907

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

Closed
rhjohnstone opened this issue Oct 13, 2022 · 4 comments · Fixed by #908
Labels

Comments

@rhjohnstone
Copy link

After upgrading to 0.12.0, I get the warning before training starts:

UserWarning: You set iterator_valid__shuffle=True; this is most likely not what you want because the values returned by predict and predict_proba will be shuffled.

However, I definitely create the model with iterator_valid__shuffle = False. And, just to confirm, after training I do

print(estimator.get_params()["iterator_valid__shuffle"])  # False

I'm hoping the warning itself is wrong, otherwise I don't see how I can fix this within my own code.

@rhjohnstone
Copy link
Author

I'm pretty sure this is a Skorch bug in 0.12.0.

In an earlier version, this parameter was checked here:

        # warn about usage of iterator_valid__shuffle=True, since this
        # is almost certainly not what the user wants
        if kwargs.get('iterator_valid__shuffle'):
            warnings.warn(
                "You set iterator_valid__shuffle=True; this is most likely not "
                "what you want because the values returned by predict and "
                "predict_proba will be shuffled.",
                UserWarning)

so it's actually looking up the value of iterator_valid__shuffle.

However, in the new version, it is done here:

        # warn about usage of iterator_valid__shuffle=True, since this
        # is almost certainly not what the user wants
        if 'iterator_valid__shuffle' in self._params_to_validate:
            warnings.warn(
                "You set iterator_valid__shuffle=True; this is most likely not "
                "what you want because the values returned by predict and "
                "predict_proba will be shuffled.",
                UserWarning)

so it's only checking for the existence of iterator_valid__shuffle as a kwarg when creating the estimator, not checking its value.

@BenjaminBossan
Copy link
Collaborator

Thank you for reporting this, it's indeed a bug, showing a false positive. In your case, could you just not set the value to False, as it's the default anyway?

BenjaminBossan added a commit that referenced this issue Oct 13, 2022
Resolves #907

The problem initially occurred because the warning did not check for the
value, just the presence of the key.

Even though there is a test for this, the test didn't detect the error.
This is because during a refactor (#751), the parameter validation was
moved to initialize() from __init__() but the test was not adjusted to
take the change into account.
@BenjaminBossan
Copy link
Collaborator

@rhjohnstone I created a bugfix PR (#908). As this issue is quite minor, we probably won't make a patch release of skorch, I hope that's okay.

@rhjohnstone
Copy link
Author

@BenjaminBossan thanks! No problem about a patch release; I really just wanted to confirm that what I thought was indeed happening.

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

Successfully merging a pull request may close this issue.

2 participants