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

[BUG] check_env_specs shouldn't set torch.manual_seed by default. #1855

Closed
2 of 3 tasks
maximilianigl opened this issue Jan 31, 2024 · 3 comments · Fixed by #1872
Closed
2 of 3 tasks

[BUG] check_env_specs shouldn't set torch.manual_seed by default. #1855

maximilianigl opened this issue Jan 31, 2024 · 3 comments · Fixed by #1872
Assignees
Labels
bug Something isn't working

Comments

@maximilianigl
Copy link

Describe the bug

Current default behaviour of check_env_specs() is to set the manual key to 0.
I think this is dangerous as people might run this function after setting their own seed (at the beginning of main(), not realizing it's overwriting what they set before.

Reason and Possible fixes

Set default argument of seed in check_env_specs to None.

Checklist

  • I have checked that there is no similar issue in the repo (required)
  • I have read the documentation (required)
  • I have provided a minimal working example to reproduce the bug (required)
@maximilianigl maximilianigl added the bug Something isn't working label Jan 31, 2024
@vmoens
Copy link
Contributor

vmoens commented Jan 31, 2024

Yep I agree it isn't ideal.

Note that the intended use of this function is to be run out of your training loop.

I wish PyTorch had some more refined way of locally set the seed but as of now it isn't the case.

I can have a look at not seeding things by default

@vmoens vmoens changed the title [BUG] check_env_specs shouldn't set torch.manual_key by default. [BUG] check_env_specs shouldn't set torch.manual_seed by default. Jan 31, 2024
@vmoens vmoens linked a pull request Feb 5, 2024 that will close this issue
@vmoens
Copy link
Contributor

vmoens commented Feb 5, 2024

If you want to have a look at #1872 it should be ready

@maximilianigl
Copy link
Author

Looks great, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants