Skip to content

Possible BC break by the use of torch.rand() #2520

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
hkchengrex opened this issue Jul 30, 2020 · 2 comments
Closed

Possible BC break by the use of torch.rand() #2520

hkchengrex opened this issue Jul 30, 2020 · 2 comments

Comments

@hkchengrex
Copy link

I find that random.random() was replaced by torch.rand() in some of the random transforms (e.g. RandomHorizontalFlip) not in some others (e.g. RandomGrayscale).

I believe a lot of users rely on the previous random.random() behavior by manually seeding random to apply an identical transform to the image/segmentation/etc. See this highly upvoted (and thus copied and used) comment: #9 (comment)

Existing code will still train with inferior performance and it is hard to notice. This is kind of horrible.

If the change is inevitable, maybe it can at least be mentioned in the release note?

@fmassa
Copy link
Member

fmassa commented Jul 30, 2020

Hi,

Thanks for opening the issue.

This change is part of a larger change that we haven't yet finished for the 0.7.0 release, and for the next release all classes will use torch.rand() instead of random.random().

I agree we should have mentioned it more broadly in the release notes though, will make a note there.

There is unfortunately no way to avoid the change to random.random() for now (it is dependent on torchscript support for random, which is not available).

I will edit the comment you pointed out mentioning that we should now also seed torch.manual_seed in order for that behavior to work, but note that seeding the random seed in the __getitem__ is a fragile approach and I would definitely NOT recommend doing it. Instead, I would encourage using the functional interface, while we don't come up with a better proposal for this, see #1406.

@fmassa
Copy link
Member

fmassa commented Jul 30, 2020

Updated the release notes and the comment in #9 (comment) , thanks a lot for the feedback @hkchengrex !

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

No branches or pull requests

2 participants