Port random affine, rotate, perspective and to_grayscale to pytest #4000
Conversation
There was a problem hiding this comment.
Thanks @vivekkumar7089 ! a few comments but this looks good
test/test_transforms_tensor.py
Outdated
| self.device = "cuda" | ||
|
|
||
|
|
||
| def _test(device, **kwargs): |
There was a problem hiding this comment.
let's call this _test_random_affine_helper to be more specific
| with get_tmp_dir() as tmp_dir: | ||
| s_transform.save(os.path.join(tmp_dir, "t_random_rotate.pt")) |
There was a problem hiding this comment.
Let's move this into a separate test: with all the parametrization, it's called many many times and adds up about 4 seconds to the execution, which is no negligible. We just need to call it once in a test called e.g. test_random_rotate_save
There was a problem hiding this comment.
Hi @NicolasHug !! I have changed everything, the rest of this. Actually, I couldn't find a way to reuse s_transform outside of its parent function. Could you please help me to solve this problem?
There was a problem hiding this comment.
I made a comment above, hopefully this helps? LMK!
There was a problem hiding this comment.
It means here I can define a function.
def test_random_rotate_save():
s_transform = torch.jit.scriptT.RandomAffine(...)
with get_tmp_dir() as tmp_dir:
s_transform.save(os.path.join(tmp_dir, "t_random_rotate.pt")
And also in a similar manner in the function test_random_perspective.
@NicolasHug
There was a problem hiding this comment.
Yes this sounds like the right way here!
There was a problem hiding this comment.
Hello @NicolasHug !!
s_transform = torch.jit.script(T.RandomAffine())
and
s_transform = torch.jit.script(T.RandomRotation())
both are giving same error -: TypeError: init() missing 1 required positional argument: 'degrees'
while
s_transform = torch.jit.script(T.RandomPerspective()) is working fine.
Can we fix some degrees here? (in above cases)
There was a problem hiding this comment.
yes we can just pass degree=0
EDIT - or 45 as you did!
| with get_tmp_dir() as tmp_dir: | ||
| s_transform.save(os.path.join(tmp_dir, "t_perspective.pt")) |
test/test_transforms_tensor.py
Outdated
|
|
||
|
|
||
| @pytest.mark.parametrize('device', cpu_and_gpu()) | ||
| @pytest.mark.parametrize('tol', [1.0 + 1e-10]) |
There was a problem hiding this comment.
as there's only one value, we don't need to parametrize over it. Let's just write tol = ... in the function body
test/test_transforms_tensor.py
Outdated
| @pytest.mark.parametrize('device', cpu_and_gpu()) | ||
| @pytest.mark.parametrize('tol', [1.0 + 1e-10]) | ||
| @pytest.mark.parametrize('num_output_channels', [0, 1, 3]) | ||
| def test_to_grayscale(device, num_output_channels, tol): |
There was a problem hiding this comment.
here maybe it woudl make more sense to parametrize over
@pytest.mark.parametrize('Klass, meth_kwargs', [
(T.Grayscale, {"num_output_channels": 1}),
(T.Grayscale, {"num_output_channels": 3}),
(T.RandomGrayscale, {}),so that we can remove the if/else in the test?
There was a problem hiding this comment.
Thanks, @NicolasHug !! I will take care of all comments which you have made.
test/test_transforms_tensor.py
Outdated
|
|
||
| @pytest.mark.parametrize('device', cpu_and_gpu()) | ||
| def test_random_affine(device): | ||
| s_transform = _test(device, degrees=0.0) |
There was a problem hiding this comment.
I realize this was already present, but we don't need to get s_transform from _test(). _test() doesn't need to return anything and we can just declare s_transform here as s_transform = torch.jit.scriptT.RandomAffine(...)
(where ... may actually be empty)
|
I took care of a minor pep8 issue, thanks a lot @vivekkumar7089 ! |
…pytest (#4000) Reviewed By: fmassa Differential Revision: D29097740 fbshipit-source-id: c8303ca0665d6ee7e2420b7be9ce6eabe483a010
Refactor Group D as mentioned in #3987