Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cleanup affine grid image kernels #8004

Merged
merged 3 commits into from
Oct 2, 2023
Merged

Conversation

pmeier
Copy link
Collaborator

@pmeier pmeier commented Sep 29, 2023

Factoring out the changes to _apply_grid_transform from #7979. This pulls the squashing / unsquashing logic as well as the image.numel() == 0 handling into _apply_grid_transform to avoid repeating it in every kernel that uses it.

cc @vfdev-5

@pytorch-bot
Copy link

pytorch-bot bot commented Sep 29, 2023

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/vision/8004

Note: Links to docs will display an error until the docs builds have been completed.

❌ 12 New Failures, 10 Unrelated Failures

As of commit e9cad2d with merge base f96deba (image):

NEW FAILURES - The following jobs have failed:

FLAKY - The following jobs failed but were likely due to flakiness present on trunk:

BROKEN TRUNK - The following job failed but were present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

This comment was automatically generated by Dr. CI and updates every 15 minutes.

Copy link
Member

@NicolasHug NicolasHug left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM in principle, although I haven't double checked that all the new logic is strictly equivalent (hopefully it would be caught by the test). One Q below, but non-blocking as long as it's adressed / or if it's a non-issue


return output
grid = _create_identity_grid((height, width), device=device, dtype=dtype).add_(
displacement.to(dtype=dtype, device=device)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it equivalent to cast to float16 before the call to _apply_grid_transform? It used to be done after.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also why is to(device) needed now?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The float16 handling above is untouched and deals with the image. Here we are just aligning the displacement with the grid. This is the same as

if displacement.dtype != dtype or displacement.device != device:
displacement = displacement.to(dtype=dtype, device=device)

but relying on the noop behavior of .to() to avoid the check on our side.

@pmeier pmeier merged commit ee28bb3 into pytorch:main Oct 2, 2023
40 of 62 checks passed
@pmeier pmeier deleted the affine-grid-squash branch October 2, 2023 18:30
facebook-github-bot pushed a commit that referenced this pull request Oct 31, 2023
Reviewed By: vmoens

Differential Revision: D50789094

fbshipit-source-id: 08418d28f3c67527a58fbb2dbc32fc1490213a00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants