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

[tune] fix non-deterministic category sampling by switching back to np.random.choice #13710

Merged
merged 8 commits into from
Jan 27, 2021

Conversation

krfricke
Copy link
Contributor

Why are these changes needed?

Sampling categories in Ray Tune was non-deterministic even when setting a seed. It is unclear where this comes from exactly, but it might be some external library using python's random module. Switching back tune.choice sampling to np.random.choice fixes this issue.

Note that we introduced the switch to random.choice in #9451 because of typing incompatibilities. However, transforming the sampled choices to a list should fix these issues. An alternative would be to sample indices and return a list of these indices, but we'll try this regular route as it should be more efficient.

Related issue number

Closes #13295

Original issue for switching from np.random.choice to random.choice: #8976
Related PR: #9451

Checks

  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

Copy link
Member

@sumanthratna sumanthratna left a comment

Choose a reason for hiding this comment

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

lgtm, assuming tests pass

python/ray/tune/tests/test_sample.py Show resolved Hide resolved
@krfricke krfricke merged commit 2664a2a into ray-project:master Jan 27, 2021
@krfricke krfricke deleted the tune-preserve-seed branch January 27, 2021 15:42
fishbone pushed a commit to fishbone/ray that referenced this pull request Feb 16, 2021
…np.random.choice` (ray-project#13710)

* Enable zoopt tests again, but wait for next release

* Add test and preserve state in trial executor

* Add baseline check with integers

* [tune] fix non-deterministic category sampling, re-enable zoopt tests

* Remove random import

* Disable zoopt tests
fishbone added a commit to fishbone/ray that referenced this pull request Feb 16, 2021
fishbone added a commit to fishbone/ray that referenced this pull request Feb 16, 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.

[tune] categorical sampling is non-deterministic even after setting seed
2 participants