Refactor transformations by extracting probabilities to get_params() method

[kondela]

Fixes: #3066

Transformations that need to be refactored since they call torch.rand(1) in forward():

• RandomApply
• RandomHorizontalFlip
• RandomVerticalFlip
• RandomGrayscale
• RandomErasing
• RandomPerspective

Not sure how to handle the last two as they already have get_params() method define and the probability needs to be drawn before get_params() is called, for example RandomPerspective():

vision/torchvision/transforms/transforms.py

Lines 708 to 711 in 7f1a05a

	if torch.rand(1) < self.p:
	width, height = F._get_image_size(img)
	startpoints, endpoints = self.get_params(width, height, self.distortion_scale)
	return F.perspective(img, startpoints, endpoints, self.interpolation, fill)

[fmassa]

Hi,

Thanks for the PR!

I've just made a comment in #3065 (comment) which explains my reasoning behind get_params. Let me know what you think

[kondela]

Hi,

Thanks for the PR!

I've just made a comment in #3065 (comment) which explains my reasoning behind get_params. Let me know what you think

Hi, it seems that the discussion in that thread is still ongoing. Do we want to merge it as it is, i.e leave RandomErasing(), RandomPerspective() unchanged or how do we proceed with these two as we can't simply extract torch.rand(1).item() to get_params()?

Thanks!

[datumbox]

@kondela I wonder if you don't mind to finish the 2 remaining methods so that this PR is complete. I have also a bunch of operators that are pending to be merged and use the specific idiom, so if we decide to return a boolean instead of a weight, I can fix yours and mine in the same PR. I don't really have a preference, let me know. :)

[kondela]

@kondela I wonder if you don't mind to finish the 2 remaining methods so that this PR is complete. I have also a bunch of operators that are pending to be merged and use the specific idiom, so if we decide to return a boolean instead of a weight, I can fix yours and mine in the same PR. I don't really have a preference, let me know. :)

Hey @datumbox I refactored the last two transformations, please see if its okay. I was hoping for a hint since as I said in #3098 (comment) that these two transformations are different.

[datumbox]

I was hoping for a hint since as I said in #3098 (comment) that these two transformations are different.

Hmm, I just saw that you edited your original message. You are right, let us get back to you on this one.

[kondela]

I was hoping for a hint since as I said in #3098 (comment) that these two transformations are different.

Hmm, I just saw that you edited your original message. You are right, let us get back to you on this one.

Ok, let me know :). I will wait with fixing the failing tests.

[datumbox]

@kondela FYI we have not forgotten about this. It's just unclear right now which path we are going to take.

Note that we recently merged PR #3123 which adds more transforms. Originally I took the approach of your PR for structuring the random params but I rolled it back (see #3123 (comment)) until we are certain on how to handle this.

[datumbox]

Unfortunately, it seems we need to rethink the plan to cover better cases such as Object Detection. See #3286 for more details on the discussion.

@kondela Thank you for sending the PR, I'll close it for now but I'm happy to ping you once we have a more concrete proposal on how to move forwards.