Skip to content

[quant] Implement unsqueeze/squeeze for per-channel qtensor#38247

Closed
supriyar wants to merge 6 commits intogh/supriyar/111/basefrom
gh/supriyar/111/head
Closed

[quant] Implement unsqueeze/squeeze for per-channel qtensor#38247
supriyar wants to merge 6 commits intogh/supriyar/111/basefrom
gh/supriyar/111/head

Conversation

@supriyar
Copy link
Copy Markdown
Contributor

@supriyar supriyar commented May 11, 2020

Stack from ghstack:

Summary:
Per-channel quantized tensor axis value is shifted based on the unsqueeze/squeeze dim

Test Plan:
python test/test_quantization.py TestQuantizedTensor.test_qtensor_unsqueze

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: D21550293

Summary:
Per-channel quantized tensor axis value is shifted based on the unsqueeze/squeeze dim

Test Plan:
python test/test_quantization.py TestQuantizedTensor.test_qtensor_unsqueze

Reviewers:

Subscribers:

Tasks:

Tags:

[ghstack-poisoned]
@dr-ci
Copy link
Copy Markdown

dr-ci Bot commented May 11, 2020

💊 CI failures summary and remediations

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


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

ci.pytorch.org: 1 failed


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 on the GitHub issue tracker.

See how this bot performed.

This comment has been revised 16 times.

@supriyar supriyar requested a review from jerryzh168 May 11, 2020 18:21
supriyar added 2 commits May 11, 2020 11:32
Summary:
Per-channel quantized tensor axis value is shifted based on the unsqueeze/squeeze dim

Test Plan:
python test/test_quantization.py TestQuantizedTensor.test_qtensor_unsqueze

Reviewers:

Subscribers:

Tasks:

Tags:

[ghstack-poisoned]
Summary:
Per-channel quantized tensor axis value is shifted based on the unsqueeze/squeeze dim

Test Plan:
python test/test_quantization.py TestQuantizedTensor.test_qtensor_unsqueze

Reviewers:

Subscribers:

Tasks:

Tags:

[ghstack-poisoned]
namedinference::propagate_names_except(result, self, {dim});
return result;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we also add support for squeeze with no dimensions specified?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sure

Copy link
Copy Markdown
Contributor

@raghuramank100 raghuramank100 left a comment

Choose a reason for hiding this comment

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

Looks good, a few suggestions to add more coverage

Summary:
Per-channel quantized tensor axis value is shifted based on the unsqueeze/squeeze dim

Test Plan:
python test/test_quantization.py TestQuantizedTensor.test_qtensor_unsqueze

Reviewers:

Subscribers:

Tasks:

Tags:

[ghstack-poisoned]
@supriyar supriyar requested a review from raghuramank100 May 11, 2020 22:07
return sum_to(self, size);
}

Tensor make_qtensor(const Tensor& self, IntArrayRef size, IntArrayRef stride, QuantizerPtr quantizer) {
Copy link
Copy Markdown
Contributor

@jerryzh168 jerryzh168 May 11, 2020

Choose a reason for hiding this comment

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

I think you can define a new as_strided_qtensorimpl and remove the current as_strided_qtensorimpl

Summary:
Per-channel quantized tensor axis value is shifted based on the unsqueeze/squeeze dim

Test Plan:
python test/test_quantization.py TestQuantizedTensor.test_qtensor_unsqueze

Reviewers:

Subscribers:

Tasks:

Tags:

[ghstack-poisoned]
Copy link
Copy Markdown
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.

please make make_qtensor a native function and remove as_strided_qtensorimpl

@supriyar
Copy link
Copy Markdown
Contributor Author

please make make_qtensor a native function and remove as_strided_qtensorimpl

@jerryzh168 as_strided_qtensorimpl is still used by other ops like unfold, diagonal, expand, permute. We'd have to update all those ops as well for per-channel quant. I think we can keep it as is for now and update later when we want to make these ops work for per-channel as well. Sounds good?

@jerryzh168
Copy link
Copy Markdown
Contributor

Sure, but then could you create an issue, or add a TODO to reflect the plan?

Copy link
Copy Markdown
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.

need follow up PR to change the API of as_strided_qtensorimpl

Summary:
Per-channel quantized tensor axis value is shifted based on the unsqueeze/squeeze dim

Test Plan:
python test/test_quantization.py TestQuantizedTensor.test_qtensor_unsqueze

Reviewers:

Subscribers:

Tasks:

Tags:

[ghstack-poisoned]
@facebook-github-bot
Copy link
Copy Markdown
Contributor

This pull request has been merged in b90fc52.

@facebook-github-bot facebook-github-bot deleted the gh/supriyar/111/head branch May 17, 2020 14:18
laurentdupin pushed a commit to laurentdupin/pytorch that referenced this pull request Apr 24, 2026
…38247)

Summary:
Pull Request resolved: pytorch#38247

Per-channel quantized tensor axis value is shifted based on the unsqueeze/squeeze dim

Test Plan:
python test/test_quantization.py TestQuantizedTensor.test_qtensor_unsqueze

Imported from OSS

Differential Revision: D21550293

fbshipit-source-id: 90ea4a1bd637588360b3228cb5af9176176eb033
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants