Fixes F.affine and F.rotate to support rectangular tensor images #2553

vfdev-5 · 2020-08-04T20:39:36Z

Description:

Currently, F.affine does not work correctly on rectangular tensor images due to normalized output of affine_grid.
In this PR there is an attempt to fix the issue. Code is not nice due to dispatch in two part of implementations.

EDIT: Same is for F.rotate.

Added tests on square and rectangular images for F.affine and F.rotate

- updated F.affine tests

…-5/issue-2292-rotate

vfdev-5 · 2020-08-04T20:41:09Z

torchvision/transforms/functional.py


-    matrix = _get_inverse_affine_matrix([0, 0], angle, translate, scale, shear)
+    matrix = _get_inverse_affine_matrix([0, 0], angle, translate_f, scale, shear)


Here, I do not like that depending on if image is square or not matrice's translation part is normalized or not...

Can't we just make everything follow the same code-path?

The problem is that in case of square image we use affine_grid and it requires rescaled translation part, so we compute matrix here with rescaled translate_f. In case of rectangular images, we use custom affine grid implementation _gen_affine_grid where coords normalization is applied a posteriori and we need to deal with matrix where translation part is not normalized. Online normalization,denormalization of matrix is not evident neither. That's why there are two pathes.

can't we just use _gen_affine_grid everywhere?

vfdev-5 · 2020-08-05T14:57:20Z

torchvision/transforms/functional_tensor.py

+    pts = torch.stack([x, y, torch.ones_like(x)], dim=-1)
+    output_grid = torch.matmul(pts, theta.t())
+
+    output_grid = output_grid / torch.tensor([0.5 * w, 0.5 * h])


Here is the principal difference to affine_grid-like implementation. In affine_grid-lik implementation it would be

x = (torch.arange(ow) + d - ow * 0.5) / (0.5 * w) y = (torch.arange(oh) + d - oh * 0.5) / (0.5 * h)

instead of output_grid scaling.

fmassa

Thanks for fixing this!

I have a few questions, let me know what you think

fmassa · 2020-08-05T14:59:26Z

torchvision/transforms/functional_tensor.py

+    if shape[-2] == shape[-1]:
+        # here we need normalized translation part of theta
+        grid = affine_grid(theta, size=(1, shape[-3], shape[-2], shape[-1]), align_corners=False)
+    else:
+        # here we need denormalized translation part of theta
+        grid = _gen_affine_grid(theta[0, :, :], w=shape[-1], h=shape[-2], ow=shape[-1], oh=shape[-2])


Can you replace everything to use _gen_affine_grid, and make a comment on why affine_grid is not suited for this use-case? I even wonder if we shouldn't open an issue in PyTorch about this

I didn't benchmarked both methods to compare the performances, I assumed that affine_grid is better optimized that manual _gen_affine_grid. That's why I've chosen to split here. Do you mean to wrap everything with _gen_affine_grid and split inside this method ?

About an issue in PyTorch, I think pytorch/pytorch#24870 and pytorch/pytorch#36107 already speak about absolute pixel coords. Probably, they are related to this problem.

I mean to just dispatch to _gen_affine_grid, and not use nn.functional.affine_grid. The difference in speed should be very small I think, I'm not sure we dispatch to a cudnn-optimized affine_grid so it would be basically the same operations but called from C++

fmassa · 2020-08-05T15:00:22Z

torchvision/transforms/functional.py


-    matrix = _get_inverse_affine_matrix([0, 0], angle, translate, scale, shear)
+    matrix = _get_inverse_affine_matrix([0, 0], angle, translate_f, scale, shear)


Can't we just make everything follow the same code-path?

- Fixes flake8

…ffine-rect-imgs

- due to bad merge

vfdev-5 · 2020-08-06T12:01:34Z

@fmassa PR is ready to review. What is done:

F_t.affine and F_t.rotate use common method _gen_affine_grid to generate a grid which supports square and rectangular images (due normalization issue with torch native affine_grid). No more routing depending on image shape as it was proposed previously.
Code is tested on square/rect images for F.affine and F.rotate
_gen_affine_grid mimicks affine_grid C++ implementation and optimized such that on CPU it is almost same execution time as for affine_grid.

codecov · 2020-08-06T12:26:03Z

Codecov Report

Merging #2553 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #2553   +/-   ##
=======================================
  Coverage   71.81%   71.81%           
=======================================
  Files          94       94           
  Lines        8079     8080    +1     
  Branches     1283     1282    -1     
=======================================
+ Hits         5802     5803    +1     
  Misses       1868     1868           
  Partials      409      409

Impacted Files	Coverage Δ
torchvision/transforms/functional.py	`80.11% <100.00%> (ø)`
torchvision/transforms/functional_tensor.py	`67.50% <100.00%> (+0.10%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7666252...c862126. Read the comment docs.

fmassa

Looks great, thanks a lot!

…orch#2553) * Added code for F_t.rotate with test - updated F.affine tests * Rotate test tolerance to 2% * Fixes failing test * Optimized _expanded_affine_grid with a single matmul op * Recoded _compute_output_size * [WIP] recoded F_t.rotate internal methods * [WIP] Fixed F.affine to support rectangular images * Recoded _gen_affine_grid to optimized version ~ affine_grid - Fixes flake8 * [WIP] Use _gen_affine_grid for affine and rotate * Fixed tests on square / rectangular images for affine and rotate ops * Removed redefinition of F.rotate - due to bad merge

vfdev-5 added 8 commits July 21, 2020 09:29

Added code for F_t.rotate with test

36fef0d

- updated F.affine tests

Rotate test tolerance to 2%

2b98bdc

Fixes failing test

c7231bd

Merge branch 'master' of https://github.com/pytorch/vision into vfdev…

44da86e

…-5/issue-2292-rotate

Optimized _expanded_affine_grid with a single matmul op

d72cb3d

Recoded _compute_output_size

a249f77

[WIP] recoded F_t.rotate internal methods

a2c6dd1

[WIP] Fixed F.affine to support rectangular images

407e9c4

vfdev-5 requested a review from fmassa August 4, 2020 20:39

vfdev-5 commented Aug 4, 2020

View reviewed changes

vfdev-5 commented Aug 5, 2020

View reviewed changes

fmassa reviewed Aug 5, 2020

View reviewed changes

vfdev-5 added 6 commits August 6, 2020 11:23

Recoded _gen_affine_grid to optimized version ~ affine_grid

4325f67

- Fixes flake8

Merge branch 'master' of https://github.com/pytorch/vision into fix-a…

978c4c0

…ffine-rect-imgs

[WIP] Use _gen_affine_grid for affine and rotate

0e2a3c7

Merge branch 'vfdev-6/issue-2292-rotate' into fix-affine-rect-imgs

6f05c3e

Fixed tests on square / rectangular images for affine and rotate ops

781747c

Removed redefinition of F.rotate

c862126

- due to bad merge

vfdev-5 changed the title ~~[WIP] Fixes F.affine to support rectangular tensor images~~ Fixes F.affine and F.rotate to support rectangular tensor images Aug 6, 2020

vfdev-5 mentioned this pull request Aug 6, 2020

Unified inputs for T.RandomRotation #2496

Merged

vfdev-5 requested a review from fmassa August 6, 2020 12:27

fmassa approved these changes Aug 6, 2020

View reviewed changes

fmassa merged commit 025b71d into pytorch:master Aug 6, 2020

vfdev-5 deleted the fix-affine-rect-imgs branch August 6, 2020 13:01

vfdev-5 mentioned this pull request Aug 7, 2020

Unify Tensor and PIL transforms #2292

Closed

16 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes F.affine and F.rotate to support rectangular tensor images #2553

Fixes F.affine and F.rotate to support rectangular tensor images #2553

vfdev-5 commented Aug 4, 2020 •

edited

vfdev-5 Aug 4, 2020

fmassa Aug 5, 2020

vfdev-5 Aug 5, 2020

fmassa Aug 5, 2020

vfdev-5 Aug 5, 2020

fmassa left a comment

fmassa Aug 5, 2020

vfdev-5 Aug 5, 2020 •

edited

fmassa Aug 5, 2020

fmassa Aug 5, 2020

vfdev-5 commented Aug 6, 2020 •

edited

codecov bot commented Aug 6, 2020 •

edited

fmassa left a comment


		matrix = _get_inverse_affine_matrix([0, 0], angle, translate, scale, shear)
		matrix = _get_inverse_affine_matrix([0, 0], angle, translate_f, scale, shear)

Fixes F.affine and F.rotate to support rectangular tensor images #2553

Fixes F.affine and F.rotate to support rectangular tensor images #2553

Conversation

vfdev-5 commented Aug 4, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fmassa left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vfdev-5 Aug 5, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vfdev-5 commented Aug 6, 2020 • edited

codecov bot commented Aug 6, 2020 • edited

Codecov Report

fmassa left a comment

Choose a reason for hiding this comment

vfdev-5 commented Aug 4, 2020 •

edited

vfdev-5 Aug 5, 2020 •

edited

vfdev-5 commented Aug 6, 2020 •

edited

codecov bot commented Aug 6, 2020 •

edited