Conversation
|
Stack from ghstack (oldest at bottom): |
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/ao/1853
Note: Links to docs will display an error until the docs builds have been completed. This comment was automatically generated by Dr. CI and updates every 15 minutes. |
Summary: As titled Test Plan: ``` // note: this fails on weights only load, but the failure happens // after my changes, and already exists on main branch pytest test/prototype/test_awq.py -s -x ``` Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: a6dceb5 ghstack-comment-id: 2706766649 Pull Request resolved: #1853
danielvegamyhre
left a comment
There was a problem hiding this comment.
Change itself looks good to me but this doesn't sound good:
// note: this fails on weights only load, but the failure happens
// after my changes, and already exists on main branch
pytest test/prototype/test_awq.py -s -x
If the bug exists on main and is unrelated to these changes, how has the test been passing CI? Sounds like we should create a bug to track this and maybe the feature owner can take a look
drisspg
left a comment
There was a problem hiding this comment.
Is the model change purposeful
ah, that snuck in. No, will remove. |
in general I agree, but given that this is the prototype folder the responsibility to do what you suggested would be on the owner of awq - I'm not sure who that is :) In general for prototype folder there aren't guarantees that we'll spend our time to migrate things, if people want their thing to continue to keep working they should work on moving the feature out of prototype. |
* Update [ghstack-poisoned] * Update [ghstack-poisoned] * Update [ghstack-poisoned] * Update [ghstack-poisoned] * Update [ghstack-poisoned]
Summary:
As titled
Test Plan:
Reviewers:
Subscribers:
Tasks:
Tags: