[BC-breaking] Fixed interpolation output size to match opencv if scale factor is specified #64983

vfdev-5 · 2021-09-14T10:38:59Z

Fixes #62396

Dscription:

Fixed interpolation output size to match opencv, scikit-image, scipy if scale factor is specified

cc @heitorschueroff , @fmassa

Fixed output size to match opencv, scikit-image, scipy if scale factor is specified

facebook-github-bot · 2021-09-14T10:39:05Z

🔗 Helpful links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/64983
📄 Preview docs built from this PR
📄 Preview C++ docs built from this PR
🔧 Opt-in to CIFlow to control what jobs run on your PRs

💊 CI failures summary and remediations

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

2/2 failures introduced in this PR

🕵️ 2 new failures recognized by patterns

The following CI failures do not appear to be due to upstream breakages:

linux-xenial-py3.6-clang7-onnx / test (default, 1, 2, linux.2xlarge) (1/2)

Step: "Test" (full log | diagnosis details | 🔁 rerun)

2021-11-17T17:26:17.8591472Z AssertionError:

2021-11-17T17:26:17.8579905Z   File "/var/lib/jenkins/workspace/test/onnx/test_pytorch_onnx_onnxruntime.py", line 183, in run_model_test
2021-11-17T17:26:17.8580941Z     ort_compare_with_pytorch(ort_outs, output, rtol, atol)
2021-11-17T17:26:17.8582345Z   File "/var/lib/jenkins/workspace/test/onnx/test_pytorch_onnx_onnxruntime.py", line 126, in ort_compare_with_pytorch
2021-11-17T17:26:17.8583790Z     [np.testing.assert_allclose(out, ort_out, rtol=rtol, atol=atol) for out, ort_out in zip(outputs, ort_outs)]
2021-11-17T17:26:17.8584919Z   File "/var/lib/jenkins/workspace/test/onnx/test_pytorch_onnx_onnxruntime.py", line 126, in <listcomp>
2021-11-17T17:26:17.8586053Z     [np.testing.assert_allclose(out, ort_out, rtol=rtol, atol=atol) for out, ort_out in zip(outputs, ort_outs)]
2021-11-17T17:26:17.8587697Z   File "/opt/conda/lib/python3.6/site-packages/numpy/testing/_private/utils.py", line 1533, in assert_allclose
2021-11-17T17:26:17.8588696Z     verbose=verbose, header=header, equal_nan=equal_nan)
2021-11-17T17:26:17.8589916Z   File "/opt/conda/lib/python3.6/site-packages/numpy/testing/_private/utils.py", line 765, in assert_array_compare
2021-11-17T17:26:17.8590823Z     raise AssertionError(msg)
2021-11-17T17:26:17.8591472Z AssertionError: 
2021-11-17T17:26:17.8592176Z Not equal to tolerance rtol=0.001, atol=1e-06
2021-11-17T17:26:17.8592555Z 
2021-11-17T17:26:17.8592970Z (shapes (1, 2, 14), (1, 2, 13) mismatch)
2021-11-17T17:26:17.8593623Z  x: array([[[ 1.540996,  1.540996,  1.540996, -0.293429, -0.293429,
2021-11-17T17:26:17.8602566Z          -2.178789, -2.178789,  0.568431,  0.568431,  0.568431,
2021-11-17T17:26:17.8603514Z          -1.084522, -1.084522, -1.398595, -1.398595],...
2021-11-17T17:26:17.8603992Z  y: array([[[ 1.540996,  1.540996,  1.540996, -0.293429, -0.293429,
2021-11-17T17:26:17.8604473Z          -2.178789, -2.178789,  0.568431,  0.568431,  0.568431,
2021-11-17T17:26:17.8604909Z          -1.084522, -1.084522, -1.398595],...
2021-11-17T17:26:17.8605108Z

linux-xenial-py3.6-clang7-onnx / test (default, 2, 2, linux.2xlarge) (2/2)

Step: "Test" (full log | diagnosis details | 🔁 rerun)

2021-11-17T17:12:01.9163639Z AssertionError:

2021-11-17T17:12:01.9152755Z   File "/var/lib/jenkins/workspace/test/onnx/test_pytorch_onnx_onnxruntime.py", line 183, in run_model_test
2021-11-17T17:12:01.9153633Z     ort_compare_with_pytorch(ort_outs, output, rtol, atol)
2021-11-17T17:12:01.9154513Z   File "/var/lib/jenkins/workspace/test/onnx/test_pytorch_onnx_onnxruntime.py", line 126, in ort_compare_with_pytorch
2021-11-17T17:12:01.9155685Z     [np.testing.assert_allclose(out, ort_out, rtol=rtol, atol=atol) for out, ort_out in zip(outputs, ort_outs)]
2021-11-17T17:12:01.9157004Z   File "/var/lib/jenkins/workspace/test/onnx/test_pytorch_onnx_onnxruntime.py", line 126, in <listcomp>
2021-11-17T17:12:01.9158012Z     [np.testing.assert_allclose(out, ort_out, rtol=rtol, atol=atol) for out, ort_out in zip(outputs, ort_outs)]
2021-11-17T17:12:01.9159518Z   File "/opt/conda/lib/python3.6/site-packages/numpy/testing/_private/utils.py", line 1533, in assert_allclose
2021-11-17T17:12:01.9160730Z     verbose=verbose, header=header, equal_nan=equal_nan)
2021-11-17T17:12:01.9162135Z   File "/opt/conda/lib/python3.6/site-packages/numpy/testing/_private/utils.py", line 765, in assert_array_compare
2021-11-17T17:12:01.9162988Z     raise AssertionError(msg)
2021-11-17T17:12:01.9163639Z AssertionError: 
2021-11-17T17:12:01.9164684Z Not equal to tolerance rtol=0.001, atol=1e-06
2021-11-17T17:12:01.9165188Z 
2021-11-17T17:12:01.9165494Z (shapes (1, 2, 14), (1, 2, 13) mismatch)
2021-11-17T17:12:01.9166015Z  x: array([[[ 1.540996,  1.540996,  1.540996, -0.293429, -0.293429,
2021-11-17T17:12:01.9166475Z          -2.178789, -2.178789,  0.568431,  0.568431,  0.568431,
2021-11-17T17:12:01.9167020Z          -1.084522, -1.084522, -1.398595, -1.398595],...
2021-11-17T17:12:01.9167821Z  y: array([[[ 1.540996,  1.540996,  1.540996, -0.293429, -0.293429,
2021-11-17T17:12:01.9168438Z          -2.178789, -2.178789,  0.568431,  0.568431,  0.568431,
2021-11-17T17:12:01.9168972Z          -1.084522, -1.084522, -1.398595],...
2021-11-17T17:12:01.9169914Z �[33m=============================== warnings summary ===============================�[0m

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.

fmassa

LGTM, thanks!

This is a BC-breaking change though, so I would love to get @jbschlosser approval on how to move forward with the breaking change.

pytorch-probot · 2021-11-17T16:07:09Z

CI Flow Status

⚛️ CI Flow

Ruleset - Version: v1
Ruleset - File: https://github.com/Quansight/pytorch/blob/bf368474d51a8422ae04c9d4fd8aa1c84853f406/.github/generated-ciflow-ruleset.json
PR ciflow labels: ciflow/default

Workflows	Labels (bold enabled)	Status
Triggered Workflows
linux-bionic-py3.6-clang9	`ciflow/all`, `ciflow/cpu`, `ciflow/default`, `ciflow/linux`, `ciflow/noarch`, `ciflow/xla`	✅ triggered
linux-vulkan-bionic-py3.6-clang9	`ciflow/all`, `ciflow/cpu`, `ciflow/default`, `ciflow/linux`, `ciflow/vulkan`	✅ triggered
linux-xenial-cuda11.3-py3.6-gcc7	`ciflow/all`, `ciflow/cuda`, `ciflow/default`, `ciflow/linux`	✅ triggered
linux-xenial-py3-clang5-mobile-build	`ciflow/all`, `ciflow/default`, `ciflow/linux`, `ciflow/mobile`	✅ triggered
linux-xenial-py3-clang5-mobile-custom-build-static	`ciflow/all`, `ciflow/default`, `ciflow/linux`, `ciflow/mobile`	✅ triggered
linux-xenial-py3.6-clang7-asan	`ciflow/all`, `ciflow/cpu`, `ciflow/default`, `ciflow/linux`, `ciflow/sanitizers`	✅ triggered
linux-xenial-py3.6-clang7-onnx	`ciflow/all`, `ciflow/cpu`, `ciflow/default`, `ciflow/linux`, `ciflow/onnx`	✅ triggered
linux-xenial-py3.6-gcc5.4	`ciflow/all`, `ciflow/cpu`, `ciflow/default`, `ciflow/linux`	✅ triggered
linux-xenial-py3.6-gcc7	`ciflow/all`, `ciflow/cpu`, `ciflow/default`, `ciflow/linux`	✅ triggered
linux-xenial-py3.6-gcc7-bazel-test	`ciflow/all`, `ciflow/bazel`, `ciflow/cpu`, `ciflow/default`, `ciflow/linux`	✅ triggered
pytorch-linux-xenial-py3-clang5-android-ndk-r19c-gradle-custom-build-single	`ciflow/all`, `ciflow/android`, `ciflow/cpu`, `ciflow/default`, `ciflow/linux`	✅ triggered
pytorch-linux-xenial-py3-clang5-android-ndk-r19c-gradle-custom-build-single-full-jit	`ciflow/all`, `ciflow/android`, `ciflow/cpu`, `ciflow/default`, `ciflow/linux`	✅ triggered
win-vs2019-cpu-py3	`ciflow/all`, `ciflow/cpu`, `ciflow/default`, `ciflow/win`	✅ triggered
win-vs2019-cuda11.3-py3	`ciflow/all`, `ciflow/cuda`, `ciflow/default`, `ciflow/win`	✅ triggered
Skipped Workflows
caffe2-linux-xenial-py3.6-gcc5.4	`ciflow/all`, `ciflow/cpu`, `ciflow/linux`	🚫 skipped
docker-builds	`ciflow/all`	🚫 skipped
ios-12-5-1-arm64	`ciflow/all`, `ciflow/ios`, `ciflow/macos`	🚫 skipped
ios-12-5-1-arm64-coreml	`ciflow/all`, `ciflow/ios`, `ciflow/macos`	🚫 skipped
ios-12-5-1-arm64-custom-ops	`ciflow/all`, `ciflow/ios`, `ciflow/macos`	🚫 skipped
ios-12-5-1-arm64-full-jit	`ciflow/all`, `ciflow/ios`, `ciflow/macos`	🚫 skipped
ios-12-5-1-arm64-metal	`ciflow/all`, `ciflow/ios`, `ciflow/macos`	🚫 skipped
ios-12-5-1-x86-64	`ciflow/all`, `ciflow/ios`, `ciflow/macos`	🚫 skipped
ios-12-5-1-x86-64-coreml	`ciflow/all`, `ciflow/ios`, `ciflow/macos`	🚫 skipped
ios-12-5-1-x86-64-full-jit	`ciflow/all`, `ciflow/ios`, `ciflow/macos`	🚫 skipped
libtorch-linux-xenial-cuda10.2-py3.6-gcc7	`ciflow/all`, `ciflow/cuda`, `ciflow/libtorch`, `ciflow/linux`	🚫 skipped
libtorch-linux-xenial-cuda11.3-py3.6-gcc7	`ciflow/all`, `ciflow/cuda`, `ciflow/libtorch`, `ciflow/linux`	🚫 skipped
linux-bionic-cuda10.2-py3.9-gcc7	`ciflow/all`, `ciflow/cuda`, `ciflow/linux`, `ciflow/slow`	🚫 skipped
macos-10-15-py3-arm64	`ciflow/all`, `ciflow/macos`	🚫 skipped
macos-10-15-py3-lite-interpreter-x86-64	`ciflow/all`, `ciflow/macos`	🚫 skipped
macos-10-15-py3-x86-64	`ciflow/all`, `ciflow/macos`	🚫 skipped
parallelnative-linux-xenial-py3.6-gcc5.4	`ciflow/all`, `ciflow/cpu`, `ciflow/linux`	🚫 skipped
periodic-libtorch-linux-xenial-cuda11.1-py3.6-gcc7	`ciflow/all`, `ciflow/cuda`, `ciflow/libtorch`, `ciflow/linux`, `ciflow/scheduled`	🚫 skipped
periodic-linux-xenial-cuda10.2-py3-gcc7-slow-gradcheck	`ciflow/all`, `ciflow/cuda`, `ciflow/linux`, `ciflow/scheduled`, `ciflow/slow`, `ciflow/slow-gradcheck`	🚫 skipped
periodic-linux-xenial-cuda11.1-py3.6-gcc7-debug	`ciflow/all`, `ciflow/cuda`, `ciflow/linux`, `ciflow/scheduled`	🚫 skipped
periodic-win-vs2019-cuda11.1-py3	`ciflow/all`, `ciflow/cuda`, `ciflow/scheduled`, `ciflow/win`	🚫 skipped

You can add a comment to the PR and tag @pytorchbot with the following commands:

# ciflow rerun, "ciflow/default" will always be added automatically
@pytorchbot ciflow rerun

# ciflow rerun with additional labels "-l <ciflow/label_name>", which is equivalent to adding these labels manually and trigger the rerun
@pytorchbot ciflow rerun -l ciflow/scheduled -l ciflow/slow

For more information, please take a look at the CI Flow Wiki.

vfdev-5 · 2021-11-17T16:46:50Z

@jbschlosser could you please take a look at this PR which should fix output size issue by torch.nn.functional.interpolation vs opencv, scikit-image.

jbschlosser

@fmassa do you have a sense for how disruptive this change would be if we made it outright? The breakage seems less common and less silent than fixing the off-by-one interpolate content error, and it's still possible to get the old output by slicing (e.g. out[:,:,:-1,:-1]) if necessary.

That said, the conservative route I'd default to involves the painful process of adding another flag to switch between old and new behavior (idk a good name, round_with_scale_factor?), the JIT FC workaround for adding the new kwarg, etc. If the breakage isn't too bad, I'd like to avoid this messy situation, but it's hard for me to say.

vfdev-5 · 2022-01-26T16:37:10Z

The breakage seems less common and less silent than fixing the off-by-one interpolate content error, and it's still possible to get the old output by slicing (e.g. out[:,:,:-1,:-1]) if necessary.

@jbschlosser I think this can be a bit more trickier, for example #70559 where slicing does not give the previous output.

jbschlosser · 2022-01-31T20:26:02Z

@jbschlosser I think this can be a bit more trickier, for example #70559 where slicing does not give the previous output.

Good point and thanks for bringing it up - I think we'll have to go the long and painful route. I did a quick github code search and it does look like the behavior will be disruptive for certain use cases, especially for things like U-Nets which have a lot of scale_factor usage.

I'll put forth the name of round_with_scale_factor for the new kwarg, but other suggestions are welcome. Suggested deprecation process, following that done for interpolate previously:

Introduce new kwarg (e.g. round_with_scale_factor=None) for a release that acts disabled by default. Users can explicitly opt-in to the fix. If no explicit value is specified for this flag AND the resultant shape will be calculated differently, warn that the default behavior will change in a future release. Users can explicitly set a value for the kwarg to acknowledge and turn off the warning.
Change the default behavior for the kwarg to be enabled and remove the warning. At this point, users have had a full release to explicitly choose between rounding and no rounding. The bug fix will be applied by default, and anyone who knows they do not want rounding for some reason can explicitly disable it.

@albanD / @mruberry wdyt about the above?

albanD · 2022-02-01T20:30:57Z

Sounds good to me!

github-actions · 2022-05-21T20:37:09Z

Looks like this PR hasn't been updated in a while so we're going to go ahead and mark this as Stale.
Feel free to remove the Stale label if you feel this was a mistake.
If you are unable to remove the Stale label please contact a maintainer in order to do so.
If you want the bot to never mark this PR stale again, add the no-stale label.
Stale pull requests will automatically be closed after 30 days of inactivity.

vfdev-5 · 2022-05-22T10:11:08Z

Sorry for delay on this PR. I still plan to follow #64983 (comment) and update the PR. I remove "Stale" label.

github-actions · 2022-07-21T10:39:05Z

Looks like this PR hasn't been updated in a while so we're going to go ahead and mark this as Stale.
Feel free to remove the Stale label if you feel this was a mistake.
If you are unable to remove the Stale label please contact a maintainer in order to do so.
If you want the bot to never mark this PR stale again, add the no-stale label.
Stale pull requests will automatically be closed after 30 days of inactivity.

NicolasHug · 2023-03-29T12:15:14Z

Just chiming in as @vfdev-5 asked me to take a look at this. The strategy described in #64983 (comment) sounds like the least disruptive way forward. Perhaps with a small amendment: according to https://github.com/pytorch/pytorch/wiki/PyTorch's-Python-Frontend-Backward-and-Forward-Compatibility-Policy "stage 1" should probably last 2 versions instead of 1.

Addresses #62396 following the strategy described in #64983 (comment). Fixing output size to match opencv, scikit-image, scipy if scale factor is specified on ATen side only due to JIT FC. cc jgong5 mingfeima XiaobingSuper sanchitintel ashokei jingxu10 EikanWang [ghstack-poisoned]

Addresses #62396 following the strategy described in #64983 (comment). Fixing output size to match opencv, scikit-image, scipy if scale factor is specified on ATen side only due to JIT FC. Pull Request resolved: #97868 Approved by: https://github.com/lezcano, https://github.com/mikaylagawarecki

Fixes pytorch#62396

933d9eb

Fixed output size to match opencv, scikit-image, scipy if scale factor is specified

vfdev-5 requested review from albanD and jbschlosser as code owners September 14, 2021 10:39

facebook-github-bot added the cla signed label Sep 14, 2021

pytorchbot added the open source label Sep 14, 2021

jbschlosser requested review from fmassa and heitorschueroff September 14, 2021 13:08

jbschlosser added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Sep 14, 2021

heitorschueroff removed their request for review September 14, 2021 17:55

fmassa approved these changes Sep 15, 2021

View reviewed changes

vfdev-5 changed the title ~~Fixed interpolation output size to match opencv if scale factor is specified~~ [BC-breaking] Fixed interpolation output size to match opencv if scale factor is specified Sep 15, 2021

Merge branch 'master' into vfdev-5/fix-62396-interp-outsize

ce7973a

pytorch-probot bot added the ciflow/default label Nov 17, 2021

Enabled commented tests

bf36847

jbschlosser reviewed Jan 25, 2022

View reviewed changes

vfdev-5 mentioned this pull request Jan 26, 2022

Behavior of torch.nn.functional.interpolate with unchanged output size and recompute_scale_factor=False #70559

Open

suo removed the ciflow/default label Mar 22, 2022

github-actions bot added the Stale label May 21, 2022

vfdev-5 removed the Stale label May 22, 2022

vfdev-5 marked this pull request as draft May 22, 2022 10:11

github-actions bot added the Stale label Jul 21, 2022

github-actions bot closed this Aug 20, 2022

vfdev-5 mentioned this pull request Mar 30, 2023

Added round_with_scale_factor arg to ATen #97868

Closed

mikaylagawarecki mentioned this pull request Mar 30, 2023

Added round_with_scale_factor to python and tests #97869

Closed

mikaylagawarecki mentioned this pull request Apr 28, 2023

Enable fix for UpsampleNearest functions in Pytorch pytorch/xla#4959

Closed

[BC-breaking] Fixed interpolation output size to match opencv if scale factor is specified #64983

[BC-breaking] Fixed interpolation output size to match opencv if scale factor is specified #64983

Uh oh!

Conversation

vfdev-5 commented Sep 14, 2021

Uh oh!

facebook-github-bot commented Sep 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful links

💊 CI failures summary and remediations

🕵️ 2 new failures recognized by patterns

linux-xenial-py3.6-clang7-onnx / test (default, 1, 2, linux.2xlarge) (1/2)

linux-xenial-py3.6-clang7-onnx / test (default, 2, 2, linux.2xlarge) (2/2)

Uh oh!

fmassa left a comment

Choose a reason for hiding this comment

Uh oh!

pytorch-probot bot commented Nov 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚛️ CI Flow

Uh oh!

vfdev-5 commented Nov 17, 2021

Uh oh!

jbschlosser left a comment

Choose a reason for hiding this comment

Uh oh!

vfdev-5 commented Jan 26, 2022

Uh oh!

jbschlosser commented Jan 31, 2022

Uh oh!

albanD commented Feb 1, 2022

Uh oh!

github-actions bot commented May 21, 2022

Uh oh!

vfdev-5 commented May 22, 2022

Uh oh!

github-actions bot commented Jul 21, 2022

Uh oh!

NicolasHug commented Mar 29, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

facebook-github-bot commented Sep 14, 2021 •

edited

Loading

pytorch-probot bot commented Nov 17, 2021 •

edited

Loading