Skip to content

Support activations (Sigmoid, Tanh) for LSTM#20506

Merged
alalek merged 2 commits intoopencv:3.4from
JulieBar:lstm_activations
Aug 13, 2021
Merged

Support activations (Sigmoid, Tanh) for LSTM#20506
alalek merged 2 commits intoopencv:3.4from
JulieBar:lstm_activations

Conversation

@JulieBar
Copy link
Copy Markdown
Contributor

@JulieBar JulieBar commented Aug 5, 2021

Add support for activations for LSTM layer.
Related to issue: #18390

There are 3 models, mentioned in the issue. Two of them were produced by keras2onnx, another one - by CNTK (from onnxruntime). Unfortunately, LSTM doesn't support dynamic shapes, so I cannot test those two models.

Test data should be downloaded: opencv/opencv_extra#891

opencv_extra=lstm_activations

Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

  • I agree to contribute to the project under Apache 2 License.
  • To the best of my knowledge, the proposed patch is not based on a code under GPL or other license that is incompatible with OpenCV
  • The PR is proposed to proper branch
  • There is reference to original bug report and related work
  • There is accuracy test, performance test and test data in opencv_extra repository, if applicable
    Patch to opencv_extra has the same branch name.
  • The feature is well documented and sample code can be built with the project CMake

Copy link
Copy Markdown
Contributor

@sl-sergei sl-sergei left a comment

Choose a reason for hiding this comment

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

Thank you 👍

else if(attribute_name == "activations" && node_proto.op_type() == "LSTM")
{
lp.set(attribute_name, parseStr(attribute_proto.strings()));
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we move this to the attribute_proto.graphs_size() > 0 condition below?


TEST_P(Test_ONNX_layers, LSTM_Activations)
{
testONNXModels("lstm_cntk_tanh", pb, 0, 0, false, false);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please put test files into opencv_extra PR directly. "MIT License" of ONNXRuntime is compatible with OpenCV.

Downloading should be used to fetch large model files (>5Mb).

@alalek alalek merged commit cfb3644 into opencv:3.4 Aug 13, 2021
@alalek alalek mentioned this pull request Aug 14, 2021
@alalek alalek mentioned this pull request Oct 15, 2021
@JulieBar JulieBar deleted the lstm_activations branch November 21, 2021 01:08
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.

5 participants