Skip to content

[nnc] Strides to Tensor#72962

Closed
IvanKobzarev wants to merge 14 commits intogh/ivankobzarev/114/basefrom
gh/ivankobzarev/114/head
Closed

[nnc] Strides to Tensor#72962
IvanKobzarev wants to merge 14 commits intogh/ivankobzarev/114/basefrom
gh/ivankobzarev/114/head

Conversation

@IvanKobzarev
Copy link
Contributor

@IvanKobzarev IvanKobzarev commented Feb 17, 2022

Stack from ghstack (oldest at bottom):

Differential Revision: D34589306

[ghstack-poisoned]
@pytorch-bot
Copy link

pytorch-bot bot commented Feb 17, 2022

CI Flow Status

⚛️ CI Flow

Ruleset - Version: v1
Ruleset - File: https://github.com/pytorch/pytorch/blob/1b84d6b1d8f7c24053f5bf105ba5d51ac3742b43/.github/generated-ciflow-ruleset.json
PR ciflow labels: ciflow/default
Add ciflow labels to this PR to trigger more builds:

Workflows Labels (bold enabled) Status
Triggered Workflows
linux-binary-conda ciflow/binaries, ciflow/binaries_conda, ciflow/default ✅ triggered
linux-binary-libtorch-cxx11-abi ciflow/binaries, ciflow/binaries_libtorch, ciflow/default ✅ triggered
linux-binary-libtorch-pre-cxx11 ciflow/binaries, ciflow/binaries_libtorch, ciflow/default ✅ triggered
linux-binary-manywheel ciflow/binaries, ciflow/binaries_wheel, ciflow/default ✅ triggered
linux-bionic-py3.7-clang9 ciflow/all, ciflow/cpu, ciflow/default, ciflow/linux, ciflow/noarch, ciflow/trunk ✅ triggered
linux-bionic-rocm4.5-py3.7 ciflow/all, ciflow/default, ciflow/linux, ciflow/rocm, ciflow/trunk ✅ triggered
linux-docs ciflow/all, ciflow/cpu, ciflow/default, ciflow/docs, ciflow/linux, ciflow/trunk ✅ triggered
linux-vulkan-bionic-py3.7-clang9 ciflow/all, ciflow/cpu, ciflow/default, ciflow/linux, ciflow/trunk, ciflow/vulkan ✅ triggered
linux-xenial-cuda11.3-py3.7-gcc7 ciflow/all, ciflow/cuda, ciflow/default, ciflow/linux, ciflow/trunk ✅ triggered
linux-xenial-cuda11.3-py3.7-gcc7-bazel-test ciflow/all, ciflow/bazel, ciflow/cpu, ciflow/default, ciflow/linux, ciflow/trunk ✅ triggered
linux-xenial-py3-clang5-mobile-build ciflow/all, ciflow/default, ciflow/linux, ciflow/mobile, ciflow/trunk ✅ triggered
linux-xenial-py3-clang5-mobile-custom-build-static ciflow/all, ciflow/default, ciflow/linux, ciflow/mobile, ciflow/trunk ✅ triggered
linux-xenial-py3.7-clang7-asan ciflow/all, ciflow/cpu, ciflow/default, ciflow/linux, ciflow/sanitizers, ciflow/trunk ✅ triggered
linux-xenial-py3.7-clang7-onnx ciflow/all, ciflow/cpu, ciflow/default, ciflow/linux, ciflow/onnx, ciflow/trunk ✅ triggered
linux-xenial-py3.7-gcc5.4 ciflow/all, ciflow/cpu, ciflow/default, ciflow/linux, ciflow/trunk ✅ triggered
linux-xenial-py3.7-gcc7 ciflow/all, ciflow/cpu, ciflow/default, ciflow/linux, ciflow/trunk ✅ triggered
linux-xenial-py3.7-gcc7-no-ops ciflow/all, ciflow/cpu, ciflow/default, ciflow/linux, ciflow/trunk ✅ triggered
macos-arm64-binary-conda ciflow/binaries, ciflow/binaries_conda, ciflow/default ✅ triggered
macos-arm64-binary-wheel ciflow/binaries, ciflow/binaries_wheel, ciflow/default ✅ triggered
macos-binary-conda ciflow/binaries, ciflow/binaries_conda, ciflow/default ✅ triggered
macos-binary-libtorch-cxx11-abi ciflow/binaries, ciflow/binaries_libtorch, ciflow/default ✅ triggered
macos-binary-libtorch-pre-cxx11 ciflow/binaries, ciflow/binaries_libtorch, ciflow/default ✅ triggered
macos-binary-wheel ciflow/binaries, ciflow/binaries_wheel, ciflow/default ✅ triggered
pytorch-linux-xenial-py3-clang5-android-ndk-r19c-gradle-custom-build-single ciflow/all, ciflow/android, ciflow/cpu, ciflow/default, ciflow/linux, ciflow/trunk ✅ 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, ciflow/trunk ✅ triggered
win-vs2019-cpu-py3 ciflow/all, ciflow/cpu, ciflow/default, ciflow/trunk, ciflow/win ✅ triggered
win-vs2019-cuda11.3-py3 ciflow/all, ciflow/cuda, ciflow/default, ciflow/trunk, ciflow/win ✅ triggered
windows-binary-libtorch-cxx11-abi ciflow/binaries, ciflow/binaries_libtorch, ciflow/default ✅ triggered
windows-binary-libtorch-pre-cxx11 ciflow/binaries, ciflow/binaries_libtorch, ciflow/default ✅ triggered
windows-binary-wheel ciflow/binaries, ciflow/binaries_wheel, ciflow/default ✅ triggered
Skipped Workflows
caffe2-linux-xenial-py3.7-gcc5.4 ciflow/all, ciflow/cpu, ciflow/linux, ciflow/trunk 🚫 skipped
docker-builds ciflow/all, ciflow/trunk 🚫 skipped
ios-12-5-1-arm64 ciflow/all, ciflow/ios, ciflow/macos, ciflow/trunk 🚫 skipped
ios-12-5-1-arm64-coreml ciflow/all, ciflow/ios, ciflow/macos, ciflow/trunk 🚫 skipped
ios-12-5-1-arm64-custom-ops ciflow/all, ciflow/ios, ciflow/macos, ciflow/trunk 🚫 skipped
ios-12-5-1-arm64-full-jit ciflow/all, ciflow/ios, ciflow/macos, ciflow/trunk 🚫 skipped
ios-12-5-1-arm64-metal ciflow/all, ciflow/ios, ciflow/macos, ciflow/trunk 🚫 skipped
ios-12-5-1-x86-64 ciflow/all, ciflow/ios, ciflow/macos, ciflow/trunk 🚫 skipped
ios-12-5-1-x86-64-coreml ciflow/all, ciflow/ios, ciflow/macos, ciflow/trunk 🚫 skipped
ios-12-5-1-x86-64-full-jit ciflow/all, ciflow/ios, ciflow/macos, ciflow/trunk 🚫 skipped
libtorch-linux-xenial-cuda10.2-py3.7-gcc7 ciflow/all, ciflow/cuda, ciflow/libtorch, ciflow/linux, ciflow/trunk 🚫 skipped
libtorch-linux-xenial-cuda11.3-py3.7-gcc7 ciflow/all, ciflow/cuda, ciflow/libtorch, ciflow/linux, ciflow/trunk 🚫 skipped
linux-bionic-cuda10.2-py3.9-gcc7 ciflow/all, ciflow/cuda, ciflow/linux, ciflow/slow, ciflow/trunk 🚫 skipped
linux-docs-push ciflow/all, ciflow/cpu, ciflow/linux, ciflow/scheduled 🚫 skipped
linux-xenial-cuda11.3-py3.7-gcc7-no-ops ciflow/all, ciflow/cuda, ciflow/linux, ciflow/trunk 🚫 skipped
macos-10-15-py3-arm64 ciflow/all, ciflow/macos, ciflow/trunk 🚫 skipped
macos-10-15-py3-lite-interpreter-x86-64 ciflow/all, ciflow/macos, ciflow/trunk 🚫 skipped
macos-11-py3-x86-64 ciflow/all, ciflow/macos, ciflow/trunk 🚫 skipped
parallelnative-linux-xenial-py3.7-gcc5.4 ciflow/all, ciflow/cpu, ciflow/linux, ciflow/trunk 🚫 skipped
periodic-libtorch-linux-bionic-cuda11.5-py3.7-gcc7 ciflow/all, ciflow/cuda, ciflow/libtorch, ciflow/linux, ciflow/scheduled 🚫 skipped
periodic-libtorch-linux-xenial-cuda11.1-py3.7-gcc7 ciflow/all, ciflow/cuda, ciflow/libtorch, ciflow/linux, ciflow/scheduled 🚫 skipped
periodic-linux-bionic-cuda11.5-py3.7-gcc7 ciflow/all, ciflow/cuda, 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.7-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
periodic-win-vs2019-cuda11.5-py3 ciflow/all, ciflow/cuda, ciflow/scheduled, ciflow/win 🚫 skipped
pytorch-linux-xenial-py3-clang5-android-ndk-r19c-build ciflow/all, ciflow/android, ciflow/cpu, ciflow/linux, ciflow/trunk 🚫 skipped
pytorch-xla-linux-bionic-py3.7-clang8 ciflow/all, ciflow/cpu, ciflow/linux, ciflow/trunk, ciflow/xla 🚫 skipped

@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Feb 17, 2022

🔗 Helpful links

💊 CI failures summary and remediations

As of commit d35ce16 (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.

@facebook-github-bot facebook-github-bot added the oncall: jit Add this issue/PR to JIT oncall triage queue label Feb 17, 2022
IvanKobzarev added a commit that referenced this pull request Feb 17, 2022
ghstack-source-id: 57ec538
Pull Request resolved: #72962
IvanKobzarev added a commit that referenced this pull request Mar 2, 2022
ghstack-source-id: e36b5f2
Pull Request resolved: #72962
@IvanKobzarev
Copy link
Contributor Author

@IvanKobzarev has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

IvanKobzarev added a commit that referenced this pull request Mar 14, 2022
ghstack-source-id: 0ca7c12
Pull Request resolved: #72962
@IvanKobzarev
Copy link
Contributor Author

@IvanKobzarev has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Copy link
Contributor

@navahgar navahgar left a comment

Choose a reason for hiding this comment

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

The changes in the PR look good to me. But I have a more high-level question. What is the usecase we are trying to target here?

@IvanKobzarev
Copy link
Contributor Author

The changes in the PR look good to me. But I have a more high-level question. What is the usecase we are trying to target here?

The reason here is to remove assumptions of output memory format in our lowerings which is based on knowledge of internal implementation of the ops.
E.g. for
add(lhs, rhs):
if (isNHWC(lhs) || is NHWC(rhs)) -> output memory format NHWC else NCHW

But take the output format from the outputStrides of result of tracing graph for shapes (which also traces for strides). In that case for all the lowerings we need to add outputStrides where we had outputShapes.

IvanKobzarev added a commit that referenced this pull request Apr 1, 2022
ghstack-source-id: fb0e59f
Pull Request resolved: #72962
@IvanKobzarev
Copy link
Contributor Author

@IvanKobzarev has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

IvanKobzarev added a commit that referenced this pull request Apr 1, 2022
ghstack-source-id: af009d4
Pull Request resolved: #72962
@IvanKobzarev
Copy link
Contributor Author

@IvanKobzarev has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@IvanKobzarev IvanKobzarev requested a review from navahgar April 1, 2022 20:55
@EikanWang
Copy link
Collaborator

@ZolotukhinM @navahgar this PR is a part of NHWC support.

Copy link
Contributor

@navahgar navahgar left a comment

Choose a reason for hiding this comment

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

Sorry, I missed this earlier.

LGTM. Can you please check the failures?

IvanKobzarev added a commit that referenced this pull request Apr 22, 2022
ghstack-source-id: 2402e49
Pull Request resolved: #72962
@IvanKobzarev
Copy link
Contributor Author

@IvanKobzarev has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@IvanKobzarev
Copy link
Contributor Author

@IvanKobzarev has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

IvanKobzarev added a commit that referenced this pull request Apr 22, 2022
ghstack-source-id: 0f70ecb
Pull Request resolved: #72962
@IvanKobzarev
Copy link
Contributor Author

@IvanKobzarev has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

IvanKobzarev added a commit that referenced this pull request Apr 23, 2022
ghstack-source-id: 524e929
Pull Request resolved: #72962
@IvanKobzarev
Copy link
Contributor Author

@IvanKobzarev has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

1 similar comment
@IvanKobzarev
Copy link
Contributor Author

@IvanKobzarev has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

facebook-github-bot pushed a commit that referenced this pull request Apr 23, 2022
Summary: Pull Request resolved: #72962

Test Plan: Imported from OSS

Reviewed By: ZolotukhinM, cpuhrsch

Differential Revision: D34589306

Pulled By: IvanKobzarev

fbshipit-source-id: ecee5249760ecc0c8b2edb1842b90218899bc944
@github-actions
Copy link
Contributor

Hey @IvanKobzarev.
You've committed this PR, but it does not have both a 'release notes: ...' and 'topics: ...' label. Please add one of each to the PR. The 'release notes: ...' label should represent the part of PyTorch that this PR changes (fx, autograd, distributed, etc) and the 'topics: ...' label should represent the kind of PR it is (not user facing, new feature, bug fix, perf improvement, etc). The list of valid labels can be found here for the 'release notes: ...' and here for the 'topics: ...'.
For changes that are 'topic: not user facing' there is no need for a release notes label.

@datumbox
Copy link
Contributor

This PR is likely to have broken TorchVision. See pytorch/vision#5873 for details.

@vfdev-5
Copy link
Contributor

vfdev-5 commented Apr 25, 2022

Minimal code to reproduce the issue:

import torch
from torchvision.transforms import ColorJitter


meth_kwargs = {"brightness": 0.2, "contrast": 0.2, "saturation": 0.2, "hue": 0.2}

print("-")
channels = 3
device = "cpu"

torch.random.manual_seed(0)
t = ColorJitter(**meth_kwargs)
scripted_fn = torch.jit.script(t)
batch_tensor = torch.randint(0, 256, (4, channels, 23, 34), dtype=torch.uint8, device=device)
torch.manual_seed(12)
scripted_fn(batch_tensor)

print("--")
channels = 1
device = "cpu"

torch.random.manual_seed(0)
t = ColorJitter(**meth_kwargs)
scripted_fn = torch.jit.script(t)
batch_tensor = torch.randint(0, 256, (4, channels, 23, 34), dtype=torch.uint8, device=device)
torch.manual_seed(12)
scripted_fn(batch_tensor)


print("---")
channels = 3
device = "cuda"

torch.random.manual_seed(0)
t = ColorJitter(**meth_kwargs)
scripted_fn = torch.jit.script(t)
batch_tensor = torch.randint(0, 256, (4, channels, 23, 34), dtype=torch.uint8, device=device)
torch.manual_seed(12)
scripted_fn(batch_tensor)


print("----")
channels = 1
device = "cuda"

torch.random.manual_seed(0)
t = ColorJitter(**meth_kwargs)
scripted_fn = torch.jit.script(t)
batch_tensor = torch.randint(0, 256, (4, channels, 23, 34), dtype=torch.uint8, device=device)
torch.manual_seed(12)
scripted_fn(batch_tensor)

Output:

Traceback (most recent call last):
  File "temp/check_torchscript_colorjitter_all_flakiness.py", line 51, in <module>
    scripted_fn(batch_tensor)
  File "/usr/local/lib/python3.8/dist-packages/torch/nn/modules/module.py", line 1129, in _call_impl
    return forward_call(*input, **kwargs)
RuntimeError: The following operation failed in the TorchScript interpreter.
Traceback of TorchScript (most recent call last):
RuntimeError: is_tensor_creation || ((is_contiguous ^ is_channels_last_contiguous) && (is_contiguous || is_channels_last_contiguous)) INTERNAL ASSERT FAILED at "../torch/csrc/jit/tensorexpr/kernel.cpp":519, please report a bug to PyTorch. 

Surprisingly, changing order of test channels : 3,1,3,1 -> 1,3,1,3 and there is no error...

More precisely, it seems to fail on these lines :

https://github.com/pytorch/vision/blob/6d85d74be6ac72b2ac3057d85d8ae0004fa0de3f/torchvision/transforms/functional_tensor.py#L190

https://github.com/pytorch/vision/blob/6d85d74be6ac72b2ac3057d85d8ae0004fa0de3f/torchvision/transforms/functional_tensor.py#L259

@zengk95
Copy link
Contributor

zengk95 commented Apr 25, 2022

@pytorchbot revert this

@IvanKobzarev @datumbox we believe this is causing errors in. torch audio so going to try to revert. If this is wrongfully reverted, please just re open and re land.

@pytorchmergebot
Copy link
Collaborator

Reverting PR 72962 failed due to Can't revert PR that was landed via phabricator as D34589306
Raised by https://github.com/pytorch/pytorch/actions/runs/2221879588

@datumbox
Copy link
Contributor

@zengk95 It is causing errors on Torch Video as well, so I'm fine to have it reverted. I don't think that worked though judging from the bot's message.

zengk95 added a commit that referenced this pull request Apr 25, 2022
pytorchmergebot pushed a commit that referenced this pull request Apr 25, 2022
@IvanKobzarev
Copy link
Contributor Author

@vfdev-5
Thanks for repro steps.

facebook-github-bot pushed a commit that referenced this pull request Apr 26, 2022
Summary:
This reverts commit 9390609.

Fixes pytorch/vision#5873

Pull Request resolved: #76332
Approved by: https://github.com/seemethere

Test Plan: contbuild & OSS CI, see https://hud.pytorch.org/commit/pytorch/pytorch/1d55518198de14a011e7acd6c604afe512ee28b4

Reviewed By: osalpekar

Differential Revision: D35938180

Pulled By: zengk95

fbshipit-source-id: 15f48235b3b11ce9ca2379781f9e75cd1af39aed
@facebook-github-bot facebook-github-bot deleted the gh/ivankobzarev/114/head branch April 27, 2022 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla signed oncall: jit Add this issue/PR to JIT oncall triage queue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants