[MXNET-1121] Example to demonstrate the inference workflow using RNN #13680

leleamol · 2018-12-18T23:46:42Z

Description

Example to demonstrate the inference workflow using RNN and C++ APIs

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

[y] The PR title starts with [MXNET-$JIRA_ID], where $JIRA_ID refers to the relevant JIRA issue created (except PRs with tiny changes)
[y] Changes are complete (i.e. I finished coding on this PR)
[y] All changes have test coverage:
Unit tests are added for small changes to verify correctness (e.g. adding a new operator)
Nightly tests are added for complicated/long-running ones (e.g. changing distributed kvstore)
Build tests will be added for build configuration changes (e.g. adding a new build option with NCCL)
[y] Code is well-documented:
For user-facing API changes, API doc string has been updated.
For new C++ functions in header files, their functionalities and arguments are documented.
For new examples, README.md is added to explain the what the example does, the source of the dataset, expected performance on test set and reference to the original paper if applicable
Check the API doc at http://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-$PR_ID/$BUILD_ID/index.html
[y] To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Changes

Feature1, tests, (and when applicable, API doc)
Feature2, tests, (and when applicable, API doc)

Comments

The example demonstrates inference workflow using pre-trained RNN network and C++ APIs.

Roshrini · 2018-12-19T00:32:29Z

@mxnet-label-bot Add [Example, pr-awaiting-review]

Roshrini · 2019-01-02T20:26:39Z

@nswamy @eric-haibin-lin For review

lupesko

Left a bunch of comments.
Beyond them:

I suggest you add a paragraph to the readme explaining just a bit about the model, at a high level: what is the input for inference, what is the output, and how it was trained.
Write down an example of the model's output on a given input as part of the readme, as a simple way to explain what the model does and what the readers should expect.

cpp-package/example/inference/README.md

cpp-package/example/inference/simple_rnn.cpp

lupesko · 2019-01-08T07:36:30Z

@leleamol - I left a few comments. On the PR description, I suggest you remove the bullets/items that are not relevant for this PR (under Essentials and Changes).

…to rnn-inference

leleamol · 2019-01-09T20:03:11Z

@lupesko
Thanks for the review comments. I have updated the code as per your comments.

Also I have edited the README files with some details about the model as well as instruction to build cpp package before building the examples.

leleamol · 2019-01-09T20:32:43Z

@sandeep-krishnamurthy

cpp-package/example/inference/simple_rnn.cpp

sandeep-krishnamurthy

Thanks for working on this, few high level comments for now.

Are we adding this example to nightly tests? so we don't have this example broken and unnoticed in future.

sandeep-krishnamurthy · 2019-01-09T23:14:25Z

cpp-package/example/inference/README.md

+
+The example uses a pre-trained RNN model that is trained with the dataset containing speeches given by Obama.
+The model consists of :
+- Embedding Layer with the size of embedding to be 650


A simple image will be helpful

I can get the pdf version of model (generated using visualiation API). Need some suggestions from @aaronmarkham to embed them in README.

sandeep-krishnamurthy · 2019-01-09T23:15:28Z

cpp-package/example/inference/README.md

@@ -39,3 +39,48 @@ Alternatively, The script [unit_test_inception_inference.sh](<https://github.com
 ```
 ./unit_test_inception_inference.sh
 ```
+
+### [simple_rnn.cpp](<https://github.com/apache/incubator-mxnet/blob/master/cpp-package/example/inference/simple_rnn.cpp>)
+This example demonstrates sequence prediction workflow with pre-trained RNN model using MXNet C++ API. The purpose of this example is to demonstrate how a pre-trained RNN model can be loaded and used to generate an output sequence using C++ API.


Probably can be made more simple for making it easier to read and follow.
@aaronmarkham - Can you please help us here with the documentation? Thanks.

cpp-package/example/inference/README.md

sandeep-krishnamurthy · 2019-01-09T23:18:06Z

cpp-package/example/inference/README.md

+
+The example will output the seqence of 35 words as follows:
+```
+[waters elected Amendment Amendment Amendment Amendment retirement maximize maximize maximize acr citi sophisticatio sophisticatio sophisticatio sophisticatio sophisticatio sophisticatio sophisticatio sophisticatio sophisticatio sophisticatio sophisticatio sophisticatio sophisticatio sophisticatio sophisticatio sophisticatio sophisticatio sophisticatio sophisticatio sophisticatio sophisticatio sophisticatio sophisticatio ]


Why is the output bad? Can we give example of a well trained model?

I tried to get better output by changing model hyperparameters but couldn't get it. It would require a good amount of input data processing as well. All these efforts would require dedicated time and out of scope for this example. The example aims towards loading the model and running forward pass. Improving on the model would be a separate task.

I am working on implementing the RNN model using C++ API. I can work on improving the accuracy of that model and use it in this example later.

sandeep-krishnamurthy · 2019-01-09T23:22:18Z

cpp-package/example/inference/simple_rnn.cpp

+    throw std::runtime_error("Model parameters does not exist");
+  }
+  LG << "Loading the model parameters from " << model_parameters_file << std::endl;
+  std::map<std::string, NDArray> paramters;


nit: spell check

cpp-package/example/inference/simple_rnn.cpp

sandeep-krishnamurthy · 2019-01-09T23:27:01Z

cpp-package/example/inference/README.md

+
+The model files can be found here.
+- [obama-speaks-symbol.json](<https://s3.amazonaws.com/mxnet-cpp/RNN_model/obama-speaks-symbol.json>)
+- [obama-speaks-0100.params](<https://s3.amazonaws.com/mxnet-cpp/RNN_model/obama-speaks-0100.params>)


Why in mxnet-cpp bucket why not in mxnet pretrained models bucket?

The current bucket mxnet-cpp is not a public by default but the contents are made publicly readable. This is similar to mxnet-scala bucket used for scala examples.

cpp-package/example/inference/simple_rnn.cpp

leleamol · 2019-01-10T23:04:32Z

@sandeep-krishnamurthy as per the comment, I have added these examples to ci_test.sh file that runs in nightly builds.

piyushghai · 2019-01-11T18:45:34Z

cpp-package/example/inference/simple_rnn.cpp

+
+
+int main(int argc, char** argv) {
+  std::string model_file_json = "./obama-speaks-symbol.json";


Should the input file paths be hardcoded here ?
Can they be passed as parameters to the main method in argv ?

My bad, saw later that these are passed to the S3 Bucket to download.

…nd tested in nightly

cpp-package/example/inference/README.md

IvyBazan

Please see suggested improvements for clarity

cpp-package/example/inference/README.md

cpp-package/example/inference/simple_rnn.cpp

Co-Authored-By: leleamol <[email protected]>

cpp-package/example/inference/unit_test_sentiment_analysis_rnn.sh

cpp-package/example/inference/sentiment_analysis_rnn.cpp

cpp-package/example/inference/README.md

cpp-package/example/inference/sentiment_analysis_rnn.cpp

Isa-rentacs · 2019-01-28T08:10:25Z

@leleamol
Regarding variable length inputs, isn't it possible to update args_map["data0"] depending on the length of the input sentence?

…me and unit test files accordingly.

leleamol · 2019-01-28T20:48:52Z

@szha @Isa-rentacs @ddavydenko @lupesko @sandeep-krishnamurthy

I have updated the example that can handle the variable length input. The readme and unit test files are also updated accordingly.

IvyBazan · 2019-01-29T00:25:46Z

LGTM!

szha · 2019-01-29T01:20:46Z

cpp-package/example/inference/sentiment_analysis_rnn.cpp

+    Executor* executor = net.SimpleBind(global_ctx, args_map, std::map<std::string, NDArray>(),
+                                std::map<std::string, OpReqType>(), aux_map);
+    executor_buckets[bucket] = executor;
+  }


This won't share the memory across executors, but will allocate excessive space from which only 1 / bucket_keys.Size() is used at any time.

@szha ,

I updated the example to create shared executors.

Do you have to make sure that the master executor is created with the largest bucket size? If not, wouldn't the other executors need to allocate additional memory since the master executor only allocated enough for the smaller shape?

It looks like you are looping in whatever order bucket_keys is, and it looks like you are starting from smallest and going to largest bucket size.

(@szha -- you might know better than me. I know in the executor Reshape() call this is true. I would assume it must be true in the BindEX call that is being made here, but not 100% sure)

The shared executors that I am creating are for the keys smaller to largest.
However, I noticed that the results do not change or example does not crash even if I reverse the order.
I had single stepped through the code, the GraphExecutor::InitDataEntryMemory() uses the shared memory from the given executor.

@leleamol -- the question I had is not weather it will effect the correctness or cause your program to crash. I am wondering weather the current order ends up allocating more memory than you need.

I don't know for sure, but I suspect if you allocate the largest executor first, then all the rest of the executors don't have to allocate extra memory and they just share memory from the largest executor.

However, I would assume that if the smallest executor is the "master" executor, then all other executors would have to allocate extra memory because the master executor did not allocate enough to satisfy any of the other executors.

Can you re-run your test w/ changing the order, and look closely at total gpu memory allocated?

@stephenrawls
I tried running it on CPU and I see an improvement in the memory consumption when the order is reveresed. That is creating the executor with largest bucket first.
I have updated the PR with that change.

Great, thanks for confirming.

Just looked at your fix though.

Instead of changing the order of your default array, you should probably just have a check to see what the max element of your array is, and allocate that first.

Otherwise you have a hidden assumption that the bucket sizes should be in a specific order, and it is easy for users not to realize that.

@stephenrawls
I agree, it will imply that the order is important.
Fixed the code in latest update.

Awesome, it looks ready to go to me. Thanks. 👍

Isa-rentacs

Looks good to me, again thank you for working on this 👍

cpp-package/example/inference/sentiment_analysis_rnn.cpp

…g shared executors.

vdantu · 2019-01-31T22:27:01Z

Talked to @leleamol . This PR Fixes #12734

stephenrawls · 2019-02-02T21:30:08Z

Looks good to me, thanks for the example. Made a few small comments in the code about sharing executor memory, and extracting model output.

It would probably be nice to showcase batching too, but I know that complicates the current example code & bucketing executor, so not really a requirement! Thanks again for getting an example of doing inference in the c++ api.

szha · 2019-02-06T18:50:54Z

cpp-package/example/inference/sentiment_analysis_rnn.cpp

+ * If the next larger bucket does not exist, function looks for the previous bucket key
+ * that is lesser than given num_words.
+ */
+int Predictor::GetClosestBucketKey(int num_words) {


it seems that you can simply use lower_bound and return the largest key if not found.

If we use lower_bound, we will always find a bucket that is smaller than the input word length (unless the input word length is smaller than the smallest bucket size). In that case, we will trim the input line and may lose the information. Therefore, I chose to find the next largest bucket so that we will be able to consider all the words in input sentence.

If we use lower_bound, we will always find a bucket that is smaller than the input word length

I'm not sure if this is true. From cpprefenrece on std::map::lower_bound:

Returns an iterator pointing to the first element that is not less than key.

In your case the key is input word length.

@szha I agree, with lower_bounds we will get matching or a smaller bucket key. My concern is that, we will always find such bucket and we will trim the input sequence. In that process, we would lose some of the information. Will it be acceptable?

Unless I read the document wrong, it won't get you smaller key. lower_bound returns the first element (i.e. bucket key) that's no less than (i.e. larger or equal to) the key (i.e. input sequence length).

For input sequence that's larger than the largest key, clipping is necessary.

@szha Fixed the logic in the latest update. Getting the lower bound is sufficient and accurate.
Thanks!

cpp-package/example/inference/sentiment_analysis_rnn.cpp

cpp-package/example/inference/unit_test_sentiment_analysis_rnn.sh

The concern around sharing memory across executors is addressed.

…d the function name to be consistent with others.

sandeep-krishnamurthy · 2019-02-07T17:40:32Z

Thanks Amol and all reviewers for this contribution.
Merging the changes as all reviewer comments are addressed and tests have passed.

…pache#13680) * [MXNET-1121] Example to demonstrate the inference workflow using RNN * Addressed the review comments. Updated the ReadMe files. * Removed the unnecessary creation of NDArray * Added the unit tests to nightly tests to catch the failure. * Updated the makefiles and unit tests so that the examples are built and tested in nightly * Added the visual representation of the model and fixed the CI failure. * Added the missing pdf file. * Fixing the broken ci_test.sh * Update cpp-package/example/inference/README.md Co-Authored-By: leleamol <[email protected]> * Update cpp-package/example/inference/README.md Co-Authored-By: leleamol <[email protected]> * Update cpp-package/example/inference/README.md Co-Authored-By: leleamol <[email protected]> * Update cpp-package/example/inference/README.md Co-Authored-By: leleamol <[email protected]> * Update cpp-package/example/inference/README.md Co-Authored-By: leleamol <[email protected]> * Update cpp-package/example/inference/simple_rnn.cpp Co-Authored-By: leleamol <[email protected]> * Update cpp-package/example/inference/simple_rnn.cpp Co-Authored-By: leleamol <[email protected]> * Update cpp-package/example/inference/simple_rnn.cpp Co-Authored-By: leleamol <[email protected]> * Update cpp-package/example/inference/simple_rnn.cpp Co-Authored-By: leleamol <[email protected]> * Update cpp-package/example/inference/simple_rnn.cpp Co-Authored-By: leleamol <[email protected]> * Update cpp-package/example/inference/README.md Co-Authored-By: leleamol <[email protected]> * Update cpp-package/example/inference/README.md Co-Authored-By: leleamol <[email protected]> * Update cpp-package/example/inference/README.md Co-Authored-By: leleamol <[email protected]> * Update cpp-package/example/inference/simple_rnn.cpp Co-Authored-By: leleamol <[email protected]> * Update cpp-package/example/inference/simple_rnn.cpp Co-Authored-By: leleamol <[email protected]> * Applying unresolved changes to README file. * Fixing the CI build failure. * Updated the RNN example from sequence generation to sentiment analysis * Updated the readme files. Updated the example to use trained model and updated the unit test. * Addressed the review comment to increase the default sequence length. Added the examples with inputs of various lengths. * Updated the example to handle variable length input. Updated the readme and unit test files accordingly. * Updated the example to share the memory between executors by createing shared executors. * Updated the creation of executors from largest to smallest bucket key * Creating the executor for the highest bucket key. * Updated the unit test to check for the results in a range and modified the function name to be consistent with others. * Fixed the logic to find the right bucket.

leleamol · 2019-06-10T18:13:48Z

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

…pache#13680) * [MXNET-1121] Example to demonstrate the inference workflow using RNN * Addressed the review comments. Updated the ReadMe files. * Removed the unnecessary creation of NDArray * Added the unit tests to nightly tests to catch the failure. * Updated the makefiles and unit tests so that the examples are built and tested in nightly * Added the visual representation of the model and fixed the CI failure. * Added the missing pdf file. * Fixing the broken ci_test.sh * Update cpp-package/example/inference/README.md Co-Authored-By: leleamol <[email protected]> * Update cpp-package/example/inference/README.md Co-Authored-By: leleamol <[email protected]> * Update cpp-package/example/inference/README.md Co-Authored-By: leleamol <[email protected]> * Update cpp-package/example/inference/README.md Co-Authored-By: leleamol <[email protected]> * Update cpp-package/example/inference/README.md Co-Authored-By: leleamol <[email protected]> * Update cpp-package/example/inference/simple_rnn.cpp Co-Authored-By: leleamol <[email protected]> * Update cpp-package/example/inference/simple_rnn.cpp Co-Authored-By: leleamol <[email protected]> * Update cpp-package/example/inference/simple_rnn.cpp Co-Authored-By: leleamol <[email protected]> * Update cpp-package/example/inference/simple_rnn.cpp Co-Authored-By: leleamol <[email protected]> * Update cpp-package/example/inference/simple_rnn.cpp Co-Authored-By: leleamol <[email protected]> * Update cpp-package/example/inference/README.md Co-Authored-By: leleamol <[email protected]> * Update cpp-package/example/inference/README.md Co-Authored-By: leleamol <[email protected]> * Update cpp-package/example/inference/README.md Co-Authored-By: leleamol <[email protected]> * Update cpp-package/example/inference/simple_rnn.cpp Co-Authored-By: leleamol <[email protected]> * Update cpp-package/example/inference/simple_rnn.cpp Co-Authored-By: leleamol <[email protected]> * Applying unresolved changes to README file. * Fixing the CI build failure. * Updated the RNN example from sequence generation to sentiment analysis * Updated the readme files. Updated the example to use trained model and updated the unit test. * Addressed the review comment to increase the default sequence length. Added the examples with inputs of various lengths. * Updated the example to handle variable length input. Updated the readme and unit test files accordingly. * Updated the example to share the memory between executors by createing shared executors. * Updated the creation of executors from largest to smallest bucket key * Creating the executor for the highest bucket key. * Updated the unit test to check for the results in a range and modified the function name to be consistent with others. * Fixed the logic to find the right bucket.

[MXNET-1121] Example to demonstrate the inference workflow using RNN

9519a2d

leleamol requested a review from nswamy as a code owner December 18, 2018 23:46

marcoabreu added Example pr-awaiting-review PR is waiting for code review labels Dec 19, 2018

leleamol mentioned this pull request Jan 4, 2019

Example of RNN (incl. GRU/LSTM) with APIs defined in c_predict_api.h #12734

Closed

lupesko suggested changes Jan 8, 2019

View reviewed changes

leleamol added 2 commits January 9, 2019 10:55

Merge branch 'master' of https://github.com/apache/incubator-mxnet in…

8c06090

…to rnn-inference

Addressed the review comments. Updated the ReadMe files.

7a341f9

piyushghai reviewed Jan 9, 2019

View reviewed changes

cpp-package/example/inference/simple_rnn.cpp Outdated Show resolved Hide resolved

sandeep-krishnamurthy reviewed Jan 9, 2019

View reviewed changes

leleamol added 2 commits January 9, 2019 15:37

Removed the unnecessary creation of NDArray

c19760a

Added the unit tests to nightly tests to catch the failure.

438c3c4

piyushghai reviewed Jan 11, 2019

View reviewed changes

leleamol added 2 commits January 11, 2019 11:27

Updated the makefiles and unit tests so that the examples are built a…

c2f7a67

…nd tested in nightly

Added the visual representation of the model and fixed the CI failure.

768fe5e

piyushghai reviewed Jan 11, 2019

View reviewed changes

cpp-package/example/inference/README.md Outdated Show resolved Hide resolved

IvyBazan suggested changes Jan 11, 2019

View reviewed changes

leleamol and others added 7 commits January 11, 2019 16:14

Added the missing pdf file.

d8abf83

Fixing the broken ci_test.sh

fd33d22

Update cpp-package/example/inference/README.md

e31a5bd

Co-Authored-By: leleamol <[email protected]>

Update cpp-package/example/inference/README.md

c53d329

Co-Authored-By: leleamol <[email protected]>

Update cpp-package/example/inference/README.md

9d315a2

Co-Authored-By: leleamol <[email protected]>

Update cpp-package/example/inference/README.md

c774f3c

Co-Authored-By: leleamol <[email protected]>

Update cpp-package/example/inference/README.md

8d88feb

Co-Authored-By: leleamol <[email protected]>

Isa-rentacs reviewed Jan 28, 2019

View reviewed changes

cpp-package/example/inference/unit_test_sentiment_analysis_rnn.sh Outdated Show resolved Hide resolved

Isa-rentacs reviewed Jan 28, 2019

View reviewed changes

cpp-package/example/inference/unit_test_sentiment_analysis_rnn.sh Outdated Show resolved Hide resolved

Isa-rentacs reviewed Jan 28, 2019

View reviewed changes

cpp-package/example/inference/sentiment_analysis_rnn.cpp Show resolved Hide resolved

Isa-rentacs reviewed Jan 28, 2019

View reviewed changes

cpp-package/example/inference/README.md Outdated Show resolved Hide resolved

Isa-rentacs reviewed Jan 28, 2019

View reviewed changes

cpp-package/example/inference/sentiment_analysis_rnn.cpp Outdated Show resolved Hide resolved

Updated the example to handle variable length input. Updated the read…

bb14d79

…me and unit test files accordingly.

szha reviewed Jan 29, 2019

View reviewed changes

Isa-rentacs reviewed Jan 29, 2019

View reviewed changes

cpp-package/example/inference/sentiment_analysis_rnn.cpp Outdated Show resolved Hide resolved

Updated the example to share the memory between executors by createin…

a487ca9

…g shared executors.

leleamol added 2 commits February 5, 2019 11:19

Updated the creation of executors from largest to smallest bucket key

c3cace1

Creating the executor for the highest bucket key.

7d4a173

szha reviewed Feb 6, 2019

View reviewed changes

cpp-package/example/inference/sentiment_analysis_rnn.cpp Outdated Show resolved Hide resolved

szha reviewed Feb 6, 2019

View reviewed changes

cpp-package/example/inference/unit_test_sentiment_analysis_rnn.sh Outdated Show resolved Hide resolved

leleamol added 2 commits February 6, 2019 13:50

Updated the unit test to check for the results in a range and modifie…

b1e074f

…d the function name to be consistent with others.

Fixed the logic to find the right bucket.

0f155e9

sandeep-krishnamurthy merged commit febbdd4 into apache:master Feb 7, 2019

marcoabreu removed the pr-awaiting-review PR is waiting for code review label Jun 10, 2019

leleamol deleted the rnn-inference branch June 10, 2019 18:13



		int main(int argc, char** argv) {
		std::string model_file_json = "./obama-speaks-symbol.json";

[MXNET-1121] Example to demonstrate the inference workflow using RNN #13680

[MXNET-1121] Example to demonstrate the inference workflow using RNN #13680

Conversation

leleamol commented Dec 18, 2018 • edited Loading

Description

Checklist

Essentials

Changes

Comments

Roshrini commented Dec 19, 2018

Roshrini commented Jan 2, 2019

lupesko left a comment

Choose a reason for hiding this comment

lupesko commented Jan 8, 2019 • edited Loading

leleamol commented Jan 9, 2019

leleamol commented Jan 9, 2019

sandeep-krishnamurthy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leleamol commented Jan 10, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

IvyBazan left a comment

Choose a reason for hiding this comment

Isa-rentacs commented Jan 28, 2019

leleamol commented Jan 28, 2019

IvyBazan commented Jan 29, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Isa-rentacs left a comment

Choose a reason for hiding this comment

vdantu commented Jan 31, 2019

stephenrawls commented Feb 2, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sandeep-krishnamurthy commented Feb 7, 2019

leleamol commented Jun 10, 2019

leleamol commented Dec 18, 2018 •

edited

Loading

lupesko commented Jan 8, 2019 •

edited

Loading