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

[quant][graphmode] Make HistogramObserver scriptable with @torch.jit.ignore #27950

Closed
wants to merge 13 commits into from

Conversation

jerryzh168
Copy link
Contributor

@jerryzh168 jerryzh168 commented Oct 14, 2019

Stack from ghstack:

Summary:
att

Test Plan:
python test/test_quantization.py

Reviewers:
pt1quant

Subscribers:

Tasks:

Tags:

Differential Revision: D18360139

Summary:
Make observers scriptable with `@torch.jit.ignore`

Test Plan:
python test/test_quantization.py

Reviewers:
pt1quant

Subscribers:

Tasks:

Tags:

[ghstack-poisoned]
… in graph mode"

Summary:
Make observers scriptable with `@torch.jit.ignore`

Test Plan:
python test/test_quantization.py

Reviewers:
pt1quant

Subscribers:

Tasks:

Tags:

[ghstack-poisoned]
jerryzh168 added a commit that referenced this pull request Oct 15, 2019
Summary:
Make observers scriptable with `@torch.jit.ignore`

Test Plan:
python test/test_quantization.py

Reviewers:
pt1quant

Subscribers:

Tasks:

Tags:

ghstack-source-id: 1176493638e741d8eb5710bbc8ab7e4a039919f0
Pull Request resolved: #27950
… in graph mode"

Summary:
Make observers scriptable with `@torch.jit.ignore`

Test Plan:
python test/test_quantization.py

Reviewers:
pt1quant

Subscribers:

Tasks:

Tags:

[ghstack-poisoned]
jerryzh168 added a commit that referenced this pull request Oct 17, 2019
Summary:
Make observers scriptable with `@torch.jit.ignore`

Test Plan:
python test/test_quantization.py

Reviewers:
pt1quant

Subscribers:

Tasks:

Tags:

ghstack-source-id: ee150974949872e0b506f3cd127120eba3f4e37f
Pull Request resolved: #27950
… in graph mode"

Summary:
Make observers scriptable with `@torch.jit.ignore`

Test Plan:
python test/test_quantization.py

Reviewers:
pt1quant

Subscribers:

Tasks:

Tags:

[ghstack-poisoned]
… in graph mode"

Summary:
Make observers scriptable with `@torch.jit.ignore`

Test Plan:
python test/test_quantization.py

Reviewers:
pt1quant

Subscribers:

Tasks:

Tags:

[ghstack-poisoned]
jerryzh168 added a commit that referenced this pull request Oct 17, 2019
Summary:
Make observers scriptable with `@torch.jit.ignore`

Test Plan:
python test/test_quantization.py

Reviewers:
pt1quant

Subscribers:

Tasks:

Tags:

ghstack-source-id: 4c406ac5ed0adc7cdcc3eb9016b0401513997bad
Pull Request resolved: #27950
… in graph mode"

Summary:
Make observers scriptable with `@torch.jit.ignore`

Test Plan:
python test/test_quantization.py

Reviewers:
pt1quant

Subscribers:

Tasks:

Tags:

[ghstack-poisoned]
jerryzh168 added a commit that referenced this pull request Oct 18, 2019
Summary:
Make observers scriptable with `@torch.jit.ignore`

Test Plan:
python test/test_quantization.py

Reviewers:
pt1quant

Subscribers:

Tasks:

Tags:

ghstack-source-id: 769c26551c8feea1502b92a95a203c355bf67173
Pull Request resolved: #27950
… in graph mode"

Summary:
Make observers scriptable with `@torch.jit.ignore`

Test Plan:
python test/test_quantization.py

Reviewers:
pt1quant

Subscribers:

Tasks:

Tags:

[ghstack-poisoned]
jerryzh168 added a commit that referenced this pull request Nov 2, 2019
Summary:
Make observers scriptable with `@torch.jit.ignore`

Test Plan:
python test/test_quantization.py

Reviewers:
pt1quant

Subscribers:

Tasks:

Tags:

ghstack-source-id: e14178b6c7fd2716da8ed1f372ef953038b5edd8
Pull Request resolved: #27950
… in graph mode"

Summary:
Make observers scriptable with `@torch.jit.ignore`

Test Plan:
python test/test_quantization.py

Reviewers:
pt1quant

Subscribers:

Tasks:

Tags:

[ghstack-poisoned]
… in graph mode"

Summary:
Make HistogramObserver scriptable with `@torch.jit.ignore`

Test Plan:
python test/test_quantization.py

Reviewers:
pt1quant

Subscribers:

Tasks:

Tags:

[ghstack-poisoned]
…`@torch.jit.ignore`"

Summary:
att

Test Plan:
python test/test_quantization.py

Reviewers:
pt1quant

Subscribers:

Tasks:

Tags:

[ghstack-poisoned]
@jerryzh168 jerryzh168 changed the title [quant][graphmode] Accept observers with ignored functions in graph mode [quant][graphmode] Make HistogramObserver scriptable with @torch.jit.ignore Nov 6, 2019
jerryzh168 added a commit that referenced this pull request Nov 6, 2019
….ignore`

Summary:
att

Test Plan:
python test/test_quantization.py

Reviewers:
pt1quant

Subscribers:

Tasks:

Tags:

ghstack-source-id: 87194aa08a34481a0c91b5060b2c516fa9f1091a
Pull Request resolved: #27950
@jerryzh168
Copy link
Contributor Author

This is ready for review please take a look

Copy link

@ZolotukhinM ZolotukhinM 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, but please remove unnecessary changes and split out unrelated changes and avoid that in future.

test/test_quantization.py Outdated Show resolved Hide resolved
torch/quantization/observer.py Outdated Show resolved Hide resolved
@@ -244,7 +244,7 @@ Module Module::clone_impl(
} else {
r.register_attribute(
s.name,
s.value.type(),
type()->getAttribute(s.name),

Choose a reason for hiding this comment

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

This is an unrelated change.

def _combine_histograms(
self, dst_histogram, dst_min, dst_max, src_histogram, src_min, src_max
):
# type: (Tensor, float, float, Tensor, float, float) -> Tensor

Choose a reason for hiding this comment

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

Do you need this if the function is ignored?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, we need this otherwise everything will be treated as Tensor

@@ -750,46 +756,50 @@ def _combine_histograms(
# remaining should go to dst_bin2
if dst_bin_cnt < src_bin_count:
dst_histogram[dst_bin2] += src_bin_count - dst_bin_cnt
return dst_histogram

Choose a reason for hiding this comment

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

This seems to be an unnecessary change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is needed for compile as well, ignored functions seems to take Tensor as return only

test/test_quantization.py Outdated Show resolved Hide resolved
…`@torch.jit.ignore`"

Summary:
att

Test Plan:
python test/test_quantization.py

Reviewers:
pt1quant

Subscribers:

Tasks:

Tags:

[ghstack-poisoned]
…`@torch.jit.ignore`"

Summary:
att

Test Plan:
python test/test_quantization.py

Reviewers:
pt1quant

Subscribers:

Tasks:

Tags:

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

[ghstack-poisoned]
jerryzh168 added a commit that referenced this pull request Nov 6, 2019
….ignore`

Summary:
att

Test Plan:
python test/test_quantization.py

Reviewers:
pt1quant

Subscribers:

Tasks:

Tags:

ghstack-source-id: bf26f8990ae2e5773e21df3ebf58ad84322fe4d3
Pull Request resolved: #27950
…`@torch.jit.ignore`"

Summary:
att

Test Plan:
python test/test_quantization.py

Reviewers:
pt1quant

Subscribers:

Tasks:

Tags:

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

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

This pull request has been merged in 4bcf479.

xxtEchjovs44 pushed a commit to xxtEchjovs44/pytorch that referenced this pull request Jan 29, 2020
….ignore`

Summary:
att

Test Plan:
python test/test_quantization.py

Reviewers:
pt1quant

Subscribers:

Tasks:

Tags:

ghstack-source-id: add7f5d0f6a104dcb70344c0abce9e1a8a87bcf2
Pull Request resolved: pytorch/pytorch#27950
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.

4 participants