8000 Port `index.Tensor` to structured kernels. by ysiraichi · Pull Request #69607 · pytorch/pytorch · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Port index.Tensor to structured kernels. #69607

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

Closed
wants to merge 65 commits into from

Conversation

ysiraichi
Copy link
Collaborator
@ysiraichi ysiraichi commented Dec 8, 2021

Stack from ghstack (oldest at bottom):

Tracking issue: #55070

Tracking issue: #55070

[ghstack-poisoned]
@pytorch-probot
Copy link
pytorch-probot bot commented Dec 8, 2021
CI Flow Status

⚛️ CI Flow

Ruleset - Version: v1
Ruleset - File: https://github.com/pytorch/pytorch/blob/59499170ad9fe4c8aaedfc31967b1f2f5435c8a7/.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-docs ciflow/all, ciflow/cpu, ciflow/default, ciflow/docs, ciflow/linux ✅ 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
linux-docs-push ciflow/all, ciflow/cpu, ciflow/linux, ciflow/scheduled 🚫 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-11-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-bionic-cuda11.5-py3.6-gcc7 ciflow/all, ciflow/cuda, ciflow/libtorch, ciflow/linux, ciflow/scheduled 🚫 skipped
periodic-libtorch-linux-xenial-cuda11.1-py3.6-gcc7 ciflow/all, ciflow/cuda, ciflow/libtorch, ciflow/linux, ciflow/scheduled 🚫 skipped
periodic-linux-bionic-cuda11.5-py3.6-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.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.

@facebook-github-bot
Copy link
Contributor
facebook-github-bot commented Dec 8, 2021

🔗 Helpful links

✅ No Failures (0 Pending)

As of commit 374e448 (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 Dec 8, 2021
@ysiraichi ysiraichi added the module: structured kernels Related to new structured kernels functionality label Dec 8, 2021
ysiraichi added a commit that referenced this pull request Dec 8, 2021
Tracking issue: #55070

ghstack-source-id: 7b1a144
Pull Request resolved: #69607
index_stub(iter.device_type(), iter, info.indexed_sizes, info.indexed_strides);
at::Tensor res = iter.output();

auto result = at::index(self_dq, indices);
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure how important the extra overhead incurred from the dispatcher trip is here. We already know that we're in a QuantizedCPU kernel though, so maybe it's not too unreasonable to call at::cpu::index? If we do, we should rename the function quantized_index -> quantized_cpu_index to avoid confusion.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah, you are right. That sounds way more reasonable.

@bdhirsh
Copy link
Contributor
bdhirsh commented Dec 10, 2021

I see a few CI failures. They look like they're mostly coming from unexpected successes in OpInfo tests though.

DimVector sizes,
DimVector strides,
const Tensor& result) {
index_stub(device_type(), *this, sizes, strides);
}

Tensor quantized_index(const Tensor & self, const torch::List<c10::optional<Tensor>>& indices) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Also, totally not needed but we could swap out the torch::List here with iOptTensorRefList while we're at it, to give unboxing quantized calls a small perf boost.

@@ -57,6 +57,7 @@ def __str__(self) -> str:
iOptTensorRefListT = BaseCppType('at', 'IOptTensorRefList')
dimnameT = BaseCppType('at', 'Dimname')
dimnameListT = BaseCppType('at', 'DimnameList')
dimVectorT = BaseCppType('at', 'DimVector')
Copy link
Contributor

Choose a reason for hiding this comment

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

this seems reasonable

@bdhirsh
Copy link
Contributor
bdhirsh commented Dec 10, 2021

This looks good :). I'll approve when CI looks good

bdhirsh added a commit that referenced this pull request Jun 9, 2022
Tracking issue: #55070

ghstack-source-id: 779070f
Pull Request resolved: #69607
@bdhirsh
Copy link
Contributor
bdhirsh commented Jun 10, 2022

@pytorchbot merge -g

@pytorchmergebot
Copy link
Collaborator

@pytorchbot successfully started a merge job. Check the current status here

@bdhirsh
Copy link
Contributor
bdhirsh commented Jun 10, 2022

This diff requires a hot fix in fbcode, but I wasn't able to import it. Hot fix is here, I'll rebase it on top of the internal diff once it enters the diff train: D37015324

@pytorchmergebot
Copy link
Collaborator

Merge failed due to 1 additional jobs have failed, first few of them are: Meta Internal-Only Changes Check
Raised by https://github.com/pytorch/pytorch/actions/runs/2472179147

bdhirsh added a commit that referenced this pull request Jun 10, 2022
Tracking issue: #55070

ghstack-source-id: 6ef8327
Pull Request resolved: #69607
@bdhirsh
Copy link
Contributor
bdhirsh commented Jun 10, 2022

@pytorchbot merge -g

@pytorchmergebot
Copy link
Collaborator

@pytorchbot successfully started a merge job. Check the current status here

@bdhirsh
Copy link
Contributor
bdhirsh commented Jun 10, 2022

Before the last rebase, all of CI was green except for the "meta internal-only changes check" (which I believe is safe to ignore because this PR was actually created before the GH1 workflow, and I'm planning to abandon the internal diff).

Planning to force merge to unblock XLA CI.

@bdhirsh
Copy link
Contributor
bdhirsh commented Jun 10, 2022

@pytorchbot merge -f

@pytorchmergebot
Copy link
Collaborator

@pytorchbot successfully started a merge job. Check the current status here

@github-actions
Copy link
Contributor

Hey @ysiraichi.
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.

@pytorchmergebot
Copy link
F438 Collaborator

Merge failed due to Command git -C /home/runner/actions-runner/_work/pytorch/pytorch cherry-pick -x 86c34fae86cf5f2d855ba1acf7fbe173cdc60651 returned non-zero exit code 1

The previous cherry-pick is now empty, possibly due to conflict resolution.
If you wish to commit it anyway, use:

    git commit --allow-empty

Otherwise, please use 'git cherry-pick --skip'
On branch master
Your branch is up to date with 'origin/master'.

You are currently cherry-picking commit 86c34fae86.
  (all conflicts fixed: run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

nothing to commit, working tree clean

Raised by https://github.com/pytorch/pytorch/actions/runs/2476518424

@bdhirsh
Copy link
Contributor
bdhirsh commented Jun 10, 2022

Hmm @janeyx99 do you know why the mergebot is complaining?

It looks like the PR actually merged successfully - I see this commit on master, which points back to this PR: 7b3a0ff.

If you think it's unreliable then I'll ignore it.

@janeyx99
Copy link
Contributor

Hmm @janeyx99 do you know why the mergebot is complaining?

It looks like the PR actually merged successfully - I see this commit on master, which points back to this PR: 7b3a0ff.

If you think it's unreliable then I'll ignore it.

Oh weird--I do think this is not reliable and you can ignore it--the issue tracking it is pytorch/test-infra#397

@ysiraichi ysiraichi added release notes: cpp release notes category topic: bc breaking topic category labels Jun 13, 2022
facebook-github-bot pushed a commit that referenced this pull request Jun 13, 2022
Summary:
Tracking issue: #55070

Pull Request resolved: #69607

Approved by: https://github.com/bdhirsh

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

Reviewed By: osalpekar

Differential Revision: D34706811

Pulled By: bdhirsh

fbshipit-source-id: 4146afb5dee63012e27bb92c4b5851daa9c8f6b0
@facebook-github-bot facebook-github-bot deleted the gh/ysiraichi/39/head branch June 14, 2022 14:17
facebook-github-bot pushed a commit to facebookresearch/pytorch3d that referenced this pull request Jun 29, 2022
Summary: After landing pytorch/pytorch#69607, that made it an error to use indexing with `cpu_tensor[cuda_indices]`. There was one outstanding test in fbcode that incorrectly used indexing in that way, which is fixed here

Reviewed By: bottler, osalpekar

Differential Revision: D37128838

fbshipit-source-id: 611b6f717b5b5d89fa61fd9ebeb513ad7e65a656
AA40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla signed Merged module: structured kernels Related to new structured kernels functionality oncall: jit Add this issue/PR to JIT oncall triage queue open source release notes: cpp release notes category Reverted topic: bc breaking topic category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

0