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

fix!: remove fixed_seed and add pl.set_random_seed #10388

Merged
merged 5 commits into from Aug 16, 2023

Conversation

orlp
Copy link
Collaborator

@orlp orlp commented Aug 9, 2023

Fixes #10367.

This is a partial revert of #9694. We removed the fixed_seed parameter. Calling shuffle(seed=n) should always do the same thing, instead, in the future we should support shuffle(seed=expr) that allows you to set the seed per groupby if you wish to have deterministic but different seeds per group.

This also adds a new function pl.set_random_seed that you can use to make the internal Polars RNG deterministic, instead of relying on Python's random.seed. Note that you might still get non-deterministic results in queries due to threading order.

@github-actions github-actions bot added breaking python Change that breaks backwards compatibility for the Python package breaking rust Change that breaks backwards compatibility for the Rust crate fix Bug fix python Related to Python Polars rust Related to Rust Polars labels Aug 9, 2023
Copy link
Member

@ritchie46 ritchie46 left a comment

Choose a reason for hiding this comment

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

Save, from some merge conflicts, I think this looks great.

I think we have to queue a few breaking changes, and then we can bump in a few weeks.

@avimallu
Copy link
Contributor

avimallu commented Aug 9, 2023

Does this mean that hashes will always be deterministic now, and not depend on architecture or version? Reference: #7758 (comment)

Does this also effectively allow for a global seed? Then might it close #3076?

@orlp
Copy link
Collaborator Author

orlp commented Aug 9, 2023

This does indeed close #3076. It doesn't change anything about the hashing at the moment.

@stinodego stinodego changed the title fix(rust!,python!): remove fixed_seed and add pl.set_random_seed fix!: remove fixed_seed and add pl.set_random_seed Aug 11, 2023
@stinodego stinodego removed breaking rust Change that breaks backwards compatibility for the Rust crate breaking python Change that breaks backwards compatibility for the Python package labels Aug 11, 2023
@github-actions github-actions bot added the breaking Change that breaks backwards compatibility label Aug 11, 2023
@orlp orlp added this to the 0.19 milestone Aug 16, 2023
@stinodego
Copy link
Member

@orlp can you rebase and merge this one?

@orlp orlp merged commit af4555c into pola-rs:main Aug 16, 2023
24 of 25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Change that breaks backwards compatibility fix Bug fix python Related to Python Polars rust Related to Rust Polars
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Test in CI is failing
4 participants