Skip to content

Conversation

dzdang
Copy link
Contributor

@dzdang dzdang commented Dec 10, 2021

Stack from ghstack:

Summary: Support for the 4 embedding operator has been added. The support is analogous to the preexisting support for byte/8bit embedding. A corresponding test case was added to test_quantized_embedding_op.py

Reviewers: jerryzh168

Subscribers: jerryzh168, supriyar

Test plan: In pytorch main dir, execute

python test/test_quantization.py TestStaticQuantizedModule.test_embedding_api

to run the series of tests, including the newly added test_embedding_4bit
function

Tasks: T106931792

Tags

Differential Revision: D33152673

…est case

Summary: Support for the 4 embedding operator has been added. The support is analogous to the preexisting support for byte/8bit embedding. A corresponding test case was added to test_quantized_embedding_op.py

Reviewers: jerryzh168

Subscribers: jerryzh168, supriyar

Test plan: In pytorch main dir, execute
```
python test/test_quantization.py TestStaticQuantizedModule.test_embedding_api
```
to run the series of tests, including the newly added test_embedding_4bit
function

Tasks: T106931792

Tags

[ghstack-poisoned]
@pytorch-probot
Copy link

pytorch-probot bot commented Dec 10, 2021

CI Flow Status

⚛️ CI Flow

Ruleset - Version: v1
Ruleset - File: https://github.com/pytorch/pytorch/blob/b908918a706e0fc2d654d12d32c79f3b7359c5d0/.github/generated-ciflow-ruleset.json
PR ciflow labels: ciflow/default

Workflows Labels (bold enabled) Status
Triggered Workflows
linux-bionic-py3.6-clang9 ciflow/all, ciflow/cpu, ciflow/default, ciflow/linux, ciflow/noarch, ciflow/xla ✅ triggered
linux-docs ciflow/all, ciflow/cpu, ciflow/default, ciflow/docs, ciflow/linux ✅ triggered
linux-vulkan-bionic-py3.6-clang9 ciflow/all, ciflow/cpu, ciflow/default, ciflow/linux, ciflow/vulkan ✅ triggered
linux-xenial-cuda11.3-py3.6-gcc7 ciflow/all, ciflow/cuda, ciflow/default, ciflow/linux ✅ triggered
linux-xenial-py3-clang5-mobile-build ciflow/all, ciflow/default, ciflow/linux, ciflow/mobile ✅ triggered
linux-xenial-py3-clang5-mobile-custom-build-static ciflow/all, ciflow/default, ciflow/linux, ciflow/mobile ✅ triggered
linux-xenial-py3.6-clang7-asan ciflow/all, ciflow/cpu, ciflow/default, ciflow/linux, ciflow/sanitizers ✅ triggered
linux-xenial-py3.6-clang7-onnx ciflow/all, ciflow/cpu, ciflow/default, ciflow/linux, ciflow/onnx ✅ triggered
linux-xenial-py3.6-gcc5.4 ciflow/all, ciflow/cpu, ciflow/default, ciflow/linux ✅ triggered
linux-xenial-py3.6-gcc7 ciflow/all, ciflow/cpu, ciflow/default, ciflow/linux ✅ triggered
linux-xenial-py3.6-gcc7-bazel-test ciflow/all, ciflow/bazel, ciflow/cpu, ciflow/default, ciflow/linux ✅ triggered
pytorch-linux-xenial-py3-clang5-android-ndk-r19c-gradle-custom-build-single ciflow/all, ciflow/android, ciflow/cpu, ciflow/default, ciflow/linux ✅ triggered
pytorch-linux-xenial-py3-clang5-android-ndk-r19c-gradle-custom-build-single-full-jit ciflow/all, ciflow/android, ciflow/cpu, ciflow/default, ciflow/linux ✅ triggered
win-vs2019-cpu-py3 ciflow/all, ciflow/cpu, ciflow/default, ciflow/win ✅ triggered
win-vs2019-cuda11.3-py3 ciflow/all, ciflow/cuda, ciflow/default, ciflow/win ✅ triggered
Skipped Workflows
caffe2-linux-xenial-py3.6-gcc5.4 ciflow/all, ciflow/cpu, ciflow/linux 🚫 skipped
docker-builds ciflow/all 🚫 skipped
ios-12-5-1-arm64 ciflow/all, ciflow/ios, ciflow/macos 🚫 skipped
ios-12-5-1-arm64-coreml ciflow/all, ciflow/ios, ciflow/macos 🚫 skipped
ios-12-5-1-arm64-custom-ops ciflow/all, ciflow/ios, ciflow/macos 🚫 skipped
ios-12-5-1-arm64-full-jit ciflow/all, ciflow/ios, ciflow/macos 🚫 skipped
ios-12-5-1-arm64-metal ciflow/all, ciflow/ios, ciflow/macos 🚫 skipped
ios-12-5-1-x86-64 ciflow/all, ciflow/ios, ciflow/macos 🚫 skipped
ios-12-5-1-x86-64-coreml ciflow/all, ciflow/ios, ciflow/macos 🚫 skipped
ios-12-5-1-x86-64-full-jit ciflow/all, ciflow/ios, ciflow/macos 🚫 skipped
libtorch-linux-xenial-cuda10.2-py3.6-gcc7 ciflow/all, ciflow/cuda, ciflow/libtorch, ciflow/linux 🚫 skipped
libtorch-linux-xenial-cuda11.3-py3.6-gcc7 ciflow/all, ciflow/cuda, ciflow/libtorch, ciflow/linux 🚫 skipped
linux-bionic-cuda10.2-py3.9-gcc7 ciflow/all, ciflow/cuda, ciflow/linux, ciflow/slow 🚫 skipped
linux-docs-push ciflow/all, ciflow/cpu, ciflow/linux, ciflow/scheduled 🚫 skipped
macos-10-15-py3-arm64 ciflow/all, ciflow/macos 🚫 skipped
macos-10-15-py3-lite-interpreter-x86-64 ciflow/all, ciflow/macos 🚫 skipped
macos-11-py3-x86-64 ciflow/all, ciflow/macos 🚫 skipped
parallelnative-linux-xenial-py3.6-gcc5.4 ciflow/all, ciflow/cpu, ciflow/linux 🚫 skipped
periodic-libtorch-linux-bionic-cuda11.5-py3.6-gcc7 ciflow/all, ciflow/cuda, ciflow/libtorch, ciflow/linux, ciflow/scheduled 🚫 skipped
periodic-libtorch-linux-xenial-cuda11.1-py3.6-gcc7 ciflow/all, ciflow/cuda, ciflow/libtorch, ciflow/linux, ciflow/scheduled 🚫 skipped
periodic-linux-bionic-cuda11.5-py3.6-gcc7 ciflow/all, ciflow/cuda, ciflow/linux, ciflow/scheduled 🚫 skipped
periodic-linux-xenial-cuda10.2-py3-gcc7-slow-gradcheck ciflow/all, ciflow/cuda, ciflow/linux, ciflow/scheduled, ciflow/slow, ciflow/slow-gradcheck 🚫 skipped
periodic-linux-xenial-cuda11.1-py3.6-gcc7-debug ciflow/all, ciflow/cuda, ciflow/linux, ciflow/scheduled 🚫 skipped
periodic-win-vs2019-cuda11.1-py3 ciflow/all, ciflow/cuda, ciflow/scheduled, ciflow/win 🚫 skipped
periodic-win-vs2019-cuda11.5-py3 ciflow/all, ciflow/cuda, ciflow/scheduled, ciflow/win 🚫 skipped

You can add a comment to the PR and tag @pytorchbot with the following commands:
# ciflow rerun, "ciflow/default" will always be added automatically
@pytorchbot ciflow rerun

# ciflow rerun with additional labels "-l <ciflow/label_name>", which is equivalent to adding these labels manually and trigger the rerun
@pytorchbot ciflow rerun -l ciflow/scheduled -l ciflow/slow

For more information, please take a look at the CI Flow Wiki.

dzdang added a commit that referenced this pull request Dec 10, 2021
…est case

Summary: Support for the 4 embedding operator has been added. The support is analogous to the preexisting support for byte/8bit embedding. A corresponding test case was added to test_quantized_embedding_op.py

Reviewers: jerryzh168

Subscribers: jerryzh168, supriyar

Test plan: In pytorch main dir, execute
```
python test/test_quantization.py TestStaticQuantizedModule.test_embedding_api
```
to run the series of tests, including the newly added test_embedding_4bit
function

Tasks: T106931792

Tags

ghstack-source-id: 318c8b8
Pull Request resolved: #69768
@dzdang dzdang requested a review from jerryzh168 December 10, 2021 17:44
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Dec 11, 2021

🔗 Helpful links

💊 CI failures summary and remediations

As of commit b908918 (more details on the Dr. CI page):


  • 1/1 failures possibly* introduced in this PR
    • 1/1 non-scanned failure(s)

ci.pytorch.org: 1 failed


This comment was automatically generated by Dr. CI (expand for details).

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

…esponding test case"

Summary: Support for the 4 embedding operator has been added. The support is analogous to the preexisting support for byte/8bit embedding. A corresponding test case was added to test_quantized_embedding_op.py

Reviewers: jerryzh168

Subscribers: jerryzh168, supriyar

Test plan: In pytorch main dir, execute
```
python test/test_quantization.py TestStaticQuantizedModule.test_embedding_api
```
to run the series of tests, including the newly added test_embedding_4bit
function

Tasks: T106931792

Tags

[ghstack-poisoned]
torch.testing.assert_close(ref, qresult, atol=0.005, rtol=1e-3)

""" Tests the correctness of the quantized 4 bit embedding lookup operator """
@given(num_embeddings=st.integers(10, 100),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it possible to merge this test with test_embedding_byte?
also we don't want to use hypothesis for testing, might be good to remove hypothesis here as well and change it to simple for loops. (remove @given, example: https://github.com/pytorch/pytorch/blob/master/test/quantization/core/test_quantized_op.py#L1430), can be done in a separate PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean removing hypothesis for both test_embedding_byte and test_embedding_4bit or just the latter? It was already there in test_embedding_byte, so I thought I was supposed to add it.

And yeah, I can easily combine test_embedding_byte and test_embedding_4bit. Is it acceptable to rename the function to something like test_embedding?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean removing hypothesis for both test_embedding_byte and test_embedding_4bit or just the latter? It was already there in test_embedding_byte, so I thought I was supposed to add it.

we can combine the tests and remove hypothesis, it can be in a separate PR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good. I'll put that in PR5

Copy link
Contributor

@jerryzh168 jerryzh168 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good, had a comment for test

…esponding test case"

Summary: Support for the 4 embedding operator has been added. The support is analogous to the preexisting support for byte/8bit embedding. A corresponding test case was added to test_quantized_embedding_op.py

Reviewers: jerryzh168

Subscribers: jerryzh168, supriyar

Test plan: In pytorch main dir, execute
```
python test/test_quantization.py TestStaticQuantizedModule.test_embedding_api
```
to run the series of tests, including the newly added test_embedding_4bit
function

Tasks: T106931792

Tags

[ghstack-poisoned]
…esponding test case"

Summary: Support for the 4 embedding operator has been added. The support is analogous to the preexisting support for byte/8bit embedding. A corresponding test case was added to test_quantized_embedding_op.py

Reviewers: jerryzh168

Subscribers: jerryzh168, supriyar

Test plan: In pytorch main dir, execute
```
python test/test_quantization.py TestStaticQuantizedModule.test_embedding_api
```
to run the series of tests, including the newly added test_embedding_4bit
function

Tasks: T106931792

Tags

[ghstack-poisoned]
@jerryzh168
Copy link
Contributor

please import the PR with ghimport

…esponding test case"

Summary: Support for the 4 embedding operator has been added. The support is analogous to the preexisting support for byte/8bit embedding. A corresponding test case was added to test_quantized_embedding_op.py

Reviewers: jerryzh168

Subscribers: jerryzh168, supriyar

Test plan: In pytorch main dir, execute
```
python test/test_quantization.py TestStaticQuantizedModule.test_embedding_api
```
to run the series of tests, including the newly added test_embedding_4bit
function

Tasks: T106931792

Tags

[ghstack-poisoned]
…esponding test case"

Summary: Support for the 4 embedding operator has been added. The support is analogous to the preexisting support for byte/8bit embedding. A corresponding test case was added to test_quantized_embedding_op.py

Reviewers: jerryzh168

Subscribers: jerryzh168, supriyar

Test plan: In pytorch main dir, execute
```
python test/test_quantization.py TestStaticQuantizedModule.test_embedding_api
```
to run the series of tests, including the newly added test_embedding_4bit
function

Tasks: T106931792

Tags

[ghstack-poisoned]
@dzdang
Copy link
Contributor Author

dzdang commented Dec 16, 2021

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

@facebook-github-bot facebook-github-bot deleted the gh/dzdang/12/head branch December 22, 2021 15:16
dzdang added a commit that referenced this pull request Jan 17, 2022
dzdang added a commit that referenced this pull request Jan 17, 2022
dzdang added a commit that referenced this pull request Jan 21, 2022
dzdang added a commit that referenced this pull request Jan 21, 2022
dzdang added a commit that referenced this pull request Jan 27, 2022
dzdang added a commit that referenced this pull request Jan 27, 2022
dzdang added a commit that referenced this pull request Jan 27, 2022
dzdang added a commit that referenced this pull request Jan 27, 2022
dzdang added a commit that referenced this pull request Jan 28, 2022
dzdang added a commit that referenced this pull request Jan 28, 2022
dzdang added a commit that referenced this pull request Jan 28, 2022
dzdang added a commit that referenced this pull request Jan 28, 2022
dzdang added a commit that referenced this pull request Jan 28, 2022
facebook-github-bot pushed a commit that referenced this pull request Jan 28, 2022
Summary: Pull Request resolved: #71387

Test Plan: Imported from OSS

Reviewed By: albanD

Differential Revision: D33808593

Pulled By: dzdang

fbshipit-source-id: 3950400dc7506006666fcd055819e9a08a42eda9
pytorchmergebot pushed a commit that referenced this pull request Jan 28, 2022
Summary: Pull Request resolved: #71387

Test Plan: Imported from OSS

Reviewed By: albanD

Differential Revision: D33808593

Pulled By: dzdang

fbshipit-source-id: 3950400dc7506006666fcd055819e9a08a42eda9
(cherry picked from commit 38dc2de)
cyyever pushed a commit to cyyever/pytorch_private that referenced this pull request Feb 3, 2022
…69768 (#71387)

Summary: Pull Request resolved: pytorch/pytorch#71387

Test Plan: Imported from OSS

Reviewed By: albanD

Differential Revision: D33808593

Pulled By: dzdang

fbshipit-source-id: 3950400dc7506006666fcd055819e9a08a42eda9
(cherry picked from commit 38dc2de)
cyyever pushed a commit to cyyever/pytorch_private that referenced this pull request Feb 3, 2022
…69768 (#71387)

Summary: Pull Request resolved: pytorch/pytorch#71387

Test Plan: Imported from OSS

Reviewed By: albanD

Differential Revision: D33808593

Pulled By: dzdang

fbshipit-source-id: 3950400dc7506006666fcd055819e9a08a42eda9
(cherry picked from commit 38dc2de)
cyyever pushed a commit to cyyever/pytorch_private that referenced this pull request Feb 9, 2022
…69768 (#71387)

Summary: Pull Request resolved: pytorch/pytorch#71387

Test Plan: Imported from OSS

Reviewed By: albanD

Differential Revision: D33808593

Pulled By: dzdang

fbshipit-source-id: 3950400dc7506006666fcd055819e9a08a42eda9
(cherry picked from commit 38dc2de)
cyyever pushed a commit to cyyever/pytorch_private that referenced this pull request Feb 9, 2022
…69768 (#71387)

Summary: Pull Request resolved: pytorch/pytorch#71387

Test Plan: Imported from OSS

Reviewed By: albanD

Differential Revision: D33808593

Pulled By: dzdang

fbshipit-source-id: 3950400dc7506006666fcd055819e9a08a42eda9
(cherry picked from commit 38dc2de)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants