Allow more consistent degrees=None in RandomAffine constructor

[vadimkantorov]

🐛 Describe the bug

All other arguments allow for setting None to desactivate, except degrees. For the cases of when only some of these arguments are set as kwargs, allowing same values for desactivation is a bit better.

Versions

N/A

[datumbox]

Thanks for the proposal. This is probably a feature request/enhancement rather than a bug.

cc @vfdev-5 as I know you've been looking into affine recently.

[vfdev-5]

Basically, the proposal is about

- RandomAffine(0, translate=(0.2, 0.3))
+ RandomAffine(translate=(0.2, 0.3))

It is possible to implement that, while thinking about raising an error for RandomAffine().

[vadimkantorov]

I would even say that RandomAffine() without any arguments should not raise an error and just be equivalent to Identity

[vfdev-5]

RandomAffine() == Identity I'm not sure about such API, e.g. why to use RandomAffine() as indentity. But we can iterate over that if there can be some arguments for that.

@vadimkantorov would you like to send a PR with this feature ?

[vadimkantorov]

why to use RandomAffine() as indentity.

I think no-one would use RandomAffine() in place of identity, but it may simplify error handling in user code that just pass through the arguments to RandomAffine (e.g. optional affine transform as part of more complex augmentation pipeline), and then it's better to have simpler error handling and ideally avoid it altogether (just like it's meaningful to sometimes support zero batch size / empty tensors etc, it makes handling of certain edge cases much simpler in the user code)

[vfdev-5]

For me it is a bit different. I'd compare that to configuring Conv2d() without any params and say that it should behave as Identity in this case. In general, Conv2d should do a convolution and should be configured for that. The same logic I'd apply to RandomAffine. A use-case where RandomAffine configuration can be a sort of input (as you talk about zero batch tensors etc) is when user does some search on dataaug and reconfigure RandomAffine differently during the search and would like to provide an empty config and have everything working. In general case, I'd say we have to raise an error on empty configuration.

[vadimkantorov]

Yeah, my argument was about your second paragraph. Imagine a complex augmentation pipeline that reads affine params from config (or constructor arguments) - this happens in practice quite a lot. In this complex pipeline RandomAffine is just one component. For this setting, disabling Affine transform by letting the final user to set all affine params None / zero is a valid usecase, and no error (identity stub) frees the user from error handling or special-casing this behavior. e.g. Dropout with p = 0 or p = 1 is accepted, in the former case Identity is performed, but of course torchvision can deviate from torch core designs. The example about zero batch size was to showcase the value of no error checking / special casing for end-user experience when some trivial output can be provided (even if the output is trivial and not very useful as computation result or a value by itself).

I also don't mind allowing plain 0 (in place of None) to specify Identity behavior (although probably if all-zeros is accepted, all-Nones could be accepted as well).