Add unit tests for SOKEmbedding #980
Conversation
tox.ini
Outdated
| @@ -33,6 +33,7 @@ commands = | |||
| conda env create --prefix {envdir}/env --file requirements/horovod-cpu-environment.yml --force | |||
| {envdir}/env/bin/python -m pip install horovod --no-cache-dir | |||
| {envdir}/env/bin/horovodrun --check-build | |||
| {envdir}/env/bin/python -m pip install merlin-sok | |||
There was a problem hiding this comment.
Is merlin-sok the correct package to install?
There was a problem hiding this comment.
No, let me revise the code.
There was a problem hiding this comment.
@edknv I just realize the branch is on your repo, So I cannot modify it. the correct way to install sok is pip install sparse_operation_kit
| weights = np.random.rand(10, 16) | ||
| embedding = SOKEmbedding.from_pretrained(16, [weights]) | ||
| inputs = [tf.ragged.constant([[0, 1, 0], [1, 0]])] | ||
| combiners = ["sum"] |
There was a problem hiding this comment.
I think it would also make sense to have a sensible default for combiners and make this an optional parameter.
There was a problem hiding this comment.
@edknv Make sense. I will modify it
There was a problem hiding this comment.
Thanks. We can merge this PR to the main PR and apply the changes there.
| print("[SOK INFO] diff:", diff) | ||
| assert diff < 1e-6 | ||
| def test_sok_embedding_basic(self): | ||
| embedding = SOKEmbedding(16, self.sample_column_schema, vocab_sizes=[10]) |
There was a problem hiding this comment.
Can vocab_sizes be determined from the input schema?
There was a problem hiding this comment.
I think we can get from the input schema. But I don't know the right way to parse it.
There was a problem hiding this comment.
We can also skip it for now in this version, and iterate on it later in the next version.
|
@WonderingWJ I'll go ahead and merge this PR onto your PR since you can't make changes to this one. |
|
@edknv Thank you! |
* new sok class * new sok class * test sok dynamic variable * test sok dynamic variable * bug fix comma * add some comments and test distributed var * format the comments * assert condition in sok lookup sparse * Move SOKEmbedding to a separate file * Clean up * Clean up * fix some import and param bug * remove some unused variable * remove intial vals * fix import * reorder the import * fix import error in test embedding * format the code * change the way of import * support sp_weights in lookup * Add unit tests for SOKEmbedding (#980) * Add unit tests for SOKEmbedding * pip install sparse_operation_kit * remove sok from ci since no gpu * lint * pip install sparse_operation_kit in tox.ini * fix spelling * resolve init method issue * fix schema issue * add indices and weights in from_pretrained * schema issue in from_pretrained * init method in DET * tensor type in sok assing * resolve config passing * lint * install sok in tox --------- Co-authored-by: zhuwenjing <zhuwenjing@360.cn> Co-authored-by: wenjingz <wenjingz@nvidia.com> Co-authored-by: edknv <edwardk@nvidia.com> Co-authored-by: Marc Romeyn <marcromeyn@gmail.com> Co-authored-by: edknv <109497216+edknv@users.noreply.github.com> Co-authored-by: rnyak <ronayak@hotmail.com>
This PR adds unit tests for
SOKEmbedding, proposed in #863. Note: this PR is for thefea-sok-integration-wjbranch, not themainbranch.