[ao] updating embedding_bag support for fx and eager #107623

HDCharles · 2023-08-21T20:06:36Z

Stack from ghstack (oldest at bottom):

Summary: our docs were saying dynamic embedding bag wasn't supported but
it actually is (at least at the same level as embeddings were) it just wasn't previously tested/listed.

Test Plan: python test/test_quantization.py -k "test_embedding"

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: D51489344

Summary: our docs were saying dynamic embedding bag wasn't supported but it actually is (at least at the same level as embeddings were) it just wasn't previously tested/listed. Test Plan: python test/test_quantization.py -k "test_embedding" Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

pytorch-bot · 2023-08-21T20:06:40Z

🔗 Helpful Links

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

📄 Preview Python docs built from this PR
📄 Preview C++ docs built from this PR
❓ Need help or want to give feedback on the CI? Visit the bot commands wiki or our office hours

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

✅ No Failures

As of commit e924738 with merge base d4189d8 ():
💚 Looks good so far! There are no failures yet. 💚

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

Summary: our docs were saying dynamic embedding bag wasn't supported but it actually is (at least at the same level as embeddings were) it just wasn't previously tested/listed. Test Plan: python test/test_quantization.py -k "test_embedding" Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: e8f2b82616a4a24b179a2fbfa61eb9dc5a56e96f Pull Request resolved: #107623

Summary: our docs were saying dynamic embedding bag wasn't supported but it actually is (at least at the same level as embeddings were) it just wasn't previously tested/listed. Test Plan: python test/test_quantization.py -k "test_embedding" Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

Summary: our docs were saying dynamic embedding bag wasn't supported but it actually is (at least at the same level as embeddings were) it just wasn't previously tested/listed. Test Plan: python test/test_quantization.py -k "test_embedding" Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 9dc1e2c339c2c4fa06ad46129a188a85fba37acd Pull Request resolved: #107623

Summary: our docs were saying dynamic embedding bag wasn't supported but it actually is (at least at the same level as embeddings were) it just wasn't previously tested/listed. Test Plan: python test/test_quantization.py -k "test_embedding" Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

Summary: our docs were saying dynamic embedding bag wasn't supported but it actually is (at least at the same level as embeddings were) it just wasn't previously tested/listed. Test Plan: python test/test_quantization.py -k "test_embedding" Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 7da407475089963becf7c04d95ae6cb9598ac240 Pull Request resolved: #107623

Summary: our docs were saying dynamic embedding bag wasn't supported but it actually is (at least at the same level as embeddings were) it just wasn't previously tested/listed. Test Plan: python test/test_quantization.py -k "test_embedding" Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

Summary: our docs were saying dynamic embedding bag wasn't supported but it actually is (at least at the same level as embeddings were) it just wasn't previously tested/listed. Test Plan: python test/test_quantization.py -k "test_embedding" Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 7da407475089963becf7c04d95ae6cb9598ac240 Pull Request resolved: #107623

Summary: our docs were saying dynamic embedding bag wasn't supported but it actually is (at least at the same level as embeddings were) it just wasn't previously tested/listed. Test Plan: python test/test_quantization.py -k "test_embedding" Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

Summary: our docs were saying dynamic embedding bag wasn't supported but it actually is (at least at the same level as embeddings were) it just wasn't previously tested/listed. Test Plan: python test/test_quantization.py -k "test_embedding" Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: f8f60b4c39368a3e39ef7a7f8d6828e410f69647 Pull Request resolved: #107623

jerryzh168 · 2023-08-22T00:14:30Z

docs/source/quantization.rst

@@ -129,7 +128,7 @@ Note that for FX quantization, the corresponding functionals are also supported.
 |nn.EmbeddingBag            | Y (activations    |                    |
 |                           | are in fp32)      | Y                  |
 +---------------------------+-------------------+--------------------+
-|nn.Embedding               | Y                 | N                  |
+|nn.Embedding               | Y                 | Y                  |


could you add ("float qparams weight only") in the Dynamic column:

Y (float qparams weight only)

isn't it fp32 activation for both embedding and embedding bag for both static and dynamic? It seems like it should be Y (float qparams weight only) for all 4 entries for both fx and eager mode.

jerryzh168 · 2023-08-22T00:14:35Z

docs/source/quantization.rst

@@ -810,7 +809,7 @@ Note that for FX Graph Mode Quantization, the corresponding functionals are also
 |nn.EmbeddingBag            | Y (activations    |                    |
 |                           | are in fp32)      | Y                  |
 +---------------------------+-------------------+--------------------+
-|nn.Embedding               | Y                 | N                  |
+|nn.Embedding               | Y                 | Y                  |


Summary: our docs were saying dynamic embedding bag wasn't supported but it actually is (at least at the same level as embeddings were) it just wasn't previously tested/listed. Test Plan: python test/test_quantization.py -k "test_embedding" Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

Summary: our docs were saying dynamic embedding bag wasn't supported but it actually is (at least at the same level as embeddings were) it just wasn't previously tested/listed. Test Plan: python test/test_quantization.py -k "test_embedding" Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: bc0a1e3a1c77f6620aa4ec5b95649a05214eeb53 Pull Request resolved: #107623

github-actions · 2023-10-22T22:33:46Z

Looks like this PR hasn't been updated in a while so we're going to go ahead and mark this as Stale.
Feel free to remove the Stale label if you feel this was a mistake.
If you are unable to remove the Stale label please contact a maintainer in order to do so.
If you want the bot to never mark this PR stale again, add the no-stale label.
Stale pull requests will automatically be closed after 30 days of inactivity.

Summary: our docs were saying dynamic embedding bag wasn't supported but it actually is (at least at the same level as embeddings were) it just wasn't previously tested/listed. Test Plan: python test/test_quantization.py -k "test_embedding" Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

HDCharles · 2023-11-18T00:32:43Z

@pytorchmergebot merge

pytorchmergebot · 2023-11-18T00:34:43Z

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

pytorchmergebot · 2023-11-18T01:43:03Z

Merge failed

Reason: 1 mandatory check(s) failed. The first few are:

Lint / lintrunner / linux-job

Dig deeper by viewing the failures on hud

Details for Dev Infra team

Raised by workflow job

Failing merge rule: Core Maintainers

Summary: our docs were saying dynamic embedding bag wasn't supported but it actually is (at least at the same level as embeddings were) it just wasn't previously tested/listed. Test Plan: python test/test_quantization.py -k "test_embedding" Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

HDCharles · 2023-11-21T03:51:59Z

@pytorchmergebot merge

pytorchmergebot · 2023-11-21T03:53:43Z

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

HDCharles · 2023-11-21T04:36:24Z

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

Summary: our docs were saying dynamic embedding bag wasn't supported but it actually is (at least at the same level as embeddings were) it just wasn't previously tested/listed. Test Plan: python test/test_quantization.py -k "test_embedding" Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 829622fa5a662f8cb1bab42bcaf463cf234288e8 Pull Request resolved: pytorch/pytorch#107623

HDCharles requested a review from jerryzh168 as a code owner August 21, 2023 20:06

pytorch-bot bot added the release notes: quantization release notes category label Aug 21, 2023

HDCharles requested review from vkuzo and supriyar August 21, 2023 20:31

HDCharles changed the title ~~[ao] updating embedding_bag support~~ [ao] updating embedding_bag support for fx and eager Aug 21, 2023

HDCharles mentioned this pull request Aug 21, 2023

[ao] making hist_obs handle torch.inf and closeby values #103467

Closed

jerryzh168 reviewed Aug 22, 2023

View reviewed changes

jerryzh168 approved these changes Aug 22, 2023

View reviewed changes

github-actions bot added the Stale label Oct 22, 2023

pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Nov 18, 2023

pytorchmergebot added the merging label Nov 18, 2023

pytorchmergebot removed the merging label Nov 18, 2023

pytorchmergebot added the merging label Nov 21, 2023

pytorchmergebot added the Merged label Nov 21, 2023

pytorchmergebot closed this in 18e1a37 Nov 21, 2023

pytorchmergebot removed the merging label Nov 21, 2023

facebook-github-bot deleted the gh/HDCharles/165/head branch November 24, 2023 15:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ao] updating embedding_bag support for fx and eager #107623

[ao] updating embedding_bag support for fx and eager #107623

HDCharles commented Aug 21, 2023 •

edited

pytorch-bot bot commented Aug 21, 2023 •

edited

jerryzh168 Aug 22, 2023

HDCharles Aug 23, 2023

jerryzh168 Aug 22, 2023

github-actions bot commented Oct 22, 2023

HDCharles commented Nov 18, 2023

pytorchmergebot commented Nov 18, 2023

pytorchmergebot commented Nov 18, 2023

HDCharles commented Nov 21, 2023

pytorchmergebot commented Nov 21, 2023

HDCharles commented Nov 21, 2023

[ao] updating embedding_bag support for fx and eager #107623

[ao] updating embedding_bag support for fx and eager #107623

Conversation

HDCharles commented Aug 21, 2023 • edited

pytorch-bot bot commented Aug 21, 2023 • edited

🔗 Helpful Links

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

✅ No Failures

jerryzh168 Aug 22, 2023

Choose a reason for hiding this comment

HDCharles Aug 23, 2023

Choose a reason for hiding this comment

jerryzh168 Aug 22, 2023

Choose a reason for hiding this comment

github-actions bot commented Oct 22, 2023

HDCharles commented Nov 18, 2023

pytorchmergebot commented Nov 18, 2023

Merge started

pytorchmergebot commented Nov 18, 2023

Merge failed

HDCharles commented Nov 21, 2023

pytorchmergebot commented Nov 21, 2023

Merge started

HDCharles commented Nov 21, 2023

HDCharles commented Aug 21, 2023 •

edited

pytorch-bot bot commented Aug 21, 2023 •

edited