Fixes observer attachment to model based on config for wanda sparsifier#1265
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/ao/1265
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit f2e8f82 with merge base 53d2486 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
Hi @agrawal-aka! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
|
Didn't find following labels among repository labels: bug fix |
|
@pytorchbot label "topic: bug fix" |
15f8e40 to
366261e
Compare
|
Hi, @jcaip @HDCharles @msaroufim. Can you please review this PR whenever you get some time. |
|
Hi @agrawal-aka thanks for the fix - looks good, but can you add in a test for this case here: ao/test/sparsity/test_wanda.py Line 4 in 54c3f31 |
b1f1c61 to
63eeb7e
Compare
|
Hi @jcaip, thanks for reviewing. I have added a test case in test_wanda.py, which sparsify layers based on config, let me know if anything needs to be changed |
ce020eb to
b04b587
Compare
|
Hi @jcaip, I rebased the PR with the latest main, could you retrigger the build pipelines? |
|
Hi @jcaip, lint CI seems to be failing on some different file which was not edited in this PR. Could you tell the fix? |
b04b587 to
f2e8f82
Compare
|
@jcaip , all checks are passed, I think you can go ahead and merge the PR and close the issue as well, if everything looks good to you |
|
@pytorchbot rebase |
|
You don't have permissions to rebase this PR since you are a first time contributor. If you think this is a mistake, please contact PyTorch Dev Infra. |
|
@pytorchbot rebase |
|
You don't have permissions to rebase this PR since you are a first time contributor. If you think this is a mistake, please contact PyTorch Dev Infra. |
|
@jcaip @jerryzh168 @msaroufim, can you help in merging this PR? The changes are already reviewed and approved. |
…er (#1265) * Fixes observer attachment to model based on config for wanda sparsifier * handles case when no config is specified * Added test case in test_wanda.py for custom config * lint fix
This PR fixes the issue https://github.com/pytorch/ao/issues/1133 by only attaching the observers to the layers passed in the sparse config.
Earlier irrespective of the layers being defined in sparse config, the observer was being attached to the entire model, leading to runtime errors as mentioned in the issue.
cc: @jcaip @HDCharles @msaroufim