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.

Sign up for GitHub

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

Jump to bottom

[tune] Add random state to BasicVariantGenerator #20926

Merged
merged 14 commits into from
Dec 8, 2021
Merged

[tune] Add random state to BasicVariantGenerator #20926

merged 14 commits into from
Dec 8, 2021

Conversation

Yard1
Copy link
Member

@Yard1 Yard1 commented Dec 6, 2021 •
edited

Why are these changes needed?

This PR adds an ability to set a random seed/numpy random generator in BasicVariantGenerator, allowing for reproducibility across separate runs. All the changes are fully backwards compatible.

Related issue number

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 :(

Yard1
Yard1 commented Dec 7, 2021
View reviewed changes
Copy link
Member Author

@Yard1 Yard1 left a comment

Choose a reason for hiding this comment

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

nits

python/ray/tune/suggest/basic_variant.py Outdated Show resolved Hide resolved
python/ray/tune/tests/test_sample.py Outdated Show resolved Hide resolved
krfricke
krfricke reviewed Dec 8, 2021
View reviewed changes
Copy link
Contributor

@krfricke krfricke left a comment

Choose a reason for hiding this comment

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

Awesome, just some minor comments

python/ray/tune/sample.py Outdated Show resolved Hide resolved
python/ray/tune/sample.py Outdated Show resolved Hide resolved
python/ray/tune/suggest/variant_generator.py Outdated Show resolved Hide resolved
Yard1 and others added 4 commits December 8, 2021 10:29
@Yard1 @krfricke
Apply suggestions from code review
a15b979 
Co-authored-by: Kai Fricke <krfricke@users.noreply.github.com>
@Yard1
Typing
8b52db3
@Yard1
Merge branch 'basic_variant_set_seed' of https://github.com/Yard1/ray …
c6be86b 
…into basic_variant_set_seed
@Yard1
Nits
543837f
@Yard1 Yard1 requested a review from krfricke December 8, 2021 09:36
krfricke
krfricke reviewed Dec 8, 2021
View reviewed changes
Copy link
Contributor

@krfricke krfricke left a comment

Choose a reason for hiding this comment

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

Some nits

python/ray/tune/sample.py Outdated Show resolved Hide resolved
python/ray/tune/sample.py Outdated Show resolved Hide resolved
python/ray/tune/sample.py Outdated Show resolved Hide resolved
python/ray/tune/sample.py Outdated Show resolved Hide resolved
python/ray/tune/suggest/variant_generator.py Outdated Show resolved Hide resolved
@Yard1 Yard1 requested a review from krfricke December 8, 2021 16:51
@krfricke krfricke merged commit 4c47f56 into ray-project:master Dec 8, 2021
@Yard1 Yard1 deleted the basic_variant_set_seed branch December 8, 2021 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@krfricke krfricke krfricke approved these changes

@xwjiang2010 xwjiang2010 Awaiting requested review from xwjiang2010

Assignees

@krfricke krfricke

@xwjiang2010 xwjiang2010

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@Yard1 @krfricke @xwjiang2010