[torch.onnx] support `torch.nn.functional.grid_sample` by crcrpar · Pull Request #76159 · pytorch/pytorch

crcrpar · 2022-04-21T01:46:03Z

summary

Adds F.grid_sample support
Adds a test case

Fixes #27212

facebook-github-bot · 2022-04-21T01:46:09Z

🔗 Helpful links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/76159
📄 Preview Python docs built from this PR
📄 Preview C++ docs built from this PR
❓Need help or want to give feedback on the CI? Visit our office hours

💊 CI failures summary and remediations

As of commit 71f8806 (more details on the Dr. CI page):

Expand to see more

💚 💚 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.

BowenBao · 2022-04-26T21:00:25Z

@crcrpar thanks for contributing! Could you add a test case in test/onnx/test_pytorch_onnx_onnxruntime.py for GridSample? I believe onnxruntime has supported this operator since version 1.11, which is what we are running in CI.

You will need to update version to 16 here to enable CI run for opset 16 test with onnxruntime.

pytorch/scripts/onnx/test.sh

Line 83 in ca37477

for i in $(seq 10 15); do

BowenBao · 2022-04-27T18:04:16Z

@crcrpar Looks like CI onnx check is failing on a few tests

FAILED test/onnx/test_pytorch_onnx_onnxruntime.py::TestONNXRuntime_opset16::test_grid_sample
FAILED test/onnx/test_pytorch_onnx_onnxruntime.py::TestONNXRuntime_opset16::test_multi_scale_roi_align
FAILED test/onnx/test_pytorch_onnx_onnxruntime.py::TestONNXRuntime_opset16::test_roi_align
FAILED test/onnx/test_pytorch_onnx_onnxruntime.py::TestONNXRuntime_opset16::test_roi_align_aligned

The roi_align ones look like due to ONNX spec update on opset 16. If you'd like, you can wrap the test case with @skipIfUnsupportedMaxOpsetVersion(16) # TODO: Opset 16 RoiAlign result mismatch..
the grid_sample one is due to issue with passing strings and boolean as model inputs. I'd recommend rewrite it such that the configuration is set at model initialization time.

test/onnx/test_pytorch_onnx_onnxruntime.py

torch/onnx/symbolic_opset16.py

thiagocrepaldi · 2022-04-28T19:03:50Z

Fixes #69674

…nx's GridSample

test/onnx/test_pytorch_onnx_onnxruntime.py

BowenBao · 2022-04-29T18:10:51Z

Thanks @crcrpar ! I left a some comments regarding CI failure and the local test failure you noted. Otherwise this is ready to merge.

test/onnx/test_pytorch_onnx_onnxruntime.py

justinchuby · 2022-04-29T19:25:25Z

test/onnx/test_onnx_opset.py

+        ops = [{"op_name": "GridSample"}]
+        ops = {16: ops}
+
+        class MyModule(Module):


Possible to use a more descriptive name for the module? E.g. GridSampleModule, TestGridSampleModule etc.

test/onnx/test_onnx_opset.py

justinchuby · 2022-04-29T19:28:00Z

test/onnx/test_onnx_opset.py

+            def forward(self, x, grid, mode, padding_mode, align_corers):
+                return torch.nn.functional.grid_sample(x, grid, mode, padding_mode, align_corners)
+
+        for mode, padding_mode, align_corners in itertools.product(


Possible to parameterize this test? E.g.

pytorch/test/distributed/fsdp/test_fsdp_core.py

Lines 141 to 155 in b34b192

@skip_if_lt_x_gpu(2)

@parametrize(params, configs, subtest_name)

@parametrize("clip_norm_type", [2.0, None])

def test_transformer_parameterized(self, cpu_offload, backward_prefetch, sharding_strategy, clip_norm_type):

init_modes = self._get_init_modes_for_test(cpu_offload)

for fsdp_init_mode in init_modes:

with self.subTest(fsdp_init_mode=fsdp_init_mode):

self._test_identical_outputs(

TransformerWithSharedParams,

fsdp_init_mode=fsdp_init_mode,

cpu_offload=cpu_offload,

backward_prefetch=backward_prefetch,

norm_type=clip_norm_type,

sharding_strategy=sharding_strategy,

)

locally parametrize didn't work so skipped it this time

also, I'm not sure if we want a bunch of different test cases for one GridSample op.

That's cool. I wouldn't worry about having different tests for an op, as long as it's clear what they are testing for. For now I would also recommend subtests: https://docs.python.org/3/library/unittest.html#distinguishing-test-iterations-using-subtests

I'm not that inclined to use that as that's not supported by pytest IIRC: https://docs.pytest.org/en/6.2.x/unittest.html#unittest-testcase-support

Do you know if pytest will then skip the subtests or treat them as one test instead? It seems they provide support via a plugin: https://github.com/pytest-dev/pytest-subtests. Feel free to skip any changes in this PR!

IIRC it was more similar to the latter when I used pytest for other tests with subtest.
I haven't checked the code nor sought for a plugin though

test/onnx/test_pytorch_onnx_onnxruntime.py

justinchuby · 2022-04-29T19:36:31Z

torch/onnx/symbolic_opset16.py

+from torch.nn.functional import GRID_SAMPLE_INTERPOLATION_MODES, GRID_SAMPLE_PADDING_MODES
+
+
+# note (mkozuki): Why `grid_sampler` instead of `grid_sample`?


I would add this to the docstring instead.

can't that be a follow-up thing? I didn't see any docstring in symbolic_opset files.

It can be! Totally non blocking

justinchuby · 2022-04-29T19:38:02Z

Commented just as I was passing by! They are non-blocking

HSJ-007 · 2022-04-30T13:44:17Z

Great, when can I use this?

justinchuby · 2022-04-30T15:43:48Z

Great, when can I use this?

Thanks for asking! You should be able to find the updated version in PyTorch nightly once the pull request is merged. You can find installation guides for the nightly build on https://pytorch.org by selecting Preview(Nightly).

HSJ-007 · 2022-04-30T16:47:16Z

Wow, while I don't understand what's in it, I really need this feature

justinchuby · 2022-04-30T16:50:32Z

Wow, while I don't understand what's in it, I really need this feature

@HSJ-007 Could you share more on what you are trying to accomplish and how this feature can be useful?

HSJ-007 · 2022-04-30T17:13:48Z

I'm going to finish a project, and it's tied to my graduation. I used the STN network

justinchuby · 2022-05-02T18:09:33Z

torch/onnx/symbolic_opset16.py

+# note (mkozuki): Why `grid_sampler` instead of `grid_sample`?
+# Because `torch.nn.functional.grid_sample` calls `torch.grid_sampler`.
+@parse_args("v", "v", "i", "i", "b")
+def grid_sampler(g, input, grid, mode_enum, padding_mode_enum, align_corners):


I would annotate the param and return types and include a docstring for the function.

https://google.github.io/styleguide/pyguide.html#383-functions-and-methods

BowenBao

LGTM

BowenBao · 2022-05-02T22:06:31Z

@pytorchbot merge this

Summary: summary - Adds `F.grid_sample` support - Adds a test case Fixes #27212 Pull Request resolved: #76159 Approved by: https://github.com/justinchuby, https://github.com/BowenBao Test Plan: contbuild & OSS CI, see https://hud.pytorch.org/commit/pytorch/pytorch/0ae3aa648e0d997af4d5c092a6ad7a06c8f65798 Reviewed By: malfet Differential Revision: D36101848 Pulled By: malfet fbshipit-source-id: bb271ec0a4728056a58bac469638c5019a6c46c7

crcrpar requested review from BowenBao and shubhambhokare1 as code owners April 21, 2022 01:46

facebook-github-bot added the cla signed label Apr 21, 2022

facebook-github-bot added the oncall: jit Add this issue/PR to JIT oncall triage queue label Apr 21, 2022

pytorchbot added the open source label Apr 21, 2022

albanD added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Apr 25, 2022

BowenBao self-assigned this Apr 26, 2022

BowenBao added module: onnx Related to torch.onnx release notes: onnx torch.onnx related changes that should show up in the release notes topic: improvements topic category labels Apr 26, 2022

thiagocrepaldi mentioned this pull request Apr 27, 2022

Add support for ONNX-only and Caffe2 ONNX export facebookresearch/detectron2#4120

Closed

BowenBao reviewed Apr 28, 2022

View reviewed changes

crcrpar added 13 commits April 29, 2022 04:49

opset16

c4dfd09

gridsample test

cc91573

add opset16

29193bf

fix signature

b59eafb

opset16

1a7b4b9

fix signature

47bcbbe

cosmetic

d124ec1

onnxruntime test

415a90d

skip opset16 for roi align

5856ce5

pass str and bool to dunder init

4b61b6a

style fix

78530b8

Re-re-map mode and padding_mode as we're mapping grid_sampler to on…

2b2950d

…nx's GridSample

Skip locally failing cases

5542914

crcrpar force-pushed the onnx-opset16-gridsample branch from 0a0a42e to 5542914 Compare April 29, 2022 04:55

lintrunner: mypy

e813433

BowenBao reviewed Apr 29, 2022

View reviewed changes

test/onnx/test_pytorch_onnx_onnxruntime.py Outdated Show resolved Hide resolved

justinchuby reviewed Apr 29, 2022

View reviewed changes

crcrpar added 3 commits April 30, 2022 23:20

stop reassigninig

fc97d87

skip opset16 for roi align ops

06f8164

cosmetic & adjust tolerance

71f8806

PINTO0309 mentioned this pull request May 2, 2022

affine grid and grid sample support in onnxruntime microsoft/onnxruntime#10232

Closed

justinchuby reviewed May 2, 2022

View reviewed changes

justinchuby requested a review from BowenBao May 2, 2022 19:29

justinchuby approved these changes May 2, 2022

View reviewed changes

BowenBao approved these changes May 2, 2022

View reviewed changes

pytorchmergebot closed this in 0ae3aa6 May 2, 2022

crcrpar deleted the onnx-opset16-gridsample branch May 2, 2022 23:36

thiagocrepaldi mentioned this pull request May 4, 2022

Unable to export PointRend model (from detectron2) to ONNX #69674

Closed

qqaatw mentioned this pull request Jun 6, 2022

Exporting the operator affine_grid_generator to ONNX opset version 13 is not supported #67320

Closed

MeowZheng mentioned this pull request Jun 7, 2022

pwcnet to onnx open-mmlab/mmflow#148

Closed

	@skip_if_lt_x_gpu(2)
	@parametrize(params, configs, subtest_name)
	@parametrize("clip_norm_type", [2.0, None])
	def test_transformer_parameterized(self, cpu_offload, backward_prefetch, sharding_strategy, clip_norm_type):
	init_modes = self._get_init_modes_for_test(cpu_offload)
	for fsdp_init_mode in init_modes:
	with self.subTest(fsdp_init_mode=fsdp_init_mode):
	self._test_identical_outputs(
	TransformerWithSharedParams,
	fsdp_init_mode=fsdp_init_mode,
	cpu_offload=cpu_offload,
	backward_prefetch=backward_prefetch,
	norm_type=clip_norm_type,
	sharding_strategy=sharding_strategy,
	)

		from torch.nn.functional import GRID_SAMPLE_INTERPOLATION_MODES, GRID_SAMPLE_PADDING_MODES


		# note (mkozuki): Why `grid_sampler` instead of `grid_sample`?

Conversation

crcrpar commented Apr 21, 2022 • edited by garymm Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

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

Uh oh!

🔗 Helpful links

💊 CI failures summary and remediations

Uh oh!

BowenBao commented Apr 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

BowenBao commented Apr 27, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

thiagocrepaldi commented Apr 28, 2022

Uh oh!

Uh oh!

BowenBao commented Apr 29, 2022

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

Uh oh!

Uh oh!

Uh oh!

justinchuby Apr 29, 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!

justinchuby commented Apr 29, 2022

Uh oh!

HSJ-007 commented Apr 30, 2022

Uh oh!

justinchuby commented Apr 30, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

HSJ-007 commented Apr 30, 2022

Uh oh!

justinchuby commented Apr 30, 2022

Uh oh!

HSJ-007 commented Apr 30, 2022

Uh oh!

justinchuby May 2, 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!

BowenBao left a comment

Choose a reason for hiding this comment

Uh oh!

BowenBao commented May 2, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

crcrpar commented Apr 21, 2022 •

edited by garymm

Loading

facebook-github-bot commented Apr 21, 2022 •

edited

Loading

BowenBao commented Apr 26, 2022 •

edited

Loading

justinchuby Apr 29, 2022 •

edited

Loading

justinchuby commented Apr 30, 2022 •

edited

Loading

justinchuby May 2, 2022 •

edited

Loading