Add LSTMCell by leonling-ll · Pull Request #102 · plaidml/openvino

leonling-ll · 2020-11-06T06:13:21Z

Add LSTMCell.

One thing needed to discuss is in OpenVINO Doc, LSTMCell has attributes activations_alpha, activations_beta which are used for activation functions if applicable.
But in LSTMCell ngraph implementation, I didn't see how they are used. For basic activation funciton relu, sigmoid, tanh, those attributes seem are not required.
So, my current strategy is taking in these two attributes but doing nothing with them.
Please help check if there are any better ways to deal with them, or just remove them like in ngraph.

BTW, the formula in OpenVINO Doc seems not very accurate, the actual equation I followed is

// ---- Equations ----
    // f, g, h - are activation functions.
    // it = f(Xt*(Wi^T) + Ht-1*(Ri^T) + Wbi + Rbi)
    // ft = f(Xt*(Wf^T) + Ht-1*(Rf^T) + Wbf + Rbf)
    // ct = g(Xt*(Wc^T) + Ht-1*(Rc^T) + Wbc + Rbc)
    // ot = f(Xt*(Wo^T) + Ht-1*(Ro^T) + Wbo + Rbo)
    // Ct = ft (.) Ct-1 + it (.) ct
    // Ht = ot (.) h(Ct)

where Wb* + Rb* = B*

leonling-ll · 2020-11-06T07:29:44Z

inference-engine/src/plaidml_plugin/ops/lstm_cell.cpp

+    } else {
+        THROW_IE_EXCEPTION << "Unsupported activation function";
+    }
+}


Maybe we can combine clip_Tensor() and activation_func() into one function and move it to plaidml_util.cpp.
Like

edsl::Tensor clip_activation(std::string activation, const edsl::Tensor& T, bool should_clip, float clip) { edsl::Tensor T_clipped; if (should_clip) { T_clipped = op::clip(T, edsl::Tensor(-clip), edsl::Tensor(clip)); } else { T_clipped = T; } if (func_name == "relu") { return op::relu(T_clipped); } else if (func_name == "sigmoid") { return op::sigmoid(T_clipped); } else if (func_name == "tanh") { return edsl::tanh(T_clipped); } else { THROW_IE_EXCEPTION << "Unsupported activation function"; } }

I think for now, just put these in an unnamed namespace in this file.

But, if we need these anywhere else, then yes, we should do as you suggest, and move these into plaidml_util.cpp (or perhaps even the PlaidML oplib)

From my current observation, these works for RNNCell, GRUCell and LSTMCell. Not sure in other ops, if there are any other activation functions will be included.
Maybe we can keep these in op file and decide whether put them into plaidml_util.cpp after going through most ops.

Ok, since it's used for those 3, let's move it to plaidml_util.cpp

I have move it to plaidml_util.cpp. Once this PR has been merged, I'll apply this change to RNNCell and GRUCell.

tzerrell

Great! I agree with your analysis & strategy for alpha & beta

tzerrell · 2020-11-07T02:07:28Z

inference-engine/src/plaidml_plugin/ops/lstm_cell.cpp

+    } else {
+        THROW_IE_EXCEPTION << "Unsupported activation function";
+    }
+}


I think for now, just put these in an unnamed namespace in this file.

But, if we need these anywhere else, then yes, we should do as you suggest, and move these into plaidml_util.cpp (or perhaps even the PlaidML oplib)

tzerrell · 2020-11-07T02:10:20Z

inference-engine/src/plaidml_plugin/ops/lstm_cell.cpp

+    // TODO: activation_alpha and activation_beta are not used
+    auto activation_alpha = layer->get_activations_alpha();
+    auto activation_beta = layer->get_activations_beta();


My guess is that alpha & beta are reserved names for if more activations are added in the future, and for now they are unused. I would just ignore them altogether.

tzerrell · 2020-11-07T02:13:03Z

inference-engine/tests/functional/plugin/shared/src/single_layer_tests/lstm_cell.cpp

    std::string targetDevice;
-    std::tie(should_decompose, batch, hidden_size, input_size, activations, clip, netPrecision,
-            targetDevice) = obj.param;
+    std::tie(should_decompose, batch, hidden_size, input_size, activations, activations_alpha, activations_beta, clip,


I think since alpha & beta are unused, it would be OK to not add them here.

No harm in adding them though, so it's also OK to leave this code as-is.

I have reverted tests code to without activations alpha beta version to make it simple.
We can add them later if needed.

tzerrell

Looks good!

tzerrell · 2020-11-09T19:10:30Z

inference-engine/src/plaidml_plugin/ops/lstm_cell.cpp

+    } else {
+        THROW_IE_EXCEPTION << "Unsupported activation function";
+    }
+}


Ok, since it's used for those 3, let's move it to plaidml_util.cpp

@ref

* [DOCS] [41549] Fix broken code block in Install OpenVINO from PyPI Repository (openvinotoolkit#2800) * Turned list into headings * fixes * fix * add animation (openvinotoolkit#2865) * Align `time_tests` with master branch from 4021e14 (openvinotoolkit#2881) * Added info on DockerHub CI Framework (openvinotoolkit#2919) * Feature/azaytsev/cherry pick pr2541 to 2021 1 (openvinotoolkit#2960) * added OpenVINO Model Server to docs (openvinotoolkit#2541) * added OpenVINO Model Server * updated documentation to include valid links * minor fixes * Fixed links and style * Update README.md fixed links to model_server * more corrections * dropped reference in ie_docs and minor fixes * Update README.md Fixed links to Inference Engine pages Co-authored-by: Alina Alborova <alina.alborova@intel.com> Co-authored-by: Andrey Zaytsev <andrey.zaytsev@intel.com> * Added Model Server docs to 2021/1 Co-authored-by: Trawinski, Dariusz <dariusz.trawinski@intel.com> Co-authored-by: Alina Alborova <alina.alborova@intel.com> * See Also sections in MO Guide (openvinotoolkit#2770) * convert to doxygen comments * layouts and code comments * separate layout * Changed layouts * Removed FPGA from the documentation * Updated according to CVS-38225 * some changes * Made changes to benchmarks according to review comments * Added logo info to the Legal_Information, updated Ubuntu, CentOS supported versions * Updated supported Intel® Core™ processors list * Fixed table formatting * update api layouts * Added new index page with overview * Changed CMake and Python versions * Fixed links * some layout changes * some layout changes * some layout changes * COnverted svg images to png * layouts * update layout * Added a label for nGraph_Python_API.md * fixed links * Fixed image * removed links to ../IE_DG/Introduction.md * Removed links to tools overview page as removed * some changes * Remove link to Integrate_your_kernels_into_IE.md * remove openvino_docs_IE_DG_Graph_debug_capabilities from layout as it was removed * update layouts * Post-release fixes and installation path changes * Added PIP installation and Build from Source to the layout * Fixed formatting issue, removed broken link * Renamed section EXAMPLES to RESOURCES according to review comments * add mo faq navigation by url param * Removed DLDT description * Pt 1 * Update Deep_Learning_Model_Optimizer_DevGuide.md * Extra file * Update IR_and_opsets.md * Update Known_Issues_Limitations.md * Update Config_Model_Optimizer.md * Update Convert_Model_From_Kaldi.md * Update Convert_Model_From_Kaldi.md * Update Convert_Model_From_MxNet.md * Update Convert_Model_From_ONNX.md * Update Convert_Model_From_TensorFlow.md * Update Converting_Model_General.md * Update Cutting_Model.md * Update IR_suitable_for_INT8_inference.md * Update Aspire_Tdnn_Model.md * Update Convert_Model_From_Caffe.md * Update Convert_Model_From_TensorFlow.md * Update Convert_Model_From_MxNet.md * Update Convert_Model_From_Kaldi.md * Added references to other fws from each fw * Fixed broken links * Fixed broken links * fixes * fixes * Fixed wrong links Co-authored-by: Nikolay Tyukaev <ntyukaev_lo@jenkins.inn.intel.com> Co-authored-by: Andrey Zaytsev <andrey.zaytsev@intel.com> Co-authored-by: Tyukaev <nikolay.tyukaev@intel.com> * Fixes (openvinotoolkit#3105) * Renamed Benchmark App into Benchmark Tool in the menu (openvinotoolkit#3032) * [DOC] Update Docker install guide (openvinotoolkit#3055) (openvinotoolkit#3200) * [DOC] Update Docker install guide * [DOC] Add proxy for Windows Docker install guide * [DOC] move up prebuilt images section * Update installing-openvino-linux.md * Update installing-openvino-docker-linux.md * Update installing-openvino-docker-linux.md Formatting fixes * Update installing-openvino-docker-linux.md Fixed formatting issues * Update installing-openvino-docker-windows.md Minor fixes * Update installing-openvino-docker-linux.md Fixed formatting issues * [DOC] update text with CPU image, remove proxy for win * Update installing-openvino-docker-windows.md Minor fixes * Update installing-openvino-docker-windows.md Minor fix * Update installing-openvino-docker-windows.md Minor fix * Update installing-openvino-docker-windows.md Co-authored-by: Andrey Zaytsev <andrey.zaytsev@intel.com> (cherry picked from commit 4a09888) * Align time_tests with master (openvinotoolkit#3238) * Align time_tests with master * Fix "results" uploading to DB in time_tests * Add new model to `tgl_test_config.yml` * Fix onnx tests versions (openvinotoolkit#3240) * [40929] DL Workbench in Get Started (openvinotoolkit#2740) * Initial commit * Added the doc * More instructions and images * Added slide * Borders for screenshots * fixes * Fixes * Added link to Benchmark app * Replaced the image * tiny fix * tiny fix * Links to DL Workbench Installation Guide (openvinotoolkit#2861) * Links to WB * Changed wording * Changed wording * Fixes * Changes the wording * Minor corrections * Removed an extra point * [41545] Add links to DL Workbench from components that are available in the DL WB (openvinotoolkit#2801) * Added links to MO and Benchmark App * Changed wording * Fixes a link * fixed a link * Changed the wording * Feature/azaytsev/change layout (openvinotoolkit#3295) * Changes according to feedback comments * Replaced @ref's with html links * Fixed links, added a title page for installing from repos and images, fixed formatting issues * Added links * minor fix * Added DL Streamer to the list of components installed by default * Link fixes * Link fixes * ovms doc fix (openvinotoolkit#2988) * added OpenVINO Model Server * ovms doc fixes Co-authored-by: Trawinski, Dariusz <dariusz.trawinski@intel.com> * Add several new models to `tgl_test_config.yml` in time_tests (openvinotoolkit#3269) * Fix wrong path for `yolo-v2-tiny-ava-0001` for time_tests * Add several new models to `tgl_test_config.yml` in time_tests * Fix a typo in DL Workbench Get Started (openvinotoolkit#3338) * Fixed a typo * Update openvino_docs.xml Co-authored-by: Andrey Zaytsev <andrey.zaytsev@intel.com> * ops math formula fix (openvinotoolkit#3333) Co-authored-by: Nikolay Tyukaev <ntyukaev_lo@jenkins.inn.intel.com> * Fix paths for `squeezenet1.1` in time_tests config (openvinotoolkit#3416) * GNA Plugin doc review (openvinotoolkit#2922) * Doc review * Addressed comments * Removed an inexistent link * Port PlaidML plugin forward to 2021.1 (#32) Adds PlaidML plugin to this repo on the 2021.1 branch * Enable testing of BatchNorm (#33) * Require specific path to shared library (#34) * enable separate source/so paths * fix tabs * Fix multiple outputs and add Split (#42) Fixes the problem with multiple outputs in the PlaidML plugin described in #36. Adds the Split operation with tests, which utilizes multiple outputs. * Swish (#47) * Add Reverse & tests to PlaidML Plugin (#35) * Make separate PlaidMLProgramBuilder (#92) * Variadic Split (#91) * variadic split w/o -1 * -1 implemented * Fix -1 support in variadic split and test it * Style consistency * Cleanup * Fix exception message * Add BinaryConvolution (#93) * Add working tests back (#97) * Attempt to add tests back * Change op name for logical_and to match tests * Add Tests for *Convert *Convolution_Backprop_Data *Fake_quantize *SoftMax *Tile *Transpose * Fix comparison and logical tests, use IE ref mode for now * Remove cumsum and logical tests * Remove comparison tests and it's fixes * Add bucketize op and tests (#90) * Add extract image patches op (#96) * Hswish via ReLU (#95) * Hswish via ReLU * ReLU max_value used * Add reorg_yolo op (#101) * Remove conv bprop & fake quant tests (#106) * add EmbeddingBagOffsetsSum op and tests (#100) * Add LSTMCell (#102) * Add RNNCell (#109) Co-authored-by: Ling, Liyang <liyang.ling@intel.com> * Add space_to_batch op (#104) * Add tests for MinMax, DepthToSpace (#105) * Add GELU (#107) * Add GRUCell (#110) Co-authored-by: Ling, Liyang <liyang.ling@intel.com> * Fix support for using OpenVINO as a subproject (#111) * Build fixes for newer compilers (#113) * add EmbeddingBagPackedSum op and tests (#114) Co-authored-by: Tim Zerrell <tim.zerrell@intel.com> * Add shuffle_channels op and test. (#112) * Tests for squared difference op (#115) Co-authored-by: Tim Zerrell <tim.zerrell@intel.com> * Add acosh, asinh, atanh into tests (#118) * Reverse sequence (#116) * Add PriorBox op and test. (#117) * Remove obsolete PlaidML code (#120) Co-authored-by: Alina Alborova <alina.alborova@intel.com> Co-authored-by: Nikolay Tyukaev <nikolay.tyukaev@intel.com> Co-authored-by: Vitaliy Urusovskij <vitaliy.urusovskij@intel.com> Co-authored-by: Andrey Zaytsev <andrey.zaytsev@intel.com> Co-authored-by: Trawinski, Dariusz <dariusz.trawinski@intel.com> Co-authored-by: Nikolay Tyukaev <ntyukaev_lo@jenkins.inn.intel.com> Co-authored-by: Kate Generalova <kate.generalova@intel.com> Co-authored-by: Rafal Blaczkowski <rafal.blaczkowski@intel.com> Co-authored-by: Tim Zerrell <tim.zerrell@intel.com> Co-authored-by: Brian Retford <brian.retford@intel.com> Co-authored-by: Michael Yi <michael.w.yi@intel.com> Co-authored-by: Liyang Ling <liyang.ling@intel.com> Co-authored-by: Namrata Choudhury <namrata.choudhury@intel.com> Co-authored-by: Xin Wang <xin1.wang@intel.com> Co-authored-by: xinghong chen <xinghong.chen@intel.com> Co-authored-by: Zhibin Li <haoyouab@gmail.com> Co-authored-by: Frank Laub <frank.laub@intel.com>

leonling-ll added 6 commits November 4, 2020 20:05

Add draft lstm_cell for testing

473f96d

Add default params test

a88fdfd

Fix matrix multiply

f4cef28

Add optional params for activations and clip

7ef9512

Fix typo

50fb7ee

Add activation_alpha and activation_beta

b2389ce

leonling-ll requested review from flaub and tzerrell November 6, 2020 06:13

leonling-ll self-assigned this Nov 6, 2020

leonling-ll linked an issue Nov 6, 2020 that may be closed by this pull request

Add LSTMCell #64

Closed

leonling-ll commented Nov 6, 2020

View reviewed changes

tzerrell reviewed Nov 7, 2020

View reviewed changes

Revert activations_alpha, activations_beta

7c07bd6

leonling-ll force-pushed the liyang-lstm-cell branch from 61c127b to 91ff588 Compare November 9, 2020 02:32

leonling-ll added 3 commits November 9, 2020 10:33

Fix format

91ff588

Fix format

576ad0e

Process clip without throwing exception

f8b74cd

tzerrell reviewed Nov 9, 2020

View reviewed changes

tzerrell previously approved these changes Nov 9, 2020

View reviewed changes

tzerrell mentioned this pull request Nov 9, 2020

Add RNNCell #103

Closed

leonling-ll dismissed tzerrell’s stale review via ab6277e November 10, 2020 01:43

Move clip_activation function to plaidml_util.cpp

ab6277e

tzerrell approved these changes Nov 10, 2020

View reviewed changes

tzerrell merged commit 842e515 into plaidml:plaidml Nov 10, 2020

YangleiZouIntel pushed a commit that referenced this pull request Dec 18, 2020

Add LSTMCell (#102)

c6457f9

YangleiZouIntel pushed a commit that referenced this pull request Jan 14, 2021

Add LSTMCell (#102)

20a5a87

YangleiZouIntel deleted the liyang-lstm-cell branch January 15, 2021 05:33

YangleiZouIntel pushed a commit that referenced this pull request Feb 2, 2021

Add LSTMCell (#102)

53c48bf

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add LSTMCell#102

Add LSTMCell#102
tzerrell merged 11 commits intoplaidml:plaidmlfrom
Flex-plaidml-team:liyang-lstm-cell

leonling-ll commented Nov 6, 2020 •

edited

Loading

Uh oh!

leonling-ll Nov 6, 2020 •

edited

Loading

Uh oh!

tzerrell Nov 7, 2020

Uh oh!

leonling-ll Nov 9, 2020

Uh oh!

tzerrell Nov 9, 2020

Uh oh!

leonling-ll Nov 10, 2020

Uh oh!

tzerrell left a comment •

edited

Loading

Uh oh!

tzerrell Nov 7, 2020

Uh oh!

tzerrell Nov 7, 2020

Uh oh!

tzerrell Nov 7, 2020

Uh oh!

leonling-ll Nov 9, 2020

Uh oh!

tzerrell left a comment

Uh oh!

tzerrell Nov 9, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

leonling-ll commented Nov 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

leonling-ll Nov 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tzerrell left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tzerrell left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

leonling-ll commented Nov 6, 2020 •

edited

Loading

leonling-ll Nov 6, 2020 •

edited

Loading

tzerrell left a comment •

edited

Loading