From 76b7c92258812de8b89d909a51609610083afa7c Mon Sep 17 00:00:00 2001 From: vfdev Date: Fri, 25 Aug 2023 12:08:39 +0000 Subject: [PATCH 1/7] fix sigma input type for v2.GaussianBlur --- test/test_transforms_v2.py | 1 + test/test_transforms_v2_refactored.py | 18 ++++++++++++++---- torchvision/transforms/v2/_utils.py | 13 ++++++++----- 3 files changed, 23 insertions(+), 9 deletions(-) diff --git a/test/test_transforms_v2.py b/test/test_transforms_v2.py index c92a8cd7c52..d89e38d29d3 100644 --- a/test/test_transforms_v2.py +++ b/test/test_transforms_v2.py @@ -145,6 +145,7 @@ class TestSmoke: (transforms.ConvertBoundingBoxFormat(tv_tensors.BoundingBoxFormat.CXCYWH), None), (transforms.ConvertImageDtype(), None), (transforms.GaussianBlur(kernel_size=3), None), + (transforms.GaussianBlur(kernel_size=3, sigma=(0.1, 5)), None), ( transforms.LinearTransformation( # These are just dummy values that will be filled by the adapter. We can't define them upfront, diff --git a/test/test_transforms_v2_refactored.py b/test/test_transforms_v2_refactored.py index 6492aead369..863589599e1 100644 --- a/test/test_transforms_v2_refactored.py +++ b/test/test_transforms_v2_refactored.py @@ -2535,10 +2535,6 @@ def test_kernel_mask(self, make_mask): def test_kernel_video(self): check_kernel(F.crop_video, make_video(self.INPUT_SIZE), **self.MINIMAL_CROP_KWARGS) - @pytest.mark.parametrize( - "make_input", - [make_image_tensor, make_image_pil, make_image, make_bounding_boxes, make_segmentation_mask, make_video], - ) def test_functional(self, make_input): check_functional(F.crop, make_input(self.INPUT_SIZE), **self.MINIMAL_CROP_KWARGS) @@ -2718,3 +2714,17 @@ def test_errors(self): with pytest.raises(ValueError, match="Padding mode should be either"): transforms.RandomCrop([10, 12], padding=1, padding_mode="abc") + + +class TestGaussianBlur: + @pytest.mark.parametrize( + "make_input", + [make_image_tensor, make_image_pil, make_image, make_bounding_boxes, make_segmentation_mask, make_video], + ) + @pytest.mark.parametrize("device", cpu_and_cuda()) + def test_transform(self, make_input, device): + check_transform(transforms.GaussianBlur, make_input(device=device), kernel_size=3, sigma=(0.5, 2.0)) + + def test_input_args(self): + t = transforms.GaussianBlur(3, sigma=(0.5, 2)) + assert t.sigma == [0.5, 2.0] diff --git a/torchvision/transforms/v2/_utils.py b/torchvision/transforms/v2/_utils.py index d5669f5739f..d475055d9ac 100644 --- a/torchvision/transforms/v2/_utils.py +++ b/torchvision/transforms/v2/_utils.py @@ -18,20 +18,23 @@ from torchvision.transforms.v2.functional._utils import _FillType, _FillTypeJIT -def _setup_float_or_seq(arg: Union[float, Sequence[float]], name: str, req_size: int = 2) -> Sequence[float]: +def _setup_float_or_seq(arg: Union[float, Sequence[Union[int, float]]], name: str, req_size: int = 2) -> Sequence[float]: if not isinstance(arg, (float, Sequence)): raise TypeError(f"{name} should be float or a sequence of floats. Got {type(arg)}") if isinstance(arg, Sequence) and len(arg) != req_size: raise ValueError(f"If {name} is a sequence its length should be one of {req_size}. Got {len(arg)}") if isinstance(arg, Sequence): for element in arg: - if not isinstance(element, float): - raise ValueError(f"{name} should be a sequence of floats. Got {type(element)}") + if not isinstance(element, (int, float)): + raise ValueError(f"{name} should be a sequence of ints/floats. Got {type(element)}") if isinstance(arg, float): arg = [float(arg), float(arg)] - if isinstance(arg, (list, tuple)) and len(arg) == 1: - arg = [arg[0], arg[0]] + if isinstance(arg, (list, tuple)): + if len(arg) == 1: + arg = [arg[0], arg[0]] + else: + arg = [float(arg[0]), float(arg[1])] return arg From 0077a7c577c4bcc86b5e264e669f9db7a0e2d2bc Mon Sep 17 00:00:00 2001 From: vfdev Date: Fri, 25 Aug 2023 13:48:03 +0000 Subject: [PATCH 2/7] Updated code and tests --- test/test_transforms_v2.py | 46 +++----------------------- test/test_transforms_v2_refactored.py | 39 +++++++++++++++++++--- torchvision/transforms/v2/_geometry.py | 6 ++-- torchvision/transforms/v2/_misc.py | 17 +++------- torchvision/transforms/v2/_utils.py | 18 +++++----- 5 files changed, 56 insertions(+), 70 deletions(-) diff --git a/test/test_transforms_v2.py b/test/test_transforms_v2.py index d89e38d29d3..8e6bb557348 100644 --- a/test/test_transforms_v2.py +++ b/test/test_transforms_v2.py @@ -145,7 +145,6 @@ class TestSmoke: (transforms.ConvertBoundingBoxFormat(tv_tensors.BoundingBoxFormat.CXCYWH), None), (transforms.ConvertImageDtype(), None), (transforms.GaussianBlur(kernel_size=3), None), - (transforms.GaussianBlur(kernel_size=3, sigma=(0.1, 5)), None), ( transforms.LinearTransformation( # These are just dummy values that will be filled by the adapter. We can't define them upfront, @@ -450,37 +449,6 @@ def test__get_params(self, fill, side_range): assert 0 <= params["padding"][3] <= (side_range[1] - 1) * h -class TestGaussianBlur: - def test_assertions(self): - with pytest.raises(ValueError, match="Kernel size should be a tuple/list of two integers"): - transforms.GaussianBlur([10, 12, 14]) - - with pytest.raises(ValueError, match="Kernel size value should be an odd and positive number"): - transforms.GaussianBlur(4) - - with pytest.raises( - TypeError, match="sigma should be a single int or float or a list/tuple with length 2 floats." - ): - transforms.GaussianBlur(3, sigma=[1, 2, 3]) - - with pytest.raises(ValueError, match="If sigma is a single number, it must be positive"): - transforms.GaussianBlur(3, sigma=-1.0) - - with pytest.raises(ValueError, match="sigma values should be positive and of the form"): - transforms.GaussianBlur(3, sigma=[2.0, 1.0]) - - @pytest.mark.parametrize("sigma", [10.0, [10.0, 12.0]]) - def test__get_params(self, sigma): - transform = transforms.GaussianBlur(3, sigma=sigma) - params = transform._get_params([]) - - if isinstance(sigma, float): - assert params["sigma"][0] == params["sigma"][1] == 10 - else: - assert sigma[0] <= params["sigma"][0] <= sigma[1] - assert sigma[0] <= params["sigma"][1] <= sigma[1] - - class TestRandomPerspective: def test_assertions(self): with pytest.raises(ValueError, match="Argument distortion_scale value should be between 0 and 1"): @@ -504,24 +472,18 @@ def test__get_params(self): class TestElasticTransform: def test_assertions(self): - with pytest.raises(TypeError, match="alpha should be float or a sequence of floats"): + with pytest.raises(TypeError, match="alpha should be a number or a sequence of numbers"): transforms.ElasticTransform({}) - with pytest.raises(ValueError, match="alpha is a sequence its length should be one of 2"): + with pytest.raises(ValueError, match="alpha is a sequence its length should be 1 or 2"): transforms.ElasticTransform([1.0, 2.0, 3.0]) - with pytest.raises(ValueError, match="alpha should be a sequence of floats"): - transforms.ElasticTransform([1, 2]) - - with pytest.raises(TypeError, match="sigma should be float or a sequence of floats"): + with pytest.raises(TypeError, match="sigma should be a number or a sequence of numbers"): transforms.ElasticTransform(1.0, {}) - with pytest.raises(ValueError, match="sigma is a sequence its length should be one of 2"): + with pytest.raises(ValueError, match="sigma is a sequence its length should be 1 or 2"): transforms.ElasticTransform(1.0, [1.0, 2.0, 3.0]) - with pytest.raises(ValueError, match="sigma should be a sequence of floats"): - transforms.ElasticTransform(1.0, [1, 2]) - with pytest.raises(TypeError, match="Got inappropriate fill arg"): transforms.ElasticTransform(1.0, 2.0, fill="abc") diff --git a/test/test_transforms_v2_refactored.py b/test/test_transforms_v2_refactored.py index 863589599e1..901b99d2198 100644 --- a/test/test_transforms_v2_refactored.py +++ b/test/test_transforms_v2_refactored.py @@ -2722,9 +2722,38 @@ class TestGaussianBlur: [make_image_tensor, make_image_pil, make_image, make_bounding_boxes, make_segmentation_mask, make_video], ) @pytest.mark.parametrize("device", cpu_and_cuda()) - def test_transform(self, make_input, device): - check_transform(transforms.GaussianBlur, make_input(device=device), kernel_size=3, sigma=(0.5, 2.0)) + @pytest.mark.parametrize("sigma", [5, (0.5, 2)]) + def test_transform(self, make_input, device, sigma): + check_transform(transforms.GaussianBlur, make_input(device=device), kernel_size=3, sigma=sigma) + + def test_assertions(self): + with pytest.raises(ValueError, match="Kernel size should be a tuple/list of two integers"): + transforms.GaussianBlur([10, 12, 14]) + + with pytest.raises(ValueError, match="Kernel size value should be an odd and positive number"): + transforms.GaussianBlur(4) + + with pytest.raises(ValueError, match="If sigma is a sequence its length should be 1 or 2. Got 3"): + transforms.GaussianBlur(3, sigma=[1, 2, 3]) + + with pytest.raises(ValueError, match="sigma values should be positive and of the form"): + transforms.GaussianBlur(3, sigma=-1.0) - def test_input_args(self): - t = transforms.GaussianBlur(3, sigma=(0.5, 2)) - assert t.sigma == [0.5, 2.0] + with pytest.raises(ValueError, match="sigma values should be positive and of the form"): + transforms.GaussianBlur(3, sigma=[2.0, 1.0]) + + with pytest.raises(TypeError, match="sigma should be a number or a sequence of numbers"): + transforms.GaussianBlur(3, sigma={}) + + @pytest.mark.parametrize("sigma", [10.0, [10.0, 12.0], (10, 12.0), [10]]) + def test__get_params(self, sigma): + transform = transforms.GaussianBlur(3, sigma=sigma) + params = transform._get_params([]) + + if isinstance(sigma, float): + assert params["sigma"][0] == params["sigma"][1] == sigma + elif isinstance(sigma, list) and len(sigma) == 1: + assert params["sigma"][0] == params["sigma"][1] == sigma[0] + else: + assert sigma[0] <= params["sigma"][0] <= sigma[1] + assert sigma[0] <= params["sigma"][1] <= sigma[1] diff --git a/torchvision/transforms/v2/_geometry.py b/torchvision/transforms/v2/_geometry.py index ba3e690dd4d..5bf2867c7fc 100644 --- a/torchvision/transforms/v2/_geometry.py +++ b/torchvision/transforms/v2/_geometry.py @@ -21,7 +21,7 @@ _get_fill, _setup_angle, _setup_fill_arg, - _setup_float_or_seq, + _setup_number_or_seq, _setup_size, get_bounding_boxes, has_all, @@ -1060,8 +1060,8 @@ def __init__( fill: Union[_FillType, Dict[Union[Type, str], _FillType]] = 0, ) -> None: super().__init__() - self.alpha = _setup_float_or_seq(alpha, "alpha", 2) - self.sigma = _setup_float_or_seq(sigma, "sigma", 2) + self.alpha = _setup_number_or_seq(alpha, "alpha", 2) + self.sigma = _setup_number_or_seq(sigma, "sigma", 2) self.interpolation = _check_interpolation(interpolation) self.fill = fill diff --git a/torchvision/transforms/v2/_misc.py b/torchvision/transforms/v2/_misc.py index 739f2fb7ff5..a575d82f3d2 100644 --- a/torchvision/transforms/v2/_misc.py +++ b/torchvision/transforms/v2/_misc.py @@ -9,7 +9,7 @@ from torchvision import transforms as _transforms, tv_tensors from torchvision.transforms.v2 import functional as F, Transform -from ._utils import _parse_labels_getter, _setup_float_or_seq, _setup_size, get_bounding_boxes, has_any, is_pure_tensor +from ._utils import _parse_labels_getter, _setup_number_or_seq, _setup_size, get_bounding_boxes, has_any, is_pure_tensor # TODO: do we want/need to expose this? @@ -198,17 +198,10 @@ def __init__( if ks <= 0 or ks % 2 == 0: raise ValueError("Kernel size value should be an odd and positive number.") - if isinstance(sigma, (int, float)): - if sigma <= 0: - raise ValueError("If sigma is a single number, it must be positive.") - sigma = float(sigma) - elif isinstance(sigma, Sequence) and len(sigma) == 2: - if not 0.0 < sigma[0] <= sigma[1]: - raise ValueError("sigma values should be positive and of the form (min, max).") - else: - raise TypeError("sigma should be a single int or float or a list/tuple with length 2 floats.") - - self.sigma = _setup_float_or_seq(sigma, "sigma", 2) + self.sigma = _setup_number_or_seq(sigma, "sigma", 2) + + if not 0.0 < self.sigma[0] <= self.sigma[1]: + raise ValueError(f"sigma values should be positive and of the form (min, max). Got {self.sigma}") def _get_params(self, flat_inputs: List[Any]) -> Dict[str, Any]: sigma = torch.empty(1).uniform_(self.sigma[0], self.sigma[1]).item() diff --git a/torchvision/transforms/v2/_utils.py b/torchvision/transforms/v2/_utils.py index d475055d9ac..5429d7064bd 100644 --- a/torchvision/transforms/v2/_utils.py +++ b/torchvision/transforms/v2/_utils.py @@ -18,21 +18,23 @@ from torchvision.transforms.v2.functional._utils import _FillType, _FillTypeJIT -def _setup_float_or_seq(arg: Union[float, Sequence[Union[int, float]]], name: str, req_size: int = 2) -> Sequence[float]: - if not isinstance(arg, (float, Sequence)): - raise TypeError(f"{name} should be float or a sequence of floats. Got {type(arg)}") - if isinstance(arg, Sequence) and len(arg) != req_size: - raise ValueError(f"If {name} is a sequence its length should be one of {req_size}. Got {len(arg)}") +def _setup_number_or_seq( + arg: Union[int, float, Sequence[Union[int, float]]], name: str, req_size: int = 2 +) -> Sequence[float]: + if not isinstance(arg, (int, float, Sequence)): + raise TypeError(f"{name} should be a number or a sequence of numbers. Got {type(arg)}") + if isinstance(arg, Sequence) and len(arg) not in (1, req_size): + raise ValueError(f"If {name} is a sequence its length should be 1 or {req_size}. Got {len(arg)}") if isinstance(arg, Sequence): for element in arg: if not isinstance(element, (int, float)): - raise ValueError(f"{name} should be a sequence of ints/floats. Got {type(element)}") + raise ValueError(f"{name} should be a sequence of numbers. Got {type(element)}") - if isinstance(arg, float): + if isinstance(arg, (int, float)): arg = [float(arg), float(arg)] if isinstance(arg, (list, tuple)): if len(arg) == 1: - arg = [arg[0], arg[0]] + arg = [float(arg[0]), float(arg[0])] else: arg = [float(arg[0]), float(arg[1])] return arg From 83fbd3ac7afde49d1254b8b30de11d56898a141e Mon Sep 17 00:00:00 2001 From: vfdev Date: Fri, 25 Aug 2023 15:11:44 +0000 Subject: [PATCH 3/7] fixed code formatting issue --- torchvision/transforms/v2/_misc.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/torchvision/transforms/v2/_misc.py b/torchvision/transforms/v2/_misc.py index a575d82f3d2..3056ec78c51 100644 --- a/torchvision/transforms/v2/_misc.py +++ b/torchvision/transforms/v2/_misc.py @@ -199,7 +199,7 @@ def __init__( raise ValueError("Kernel size value should be an odd and positive number.") self.sigma = _setup_number_or_seq(sigma, "sigma", 2) - + if not 0.0 < self.sigma[0] <= self.sigma[1]: raise ValueError(f"sigma values should be positive and of the form (min, max). Got {self.sigma}") From 6ac3f5f528112081c761d47ae5f0c076cfe2c0a3 Mon Sep 17 00:00:00 2001 From: vfdev Date: Mon, 28 Aug 2023 10:02:35 +0200 Subject: [PATCH 4/7] Update test/test_transforms_v2_refactored.py Co-authored-by: Philip Meier --- test/test_transforms_v2_refactored.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_transforms_v2_refactored.py b/test/test_transforms_v2_refactored.py index 901b99d2198..d9276cd4d76 100644 --- a/test/test_transforms_v2_refactored.py +++ b/test/test_transforms_v2_refactored.py @@ -2724,7 +2724,7 @@ class TestGaussianBlur: @pytest.mark.parametrize("device", cpu_and_cuda()) @pytest.mark.parametrize("sigma", [5, (0.5, 2)]) def test_transform(self, make_input, device, sigma): - check_transform(transforms.GaussianBlur, make_input(device=device), kernel_size=3, sigma=sigma) + check_transform(transforms.GaussianBlur(kernel_size=3, sigma=sigma), make_input(device=device)) def test_assertions(self): with pytest.raises(ValueError, match="Kernel size should be a tuple/list of two integers"): From 6e542346db8552a1daaf55bb22fdfac9bc08909f Mon Sep 17 00:00:00 2001 From: vfdev Date: Wed, 30 Aug 2023 08:59:23 +0000 Subject: [PATCH 5/7] addressed review comments --- torchvision/transforms/v2/_utils.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/torchvision/transforms/v2/_utils.py b/torchvision/transforms/v2/_utils.py index 5429d7064bd..6bc98e6ed7f 100644 --- a/torchvision/transforms/v2/_utils.py +++ b/torchvision/transforms/v2/_utils.py @@ -18,13 +18,11 @@ from torchvision.transforms.v2.functional._utils import _FillType, _FillTypeJIT -def _setup_number_or_seq( - arg: Union[int, float, Sequence[Union[int, float]]], name: str, req_size: int = 2 -) -> Sequence[float]: +def _setup_number_or_seq(arg: Union[int, float, Sequence[Union[int, float]]], name: str) -> Sequence[float]: if not isinstance(arg, (int, float, Sequence)): raise TypeError(f"{name} should be a number or a sequence of numbers. Got {type(arg)}") if isinstance(arg, Sequence) and len(arg) not in (1, req_size): - raise ValueError(f"If {name} is a sequence its length should be 1 or {req_size}. Got {len(arg)}") + raise ValueError(f"If {name} is a sequence its length should be 1 or 2. Got {len(arg)}") if isinstance(arg, Sequence): for element in arg: if not isinstance(element, (int, float)): @@ -32,7 +30,7 @@ def _setup_number_or_seq( if isinstance(arg, (int, float)): arg = [float(arg), float(arg)] - if isinstance(arg, (list, tuple)): + if isinstance(arg, Sequence): if len(arg) == 1: arg = [float(arg[0]), float(arg[0])] else: From ae31288f975146c1eef2fe9c6c6dc49a80a307f8 Mon Sep 17 00:00:00 2001 From: vfdev Date: Wed, 30 Aug 2023 14:42:30 +0000 Subject: [PATCH 6/7] Fixed failing tests --- torchvision/transforms/v2/_geometry.py | 4 ++-- torchvision/transforms/v2/_misc.py | 2 +- torchvision/transforms/v2/_utils.py | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/torchvision/transforms/v2/_geometry.py b/torchvision/transforms/v2/_geometry.py index 5bf2867c7fc..721e9b7e452 100644 --- a/torchvision/transforms/v2/_geometry.py +++ b/torchvision/transforms/v2/_geometry.py @@ -1060,8 +1060,8 @@ def __init__( fill: Union[_FillType, Dict[Union[Type, str], _FillType]] = 0, ) -> None: super().__init__() - self.alpha = _setup_number_or_seq(alpha, "alpha", 2) - self.sigma = _setup_number_or_seq(sigma, "sigma", 2) + self.alpha = _setup_number_or_seq(alpha, "alpha") + self.sigma = _setup_number_or_seq(sigma, "sigma") self.interpolation = _check_interpolation(interpolation) self.fill = fill diff --git a/torchvision/transforms/v2/_misc.py b/torchvision/transforms/v2/_misc.py index 3056ec78c51..67aaf4f3753 100644 --- a/torchvision/transforms/v2/_misc.py +++ b/torchvision/transforms/v2/_misc.py @@ -198,7 +198,7 @@ def __init__( if ks <= 0 or ks % 2 == 0: raise ValueError("Kernel size value should be an odd and positive number.") - self.sigma = _setup_number_or_seq(sigma, "sigma", 2) + self.sigma = _setup_number_or_seq(sigma, "sigma") if not 0.0 < self.sigma[0] <= self.sigma[1]: raise ValueError(f"sigma values should be positive and of the form (min, max). Got {self.sigma}") diff --git a/torchvision/transforms/v2/_utils.py b/torchvision/transforms/v2/_utils.py index 6bc98e6ed7f..6147180a986 100644 --- a/torchvision/transforms/v2/_utils.py +++ b/torchvision/transforms/v2/_utils.py @@ -21,7 +21,7 @@ def _setup_number_or_seq(arg: Union[int, float, Sequence[Union[int, float]]], name: str) -> Sequence[float]: if not isinstance(arg, (int, float, Sequence)): raise TypeError(f"{name} should be a number or a sequence of numbers. Got {type(arg)}") - if isinstance(arg, Sequence) and len(arg) not in (1, req_size): + if isinstance(arg, Sequence) and len(arg) not in (1, 2): raise ValueError(f"If {name} is a sequence its length should be 1 or 2. Got {len(arg)}") if isinstance(arg, Sequence): for element in arg: @@ -30,7 +30,7 @@ def _setup_number_or_seq(arg: Union[int, float, Sequence[Union[int, float]]], na if isinstance(arg, (int, float)): arg = [float(arg), float(arg)] - if isinstance(arg, Sequence): + elif isinstance(arg, Sequence): if len(arg) == 1: arg = [float(arg[0]), float(arg[0])] else: From 2086d2ea3b9b831c6bc46741566da52547c428e6 Mon Sep 17 00:00:00 2001 From: Nicolas Hug Date: Wed, 30 Aug 2023 16:45:53 +0100 Subject: [PATCH 7/7] Fix tests --- test/test_transforms_v2_refactored.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/test_transforms_v2_refactored.py b/test/test_transforms_v2_refactored.py index a3a26ab50b3..b2e21fc4aca 100644 --- a/test/test_transforms_v2_refactored.py +++ b/test/test_transforms_v2_refactored.py @@ -2535,6 +2535,10 @@ def test_kernel_mask(self, make_mask): def test_kernel_video(self): check_kernel(F.crop_video, make_video(self.INPUT_SIZE), **self.MINIMAL_CROP_KWARGS) + @pytest.mark.parametrize( + "make_input", + [make_image_tensor, make_image_pil, make_image, make_bounding_boxes, make_segmentation_mask, make_video], + ) def test_functional(self, make_input): check_functional(F.crop, make_input(self.INPUT_SIZE), **self.MINIMAL_CROP_KWARGS)