[ao] correctly set public v private for fake_quantize.py #86022
Conversation
Summary: biggest issue was that the constructors for the fake_quantize classes use custom partials that live in the observer module and so the module for these needed to be set correctly in the constructor class method Test Plan: python test/test_public_bindings.py Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/86022
Note: Links to docs will display an error until the docs builds have been completed. ✅ No Failures, 2 PendingAs of commit b04854b: This comment was automatically generated by Dr. CI and updates every 15 minutes. |
| @@ -18,6 +18,33 @@ | |||
| from abc import ABC, abstractmethod | |||
| from typing import Any, Tuple | |||
|
|
|||
| __all__ = [ | |||
| "FakeQuantizeBase", | |||
There was a problem hiding this comment.
do we want to fix what is visible to users as well? i.e. hide the things that we don't think makes sense to be public?
There was a problem hiding this comment.
is that the intent of this? I thought it was more about what things will be accessed by other modules
There was a problem hiding this comment.
I remember it's both to clean up the json file and also make the correct public/private decision for our apis, but we can confirm in team meeting
There was a problem hiding this comment.
Everything in __all__ is supposed to be public and documented. If it's not meant to be public, then it should be preceded with an underscore.
HDCharles
added
topic: not user facing
Summary: biggest issue was that the constructors for the fake_quantize classes use custom partials that live in the observer module and so the module for these needed to be set correctly in the constructor class method Test Plan: python test/test_public_bindings.py Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]
|
/easycla As part of the transition to the PyTorch Foundation, this project now requires contributions be covered under the new CLA. See #85559 for additional details. This comment will trigger a new check of this PR. If you are already covered, you will simply see a new "EasyCLA" check that passes. If you are not covered, a bot will leave a new comment with a link to sign. |
Summary: biggest issue was that the constructors for the fake_quantize classes use custom partials that live in the observer module and so the module for these needed to be set correctly in the constructor class method Test Plan: python test/test_public_bindings.py Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]
|
@pytorchmergebot merge |
|
@pytorchbot successfully started a merge job. Check the current status here. |
…6022) Summary: biggest issue was that the constructors for the fake_quantize classes use custom partials that live in the observer module and so the module for these needed to be set correctly in the constructor class method Pull Request resolved: #86022 Approved by: https://github.com/jerryzh168 Test Plan: contbuild & OSS CI, see https://hud.pytorch.org/commit/pytorch/pytorch/c21caff8765f00ed5a2e1ed448ad5e6329c87b8d Test plan from GitHub: python test/test_public_bindings.py Reviewed By: seemethere Differential Revision: D40166937 Pulled By: seemethere fbshipit-source-id: 14970e3519aeafc86a3ee9acaa8baa08b09e33b2
Stack from ghstack (oldest at bottom):
Summary: biggest issue was that the constructors for the fake_quantize
classes use custom partials that live in the observer module and so
the module for these needed to be set correctly in the constructor class
method
Test Plan: python test/test_public_bindings.py
Reviewers:
Subscribers:
Tasks:
Tags: