[EXPERIMENTAL] Extending padding to support non-constant fill #5568

datumbox · 2022-03-08T19:34:39Z

PyTorch's pad supports only constant values. Unfortunately this can be a problem for data augmentation techniques that require padding with a specific fill colour. For some of them we have previously employed the following trick:

vision/references/detection/transforms.py

Lines 204 to 209 in 79892d3

    
           if isinstance(image, torch.Tensor): 
        
               # PyTorch's pad supports only integers on fill. So we need to overwrite the colour 
        
               v = torch.tensor(self.fill, device=image.device, dtype=image.dtype).view(-1, 1, 1) 
        
               image[..., :top, :] = image[..., :, :left] = image[..., (top + orig_h) :, :] = image[ 
        
                   ..., :, (left + orig_w) : 
        
               ] = v

This PR adapts the approach and moves it to F.pad(). The fill can be either a float or a List[float]. Unfortunately JIT doesn't allow us to include also int and List[int]. The PR modifies the default values of some of the methods.

facebook-github-bot · 2022-03-08T19:34:48Z

💊 CI failures summary and remediations

As of commit 05384cf (more details on the Dr. CI page):

💚 💚 Looks good so far! There are no failures yet. 💚 💚

This comment was automatically generated by Dr. CI (expand for details).

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

datumbox

Providing comments to assist review

datumbox · 2022-03-08T19:36:14Z

test/test_functional_tensor.py

-        {"padding_mode": "constant", "fill": 20},
+        {"padding_mode": "constant", "fill": 10.0},
+        {"padding_mode": "constant", "fill": [10.0, 10.0, 10.0]},
+        {"padding_mode": "constant", "fill": [10.0, 0.0, 10.0]},


The test won't pass if we provide integers. That's because the test conducts JIT-script checks as well.

Here we check for single values, lists with the same value and lists with different values.

datumbox · 2022-03-08T19:37:45Z

torchvision/transforms/functional.py


-def pad(img: Tensor, padding: List[int], fill: int = 0, padding_mode: str = "constant") -> Tensor:
+def pad(
+    img: Tensor, padding: List[int], fill: Union[List[float], float] = 0.0, padding_mode: str = "constant"


The default value changed from int to float. JIT will fail if we pass integers.

Where exactly we need float values ? Maybe we could keep ints and List[int] and cast to float where it is required ?

The int used previously is very misleading. We mainly use floats because our tensors get rescaled as you know. Unfortunately adding both List[int] and List[float] in the union doesn't work due to JIT issues. See pytorch/pytorch#69434

datumbox · 2022-03-08T19:38:07Z

torchvision/transforms/functional.py

            (crop_height - image_height + 1) // 2 if crop_height > image_height else 0,
        ]
-        img = pad(img, padding_ltrb, fill=0)  # PIL uses fill value 0
+        img = pad(img, padding_ltrb, fill=0.0)  # PIL uses fill value 0


Again we need floats to appease JIT.

datumbox · 2022-03-08T19:38:41Z

torchvision/transforms/functional_pil.py

    if not isinstance(padding, (numbers.Number, tuple, list)):
        raise TypeError("Got inappropriate padding arg")
-    if not isinstance(fill, (numbers.Number, str, tuple)):
+    if not isinstance(fill, (numbers.Number, str, list, tuple)):


Unrelated bug fix on the original code.

If there is a bug fix here and we do not expect this to land, maybe better to split this into a separate PR ?

Yeah we can cherrypick afterwards if we don't land this.

datumbox · 2022-03-08T19:39:55Z

torchvision/transforms/functional_pil.py

+        if isinstance(fill, (list, tuple)):
+            fill = tuple(int(x) for x in fill)
+        else:
+            fill = int(fill)


Unrelated bug fix on the original code. This method doesn't work if floats are provided for PIL images, despite the method having floats in the signature.

Same here ?

datumbox · 2022-03-08T19:40:31Z

torchvision/transforms/functional_tensor.py

    if left < 0 or top < 0 or right > w or bottom > h:
        padding_ltrb = [max(-left, 0), max(-top, 0), max(right - w, 0), max(bottom - h, 0)]
-        return pad(img[..., max(top, 0) : bottom, max(left, 0) : right], padding_ltrb, fill=0)
+        return pad(img[..., max(top, 0) : bottom, max(left, 0) : right], padding_ltrb, fill=0.0)


Floats to please JIT

datumbox · 2022-03-08T19:42:06Z

torchvision/transforms/functional_tensor.py

+        if not isinstance(fill, (tuple, list)):
+            fill = [fill]
+        fill_img = torch.tensor(fill, dtype=img.dtype, device=img.device).view(1, -1, 1, 1)
+        if pad_top > 0:


Handling negative padding values.

datumbox · 2022-03-08T19:42:34Z

torchvision/transforms/functional_tensor.py

-            "channels of the image ({} != {})"
-        )
-        raise ValueError(msg.format(len(fill), num_channels))
+    _assert_fill(fill, num_channels)


Just move out the code to reuse it above.

datumbox · 2022-03-08T19:43:13Z

torchvision/transforms/transforms.py

            raise TypeError("Got inappropriate padding arg")

-        if not isinstance(fill, (numbers.Number, str, tuple)):
+        if not isinstance(fill, (numbers.Number, str, tuple, list)):


Unrelated bug fix on the original code.

datumbox · 2022-03-08T19:43:33Z

torchvision/transforms/transforms.py

        return i, j, th, tw

-    def __init__(self, size, padding=None, pad_if_needed=False, fill=0, padding_mode="constant"):
+    def __init__(self, size, padding=None, pad_if_needed=False, fill=0.0, padding_mode="constant"):


Floats to please JIT

datumbox · 2022-03-08T20:29:45Z

The previous solution failed. Seems we might have a gap on our JIT scripts cause it was caught by the doc scripts.

vfdev-5 · 2022-03-09T14:43:05Z

torchvision/transforms/functional_tensor.py

+    if padding_mode == "constant":
+        # The following if/else can't be simplified due to JIT
+        if isinstance(fill, (tuple, list)):
+            fill_img = torch.tensor(fill).to(dtype=img.dtype, device=img.device).view(1, -1, 1, 1)


nit: can't we create it directly as

fill_img = torch.tensor(fill, dtype=img.dtype, device=img.device)

?

No. :( JIT requires it to be behind an if statement. I believe this is because it invokes a different C++ method (The one that receives a list VS a scalar).

I mean avoid to call .to:

- fill_img = torch.tensor(fill).to(dtype=img.dtype, device=img.device).view(1, -1, 1, 1) + fill_img = torch.tensor(fill, dtype=img.dtype, device=img.device).view(1, -1, 1, 1)

Sorry I missed that. That's also needed for the scalar case. I believe some of the tests were failing due to fill being float and dtype being integer. Casting solves this.

BTW you are welcome to push to the branch if you want to experiment.

@ansley FYI this is the kind of weird code one must write to make things JIT-scriptable. Without the explicit if statement, JIT doesn't know how to handle fill when scalar vs when list. I believe this has to do with the fact that the C++ implementation ends up calling a different method.

vfdev-5 · 2022-03-09T14:47:51Z

torchvision/transforms/functional_tensor.py

        img = img.to(torch.float32)

-    img = torch_pad(img, p, mode=padding_mode, value=float(fill))
+    img = torch_pad(img, p, mode=padding_mode)


if fill is a scalar now we still transform it to a tensor and apply to the image at most 4 times below (img[..., :, :pad_left] = fill_img). Maybe, for performance reasons we could do if/else here and keep previous behaviour with a single torch_pad call for scalars and for list/tuple do what you coded ?

I had that, see earlier versions of the commit. Unfortunately I couldn't find a way to write it in a JIT-friendly way. See here for more details. If you have ideas on how to have the optimization and be JIT-scriptable I'm happy to use them :)

datumbox · 2022-03-10T12:02:03Z

This solution is not good enough. Though the JIT tests pass, there are issues:

This PR leaves the code in a worse state. It contains a few "voodoo" parts which are non-obvious and exist only to please JIT.
The end result is very brittle. Making small changes can instantly break the code.
It's not efficient. JIT doesn't let us apply the patch only when necessary.

For the above reasons, I will close the PR. Perhaps we can revisit this on the future when issue pytorch/pytorch#69434 is addressed. For now I think that applying the right fill for padding can be addressed on the side of new Class Transforms where things don't have to be JIT-scriptable.

datumbox added 3 commits March 8, 2022 18:03

Add support of non-scalar float values on fill.

ccef4e4

Fix bugs

d427cbe

Fixing linter

49c6c1a

datumbox added enhancement module: transforms labels Mar 8, 2022

datumbox requested review from vfdev-5 and fmassa March 8, 2022 19:34

pytorch-bot bot added the ciflow/default label Mar 8, 2022

facebook-github-bot added the cla signed label Mar 8, 2022

datumbox changed the title ~~Extending padding to support non-constant fill~~ [EXPERIMENTAL] Extending padding to support non-constant fill Mar 8, 2022

datumbox commented Mar 8, 2022

View reviewed changes

datumbox force-pushed the transforms/pad_fill branch from 2ff188a to 8625bd4 Compare March 8, 2022 20:29

Further JIT patches to fix failures

9589e59

datumbox force-pushed the transforms/pad_fill branch from 8625bd4 to 9589e59 Compare March 8, 2022 20:30

Converting more values to floats

6d1b41f

datumbox mentioned this pull request Mar 8, 2022

JIT RuntimeError: 'Union[Tensor, List[float], List[int]]' object is not subscriptable pytorch/pytorch#69434

Open

Workaround for JIT.

05384cf

pmeier mentioned this pull request Mar 9, 2022

port RandomZoomOut from detection references to prototype transforms #5551

Merged

datumbox mentioned this pull request Mar 9, 2022

Allow F.normalize function to use float and list of float as mean and… #5569

Closed

vfdev-5 reviewed Mar 9, 2022

View reviewed changes

datumbox closed this Mar 10, 2022

datumbox deleted the transforms/pad_fill branch March 10, 2022 12:02

This was referenced Mar 11, 2022

Post-paper Detection Optimizations #5444

Merged

Better handling for Pad's fill argument #5596

Merged

	if isinstance(image, torch.Tensor):
	# PyTorch's pad supports only integers on fill. So we need to overwrite the colour
	v = torch.tensor(self.fill, device=image.device, dtype=image.dtype).view(-1, 1, 1)
	image[..., :top, :] = image[..., :, :left] = image[..., (top + orig_h) :, :] = image[
	..., :, (left + orig_w) :
	] = v

[EXPERIMENTAL] Extending padding to support non-constant fill #5568

[EXPERIMENTAL] Extending padding to support non-constant fill #5568

Uh oh!

Conversation

datumbox commented Mar 8, 2022

Uh oh!

facebook-github-bot commented Mar 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💊 CI failures summary and remediations

Uh oh!

datumbox left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

datumbox commented Mar 8, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vfdev-5 Mar 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

datumbox commented Mar 10, 2022

Uh oh!

Uh oh!

facebook-github-bot commented Mar 8, 2022 •

edited

Loading

vfdev-5 Mar 9, 2022 •

edited

Loading