-
Notifications
You must be signed in to change notification settings - Fork 262
Move more utils to TorchAOBaseTensor #784
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
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/ao/784
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 8586f6a with merge base 6080986 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
4c8eaac
to
f70e229
Compare
Summary: This moves over _implements, _dispatch__torch_dispatch__, _dispatch__torch_function__, _register_layout_cls and _get_layout_tensor_constructor to `TorchAOBaseTensor` so when people inherit from this, they can get these utils directly Test Plan: python test/quantization/test_quant_api.py python test/integration/test_integration.py rely on CI for other tests Reviewers: Subscribers: Tasks: Tags:
f70e229
to
60444e7
Compare
can this handle the other ones from #710? feels like there should be a way to set it up so that things like detach are handlable with a combination of tensor_flatten and the constructor? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good otherwise
yeah will do in a future PR |
@@ -75,7 +75,7 @@ def test_quantize_4bit_with_qmap_compile(self, device): | |||
|
|||
|
|||
class TestOptim(TestCase): | |||
@pytest.mark.skipif(not TORCH_VERSION_AT_LEAST_2_3, reason="requires PyTorch >= 2.3") | |||
@pytest.mark.skipif(not TORCH_VERSION_AT_LEAST_2_4, reason="requires PyTorch >= 2.3") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @gau-nernst multiple levels of inheritance breaks pytorch 2.3 version (https://github.com/pytorch/ao/actions/runs/10689912664/job/29633120953?pr=784), so I had to use 2.4+ for this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally I would prefer to keep supporting PyTorch 2.3 for low-bit optimizers. @msaroufim What do you think?
From the CI link you show, it seems like only 4-bit optim breaks, while 8-bit and Fp8 optim still work? Anyway, if we keep this change, also need to update the docs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After some investigation, the errors only happen to 4-bit optim, and because 4-bit optim uses dynamic-shape compile. Some strange interactions between inheritance and dynamic-shape 🤔. Might have some fix soon..
* Move more utils to TorchAOBaseTensor Summary: This moves over _implements, _dispatch__torch_dispatch__, _dispatch__torch_function__, _register_layout_cls and _get_layout_tensor_constructor to `TorchAOBaseTensor` so when people inherit from this, they can get these utils directly Test Plan: python test/quantization/test_quant_api.py python test/integration/test_integration.py rely on CI for other tests Reviewers: Subscribers: Tasks: Tags:
Summary:
This moves over implements, dispatch__torch_dispatch, dispatch__torch_function_, _register_layout_cls and _get_layout_tensor_constructor to
TorchAOBaseTensor
so when people inherit from this, they can get these utils directlyTest Plan:
python test/quantization/test_quant_api.py
python test/integration/test_integration.py
rely on CI for other tests
Reviewers:
Subscribers:
Tasks:
Tags: