This repository was archived by the owner on Oct 13, 2021. It is now read-only.
Bug fix in forward and bi-directional lstm in handling bias #103
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The converter was not setting the value of R bias correctly during onnx conversion. We simply set the R biases to zero since the current conversion code is already setting W bias to the Keras' bias value. We expect the sum of the biases Wb + Rb to be the same as Keras' bias since Keras uses a single bias combining both. Changes to be committed: modified: keras2onnx/ke2onnx/bidirectional.py modified: keras2onnx/ke2onnx/lstm.py modified: tests/test_layers.py
|
|
|
@sonu1-p can you please sign CLA? thanks |
|
Hi David, I have signed it, don't know why this page is not reflecting the same. |
|
When I click on license/cla details, it says - You have signed the CLA for onnx/keras-onnx |
|
Seems like it is reflected now. Thanks! |
|
LGTM, thank you very much for the change! |
jiafatom
approved these changes
Jun 5, 2019
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The converter was not setting the value of R bias correctly during
onnx conversion.
We simply set the R biases to zero since the current conversion code
is already setting W bias to the Keras' bias value. We expect the sum
of the biases Wb + Rb to be the same as Keras' bias since Keras uses
a single bias combining both.
Changes to be committed: