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

[ao] making _is_activation_post_process private #87520

Closed
wants to merge 17 commits into from

Conversation

[ao] making _is_activation_post_process private

Summary: same function in observer and quantize, consolidated to a
single function

Test Plan: python test/test_public_bindings.py
python test/test_quantization.py

Reviewers:

Subscribers:

Tasks:

Tags:

[ghstack-poisoned]
@pytorch-bot
Copy link

pytorch-bot bot commented Oct 21, 2022

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/87520

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit 9c87c82:
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

[ao] making _is_activation_post_process private

Summary: same function in observer and quantize, consolidated to a
single function

Test Plan: python test/test_public_bindings.py
python test/test_quantization.py

Reviewers:

Subscribers:

Tasks:

Tags:

[ghstack-poisoned]
@HDCharles
Copy link
Contributor Author

@HDCharles has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[ao] making _is_activation_post_process private

Summary: same function in observer and quantize, consolidated to a
single function

Test Plan: python test/test_public_bindings.py
python test/test_quantization.py

Reviewers:

Subscribers:

Tasks:

Tags:

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

[ghstack-poisoned]
@HDCharles
Copy link
Contributor Author

@HDCharles has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[ao] making _is_activation_post_process private

Summary: same function in observer and quantize, consolidated to a
single function

Test Plan: python test/test_public_bindings.py
python test/test_quantization.py

Reviewers:

Subscribers:

Tasks:

Tags:

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

[ghstack-poisoned]
@HDCharles
Copy link
Contributor Author

@HDCharles has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@@ -62,11 +62,6 @@ def get_default_custom_config_dict():
"""
return _DEFAULT_CUSTOM_CONFIG_DICT

def is_activation_post_process(module):
Copy link
Contributor

Choose a reason for hiding this comment

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

I saw that the observer function and this different, just wanted to double check

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, it added a few definitions that weren't captured so it will be more accurate unless people were using this as a negative test.

Summary: same function in observer and quantize, consolidated to a
single function

Test Plan: python test/test_public_bindings.py
python test/test_quantization.py

Reviewers:

Subscribers:

Tasks:

Tags:

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

[ghstack-poisoned]
@HDCharles
Copy link
Contributor Author

@HDCharles has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Summary: same function in observer and quantize, consolidated to a
single function

Test Plan: python test/test_public_bindings.py
python test/test_quantization.py

Reviewers:

Subscribers:

Tasks:

Tags:

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

[ghstack-poisoned]
Summary: same function in observer and quantize, consolidated to a
single function

Test Plan: python test/test_public_bindings.py
python test/test_quantization.py

Reviewers:

Subscribers:

Tasks:

Tags:

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

[ghstack-poisoned]
Summary: same function in observer and quantize, consolidated to a
single function

Test Plan: python test/test_public_bindings.py
python test/test_quantization.py

Reviewers:

Subscribers:

Tasks:

Tags:

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

[ghstack-poisoned]
@HDCharles
Copy link
Contributor Author

@HDCharles has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Summary: same function in observer and quantize, consolidated to a
single function. Note the definitions were slightly different, I've
changed the definition to be maximally inclusive so that the name of the
function is more accurate

Test Plan: python test/test_public_bindings.py
python test/test_quantization.py

Reviewers:

Subscribers:

Tasks:

Tags:

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

[ghstack-poisoned]
Summary: same function in observer and quantize, consolidated to a
single function. Note the definitions were slightly different, I've
changed the definition to be maximally inclusive so that the name of the
function is more accurate

Test Plan: python test/test_public_bindings.py
python test/test_quantization.py

Reviewers:

Subscribers:

Tasks:

Tags:

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

[ghstack-poisoned]
Summary: same function in observer and quantize, consolidated to a
single function. Note the definitions were slightly different, I've
changed the definition to be maximally inclusive so that the name of the
function is more accurate

Test Plan: python test/test_public_bindings.py
python test/test_quantization.py

Reviewers:

Subscribers:

Tasks:

Tags:

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

[ghstack-poisoned]
Summary: same function in observer and quantize, consolidated to a
single function. Note the definitions were slightly different, I've
changed the definition to be maximally inclusive so that the name of the
function is more accurate

Test Plan: python test/test_public_bindings.py
python test/test_quantization.py

Reviewers:

Subscribers:

Tasks:

Tags:

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

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

@pytorchbot merge

(Initiating merge automatically since Phabricator Diff has merged)

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Nov 16, 2022
@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

@bigfootjon
Copy link
Member

@pytorchbot revert -m="Diff reverted internally" -c="ghfirst"

@pytorchmergebot
Copy link
Collaborator

@pytorchbot successfully started a revert job. Check the current status here.
Questions? Feedback? Please reach out to the PyTorch DevX Team

@pytorchmergebot
Copy link
Collaborator

Reverting PR 87520 failed

Reason: Command git -C /home/runner/work/pytorch/pytorch revert --no-edit 45c62a337756ff9db97cd64d2d42d9e65dda0a85 returned non-zero exit code 1

Auto-merging test/quantization/fx/test_quantize_fx.py
Auto-merging torch/ao/quantization/fx/convert.py
Auto-merging torch/ao/quantization/observer.py
Auto-merging torch/ao/quantization/quantize.py
CONFLICT (content): Merge conflict in torch/ao/quantization/quantize.py
error: could not revert 45c62a3377... [ao] making _is_activation_post_process private (#87520)
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git revert --continue".
hint: You can instead skip this commit with "git revert --skip".
hint: To abort and get back to the state before "git revert",
hint: run "git revert --abort".
Details for Dev Infra team Raised by workflow job

@bigfootjon
Copy link
Member

@pytorchbot revert -m="Diff reverted internally" -c="ghfirst"

@pytorchmergebot
Copy link
Collaborator

@pytorchbot successfully started a revert job. Check the current status here.
Questions? Feedback? Please reach out to the PyTorch DevX Team

@pytorchmergebot
Copy link
Collaborator

@HDCharles your PR has been successfully reverted.

pytorchmergebot added a commit that referenced this pull request Nov 21, 2022
This reverts commit 45c62a3.

Reverted #87520 on behalf of https://github.com/bigfootjon due to Diff reverted internally
kulinseth pushed a commit to kulinseth/pytorch that referenced this pull request Dec 10, 2022
Summary: same function in observer and quantize, consolidated to a
single function. Note the definitions were slightly different, I've
changed the definition to be maximally inclusive so that the name of the
function is more accurate

Test Plan: python test/test_public_bindings.py
python test/test_quantization.py

Reviewers:

Subscribers:

Tasks:

Tags:

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

Pull Request resolved: pytorch#87520
Approved by: https://github.com/jcaip
kulinseth pushed a commit to kulinseth/pytorch that referenced this pull request Dec 10, 2022
@facebook-github-bot facebook-github-bot deleted the gh/HDCharles/126/head branch June 8, 2023 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ciflow/trunk Trigger trunk jobs on your pull request Merged oncall: quantization Quantization support in PyTorch release notes: quantization release notes category Reverted
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants