Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

[Large Tensor] Fixed Embedding op #17599

Merged
merged 5 commits into from
Feb 25, 2020

Conversation

connorgoggins
Copy link
Contributor

@connorgoggins connorgoggins commented Feb 15, 2020

Description

The Embedding op was previously breaking on large tensor (dimension >= 2^32) data. With the following input:

nd.Embedding(data=nd.random_normal(shape=(2**32,1)), weight=nd.random_normal(shape=(2**32,1)), input_dim=2**32, output_dim=1)

the following error was thrown:

mxnet.base.MXNetError: MXNetError: Invalid Parameter format for input_dim expect int but value='4294967296', in operator Embedding(name="", output_dim="1", input_dim="4294967296")

To fix this issue, I modified indexing_op.h to switch from storing input_dim as an int to storing it as an index_t. After implementing my fix and rebuilding, the previous input command displayed the correct output:

[[[-0.5190417]]

 [[-1.4388928]]

 [[ 1.1367434]]

 ...

 [[-1.4388928]]

 [[-1.4388928]]

 [[-0.5190417]]]
<NDArray 4294967296x1x1 @cpu(0)>

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

  • Changes are complete (i.e. I finished coding on this PR)
  • All changes have test coverage
  • Code is well-documented
  • To the best of my knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Changes

  • M src/operator/tensor/indexing_op.h

Comments

Tested on r5dn.24xl-ubuntu 16.04 and p2.16xl-ubuntu 16.04 with

  1. Individual op run
  2. Full OpPerf run

Results

The key difference between CPU and GPU tests was the instance type (r5dn.24xl for CPU, p2.16xl for GPU). All relevant build flags remain the same, and both were tested using CPU context.

Single operator test - Embedding op (GPU)
Single operator test - Embedding op (CPU)

Full OpPerf test (GPU)
Full OpPerf test (CPU)

@apeforest @access2rohit @ChaiBapchya

@connorgoggins
Copy link
Contributor Author

@mxnet-label-bot add [pr-awaiting-review]

@lanking520 lanking520 added the pr-awaiting-review PR is waiting for code review label Feb 15, 2020
@access2rohit
Copy link
Contributor

access2rohit commented Feb 15, 2020

@connorgoggins can you paste output of a case where sparseEmbedding params are used ? Are both used in the same example use case presented here ?

@connorgoggins
Copy link
Contributor Author

@access2rohit great question. Here is an example of when sparseEmbedding params are used without my fix, and the resulting error (same error as with Embedding):

>>> mx.contrib.nd.SparseEmbedding(data=mx.nd.random_normal(shape=(2**32,1)), weight=mx.nd.random_normal(shape=(2**32, 1)), input_dim=2**32, output_dim=1)

mxnet.base.MXNetError: MXNetError: Invalid Parameter format for input_dim expect int but value='4294967296', in operator _contrib_SparseEmbedding(name="", output_dim="1", input_dim="4294967296")

With my fix, the call above passes without any issues - output below:

[[[-0.5190417]]

 [[-1.4388928]]

 [[ 1.1367434]]

 ...

 [[-1.4388928]]

 [[-1.4388928]]

 [[-0.5190417]]]
<NDArray 4294967296x1x1 @cpu(0)>

@@ -66,7 +66,7 @@ enum QuantizedEmbeddingOpResource {kTempSpace};


struct SparseEmbeddingParam: public dmlc::Parameter<SparseEmbeddingParam> {
int input_dim;
index_t input_dim;
int output_dim;
Copy link
Contributor

Choose a reason for hiding this comment

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

Would there be a case where output_dim also exceeds 2^32?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@apeforest excellent point, there would be. For example, the following call throws an error:

>>> mx.nd.Embedding(data=mx.nd.random_normal(shape=(1,)), weight=mx.nd.random_normal(shape=(1,2**32)), input_dim=1, output_dim=2**32)

mxnet.base.MXNetError: MXNetError: Invalid Parameter format for output_dim expect int but value='4294967296', in operator Embedding(name="", output_dim="4294967296", input_dim="1")

With my latest update (changing the dtype of output_dim to index_t), here is the result:

>>> mx.nd.Embedding(data=mx.nd.random_normal(shape=(1,)), weight=mx.nd.random_normal(shape=(1,2**32)), input_dim=1, output_dim=2**32)

[[ 1.6323917  -0.33354783 -1.7378405  ... -0.36648417  0.6363522
   2.367109  ]]
<NDArray 1x4294967296 @cpu(0)>

@apeforest
Copy link
Contributor

LGTM. Have you verified that your nightly test can run successfully in your local machine?

@connorgoggins connorgoggins changed the title Fixed Embedding op for LT input [Large Tensor] Fixed Embedding op Feb 20, 2020
@connorgoggins
Copy link
Contributor Author

@apeforest thanks! The individual nightly tests I wrote for each op run successfully on my local machine. Running the full nightly test suite for each op now.

@connorgoggins
Copy link
Contributor Author

connorgoggins commented Feb 21, 2020

@apeforest update: full suite of nightly tests passed on r5dn.24xl instances for every one of my PRs with additional tests added.

Copy link
Contributor

@apeforest apeforest left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the good work

@apeforest
Copy link
Contributor

@connorgoggins I retriggered CI tests

@apeforest apeforest merged commit d51753b into apache:master Feb 25, 2020
anirudh2290 pushed a commit to anirudh2290/mxnet that referenced this pull request May 29, 2020
* Switched from int to index_t for input_dim

* Implemented fix for output_dim

* Added nightly test for Embedding

* Set const value for output dim

* More standardization via const param
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pr-awaiting-review PR is waiting for code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants