Skip to content

[quant] Quantized Tensor support deepcopy #28612

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.

Sign up for GitHub

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

Jump to bottom
Closed
jerryzh168 wants to merge 8 commits into from
Closed

[quant] Quantized Tensor support deepcopy #28612

jerryzh168 wants to merge 8 commits into from

Conversation

@jerryzh168
Copy link
Contributor

@jerryzh168 jerryzh168 commented Oct 24, 2019 •
edited
Loading

Stack from ghstack:

Summary:
att

Test Plan:
python test/test_quantized_tensor.py

Reviewers:
gchanan

Subscribers:

Tasks:

Tags:

Differential Revision: D18255247

@jerryzh168
[quant] Quantized Tensor support copy
1b8570f 
Summary:
att

Test Plan:
python test/test_quantized_tensor.py

Reviewers:
gchanan

Subscribers:

Tasks:

Tags:

[ghstack-poisoned]
@jerryzh168
Update on "[quant] Quantized Tensor support copy"
a03ea20 
Summary:
att

Test Plan:
python test/test_quantized_tensor.py

Reviewers:
gchanan

Subscribers:

Tasks:

Tags:

[ghstack-poisoned]
jerryzh168 added a commit that referenced this pull request Oct 24, 2019
@jerryzh168
[quant] Quantized Tensor support copy
e6bb6d3 
Summary:
att

Test Plan:
python test/test_quantized_tensor.py

Reviewers:
gchanan

Subscribers:

Tasks:

Tags:

ghstack-source-id: cad7afa
Pull Request resolved: #28612
@jerryzh168 jerryzh168 mentioned this pull request Oct 24, 2019
Closed
@jerryzh168
Update on "[quant] Quantized Tensor support copy"
c219873 
Summary:
att

Test Plan:
python test/test_quantized_tensor.py

Reviewers:
gchanan

Subscribers:

Tasks:

Tags:

[ghstack-poisoned]
jerryzh168 added a commit that referenced this pull request Oct 25, 2019
@jerryzh168
[quant] Quantized Tensor support copy
672941a 
Summary:
att

Test Plan:
python test/test_quantized_tensor.py

Reviewers:
gchanan

Subscribers:

Tasks:

Tags:

ghstack-source-id: ffb90cb
Pull Request resolved: #28612
gchanan
gchanan reviewed Oct 25, 2019
View reviewed changes
Copy link
Contributor

@gchanan gchanan left a comment

Choose a reason for hiding this comment

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

can you remind me why quantized tensors use qint8 storages and not int8 storages?

@jerryzh168
Copy link
Contributor Author

jerryzh168 commented Oct 25, 2019

can you remind me why quantized tensors use qint8 storages and not int8 storages?

I don't remember exactly, but one reason I can think of right now is because when we inspect the storage we want to distinguish between qint8 and int8 storage?

@jerryzh168
Update on "[quant] Quantized Tensor support copy"
48e2dc2 
Summary:
att

Test Plan:
python test/test_quantized_tensor.py

Reviewers:
gchanan

Subscribers:

Tasks:

Tags:

[ghstack-poisoned]
jerryzh168 added a commit that referenced this pull request Oct 25, 2019
@jerryzh168
[quant] Quantized Tensor support copy
42b6ac7 
Summary:
att

Test Plan:
python test/test_quantized_tensor.py

Reviewers:
gchanan

Subscribers:

Tasks:

Tags:

ghstack-source-id: 893238c
Pull Request resolved: #28612
gchanan
gchanan reviewed Oct 28, 2019
View reviewed changes
Copy link
Contributor

@gchanan gchanan left a comment

Choose a reason for hiding this comment

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

does this support copy_, i.e. a.copy_(b)?

@jerryzh168
Update on "[quant] Quantized Tensor support copy"
516e064 
Summary:
att

Test Plan:
python test/test_quantized_tensor.py

Reviewers:
gchanan

Subscribers:

Tasks:

Tags:

[ghstack-poisoned]
jerryzh168 added a commit that referenced this pull request Oct 29, 2019
@jerryzh168
[quant] Quantized Tensor support copy
ad5edca 
Summary:
att

Test Plan:
python test/test_quantized_tensor.py

Reviewers:
gchanan

Subscribers:

Tasks:

Tags:

ghstack-source-id: a58fafe
Pull Request resolved: #28612
@jerryzh168
Copy link
Contributor Author

jerryzh168 commented Oct 29, 2019

does this support copy_, i.e. a.copy_(b)?

Yes it does, test added

@jerryzh168 jerryzh168 requested a review from gchanan October 29, 2019 23:05
gchanan
gchanan reviewed Oct 31, 2019
View reviewed changes
@jerryzh168
Update on "[quant] Quantized Tensor support copy"
67fb5b1 
Summary:
att

Test Plan:
python test/test_quantized_tensor.py

Reviewers:
gchanan

Subscribers:

Tasks:

Tags:

[ghstack-poisoned]
@jerryzh168
Update on "[quant] Quantized Tensor support copy"
baab1e5 
Summary:
att

Test Plan:
python test/test_quantized_tensor.py

Reviewers:
gchanan

Subscribers:

Tasks:

Tags:

[ghstack-poisoned]
@jerryzh168
Update on "[quant] Quantized Tensor support copy"
e922b4a 
Summary:
att

Test Plan:
python test/test_quantized_tensor.py

Reviewers:
gchanan

Subscribers:

Tasks:

Tags:

[ghstack-poisoned]
jerryzh168 added a commit that referenced this pull request Oct 31, 2019
@jerryzh168
[quant] Quantized Tensor support copy
90b77ed 
Summary:
att

Test Plan:
python test/test_quantized_tensor.py

Reviewers:
gchanan

Subscribers:

Tasks:

Tags:

ghstack-source-id: 92ff628
Pull Request resolved: #28612
@jerryzh168 jerryzh168 requested a review from gchanan October 31, 2019 20:52
@jerryzh168 jerryzh168 changed the title [quant] Quantized Tensor support copy [quant] Quantized Tensor support deepcopy Nov 1, 2019
@jerryzh168
Copy link
Contributor Author

jerryzh168 commented Nov 1, 2019

@gchanan this PR only adds support for deepcopy, normal quantized tensor copy is already supported in another PR: #21852

gchanan
gchanan approved these changes Nov 1, 2019
View reviewed changes
Copy link
Contributor

@gchanan gchanan left a comment

Choose a reason for hiding this comment

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

this, in-and-of-itself, looks fine for deepcopy.

Why was the existing copy behavior (of clobbering the quantizer of the destination) chosen? That seems surprising, and I don't know why you even want to copy in that case.

@jerryzh168
Copy link
Contributor Author

jerryzh168 commented Nov 1, 2019

this, in-and-of-itself, looks fine for deepcopy.

Why was the existing copy behavior (of clobbering the quantizer of the destination) chosen? That seems surprising, and I don't know why you even want to copy in that case.

It's because of:

pytorch/torch/nn/modules/module.py

Line 771 in e5d6b75

param.copy_(input_param)

@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Nov 2, 2019

This pull request has been merged in 23193c1.

zdevito pushed a commit to zdevito/ATen that referenced this pull request Nov 2, 2019
@jerryzh168 @facebook-github-bot
Quantized Tensor support copy (#28612)
e614790 
Summary:
Pull Request resolved: pytorch/pytorch#28612

att

Test Plan:
python test/test_quantized_tensor.py

Imported from OSS

Differential Revision: D18255247

fbshipit-source-id: 814b12640fdf9d79b27482ee642ce430dbaeea68
soumith pushed a commit that referenced this pull request Nov 4, 2019
@jerryzh168 @soumith
Quantized Tensor support copy (#28612)
f7f5385 
Summary:
Pull Request resolved: #28612

att

Test Plan:
python test/test_quantized_tensor.py

Imported from OSS

Differential Revision: D18255247

fbshipit-source-id: 814b12640fdf9d79b27482ee642ce430dbaeea68
@facebook-github-bot facebook-github-bot deleted the gh/jerryzh168/120/head branch November 5, 2019 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gchanan gchanan gchanan approved these changes

@izdeby izdeby Awaiting requested review from izdeby

@zou3519 zou3519 Awaiting requested review from zou3519

@jamesr66a jamesr66a Awaiting requested review from jamesr66a

@bddppq bddppq Awaiting requested review from bddppq

Assignees

No one assigned

Labels

Merged

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@jerryzh168 @facebook-github-bot @gchanan @mruberry