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

quantized tensor: add preliminary support for advanced indexing, try 2 #49346

Closed
wants to merge 6 commits into from

Conversation

vkuzo
Copy link
Contributor

@vkuzo vkuzo commented Dec 14, 2020

Stack from ghstack:

Summary:

This is less ambitious redo of
#49129.

We make the

xq_slice = xq[:, [0], :, :]

indexing syntax work if xq is a quantized Tensor. For now, we are
making the code not crash, with an in efficient dq -> index -> q
implementation. A future PR can optimize performance by removing
the unnecessary memory copies (which will require some non-trivial
changes to TensorIterator).

Test Plan:

python test/test_quantization.py TestQuantizedOps.test_advanced_indexing

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: D25539365

Summary:

This is less ambitious redo of
#49129.

We make the

```
xq_slice = xq[:, [0], :, :]
```

indexing syntax work if `xq` is a quantized Tensor.  For now, we are
making the code not crash, with an in efficient `dq -> index -> q`
implementation.  A future PR can optimize performance by removing
the unnecessary memory copies (which will require some non-trivial
changes to TensorIterator).

Test Plan:

```
python test/test_quantization.py TestQuantizedOps.test_advanced_indexing
```

Reviewers:

Subscribers:

Tasks:

Tags:

[ghstack-poisoned]
…for advanced indexing, try 2"

Summary:

This is less ambitious redo of
#49129.

We make the

```
xq_slice = xq[:, [0], :, :]
```

indexing syntax work if `xq` is a quantized Tensor.  For now, we are
making the code not crash, with an in efficient `dq -> index -> q`
implementation.  A future PR can optimize performance by removing
the unnecessary memory copies (which will require some non-trivial
changes to TensorIterator).

Test Plan:

```
python test/test_quantization.py TestQuantizedOps.test_advanced_indexing
```

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: [D25539365](https://our.internmc.facebook.com/intern/diff/D25539365)

[ghstack-poisoned]
…dexing, try 2"

Summary:

This is less ambitious redo of
#49129.

We make the

```
xq_slice = xq[:, [0], :, :]
```

indexing syntax work if `xq` is a quantized Tensor.  For now, we are
making the code not crash, with an in efficient `dq -> index -> q`
implementation.  A future PR can optimize performance by removing
the unnecessary memory copies (which will require some non-trivial
changes to TensorIterator).

Test Plan:

```
python test/test_quantization.py TestQuantizedOps.test_advanced_indexing
```

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: [D25539365](https://our.internmc.facebook.com/intern/diff/D25539365)

[ghstack-poisoned]
vkuzo added a commit that referenced this pull request Dec 14, 2020
Summary:

This is less ambitious redo of
#49129.

We make the

```
xq_slice = xq[:, [0], :, :]
```

indexing syntax work if `xq` is a quantized Tensor.  For now, we are
making the code not crash, with an in efficient `dq -> index -> q`
implementation.  A future PR can optimize performance by removing
the unnecessary memory copies (which will require some non-trivial
changes to TensorIterator).

Test Plan:

```
python test/test_quantization.py TestQuantizedOps.test_advanced_indexing
```

Reviewers:

Subscribers:

Tasks:

Tags:

ghstack-source-id: 9540251c238e7eb1a61db4037b189493a6cca18d
Pull Request resolved: #49346
…ed indexing, try 2"

Summary:

This is less ambitious redo of
#49129.

We make the

```
xq_slice = xq[:, [0], :, :]
```

indexing syntax work if `xq` is a quantized Tensor.  For now, we are
making the code not crash, with an in efficient `dq -> index -> q`
implementation.  A future PR can optimize performance by removing
the unnecessary memory copies (which will require some non-trivial
changes to TensorIterator).

Test Plan:

```
python test/test_quantization.py TestQuantizedOps.test_advanced_indexing
```

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: [D25539365](https://our.internmc.facebook.com/intern/diff/D25539365)

[ghstack-poisoned]
vkuzo added a commit that referenced this pull request Dec 14, 2020
Summary:

This is less ambitious redo of
#49129.

We make the

```
xq_slice = xq[:, [0], :, :]
```

indexing syntax work if `xq` is a quantized Tensor.  For now, we are
making the code not crash, with an in efficient `dq -> index -> q`
implementation.  A future PR can optimize performance by removing
the unnecessary memory copies (which will require some non-trivial
changes to TensorIterator).

Test Plan:

```
python test/test_quantization.py TestQuantizedOps.test_advanced_indexing
```

Reviewers:

Subscribers:

Tasks:

Tags:

ghstack-source-id: c319b525c3d8640027b7c0796331a4682559e2ab
Pull Request resolved: #49346
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Dec 14, 2020

💊 CI failures summary and remediations

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


  • 1/1 failures possibly* introduced in this PR
    • 1/1 non-CircleCI failure(s)

This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions to the (internal) Dr. CI Users group.

This comment has been revised 10 times.

…exing, try 2"

Summary:

This is less ambitious redo of
#49129.

We make the

```
xq_slice = xq[:, [0], :, :]
```

indexing syntax work if `xq` is a quantized Tensor.  For now, we are
making the code not crash, with an in efficient `dq -> index -> q`
implementation.  A future PR can optimize performance by removing
the unnecessary memory copies (which will require some non-trivial
changes to TensorIterator).

Test Plan:

```
python test/test_quantization.py TestQuantizedOps.test_advanced_indexing
```

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: [D25539365](https://our.internmc.facebook.com/intern/diff/D25539365)

[ghstack-poisoned]
TORCH_INTERNAL_ASSERT(
self.qscheme() == c10::kPerTensorAffine ||
self.qscheme() == c10::kPerTensorSymmetric,
"Indexing for per-channel quantized Tensors is not supported.");
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: message should probably be only per-tensor quantized Tensor is supported

Copy link
Contributor

@jerryzh168 jerryzh168 left a comment

Choose a reason for hiding this comment

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

LG, thanks for addressing the comments. had one nit comment inline.

…exing, try 2"

Summary:

This is less ambitious redo of
#49129.

We make the

```
xq_slice = xq[:, [0], :, :]
```

indexing syntax work if `xq` is a quantized Tensor.  For now, we are
making the code not crash, with an in efficient `dq -> index -> q`
implementation.  A future PR can optimize performance by removing
the unnecessary memory copies (which will require some non-trivial
changes to TensorIterator).

Test Plan:

```
python test/test_quantization.py TestQuantizedOps.test_advanced_indexing
```

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: [D25539365](https://our.internmc.facebook.com/intern/diff/D25539365)

[ghstack-poisoned]
vkuzo added a commit that referenced this pull request Dec 16, 2020
Summary:

This is less ambitious redo of
#49129.

We make the

```
xq_slice = xq[:, [0], :, :]
```

indexing syntax work if `xq` is a quantized Tensor.  For now, we are
making the code not crash, with an in efficient `dq -> index -> q`
implementation.  A future PR can optimize performance by removing
the unnecessary memory copies (which will require some non-trivial
changes to TensorIterator).

Test Plan:

```
python test/test_quantization.py TestQuantizedOps.test_advanced_indexing
```

Reviewers:

Subscribers:

Tasks:

Tags:

ghstack-source-id: 7591b5f90438a20fc92b962887092834e220c14a
Pull Request resolved: #49346
@codecov
Copy link

codecov bot commented Dec 16, 2020

Codecov Report

Merging #49346 (e788c68) into gh/vkuzo/188/base (16f4b0e) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@                Coverage Diff                 @@
##           gh/vkuzo/188/base   #49346   +/-   ##
==================================================
  Coverage              80.62%   80.62%           
==================================================
  Files                   1875     1875           
  Lines                 202771   202781   +10     
==================================================
+ Hits                  163488   163497    +9     
- Misses                 39283    39284    +1     

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in a9137ae.

hwangdeyu pushed a commit to hwangdeyu/pytorch that referenced this pull request Jan 6, 2021
pytorch#49346)

Summary:
Pull Request resolved: pytorch#49346

This is less ambitious redo of
pytorch#49129.

We make the

```
xq_slice = xq[:, [0], :, :]
```

indexing syntax work if `xq` is a quantized Tensor.  For now, we are
making the code not crash, with an in efficient `dq -> index -> q`
implementation.  A future PR can optimize performance by removing
the unnecessary memory copies (which will require some non-trivial
changes to TensorIterator).

Test Plan:
```
python test/test_quantization.py TestQuantizedOps.test_advanced_indexing
```

Imported from OSS

Reviewed By: jerryzh168

Differential Revision: D25539365

fbshipit-source-id: 98485875aaaf5743e1a940e170258057691be4fa
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.

None yet

4 participants