Skip to content

Add RNNCell#103

Closed
leonling-ll wants to merge 3 commits intoplaidml:plaidmlfrom
Flex-plaidml-team:liyang-rnncell
Closed

Add RNNCell#103
leonling-ll wants to merge 3 commits intoplaidml:plaidmlfrom
Flex-plaidml-team:liyang-rnncell

Conversation

@leonling-ll
Copy link
Copy Markdown

No description provided.

@leonling-ll leonling-ll requested review from flaub and tzerrell November 9, 2020 03:10
@leonling-ll leonling-ll self-assigned this Nov 9, 2020
@leonling-ll leonling-ll linked an issue Nov 9, 2020 that may be closed by this pull request
tzerrell
tzerrell previously approved these changes Nov 9, 2020
Copy link
Copy Markdown

@tzerrell tzerrell left a comment

Choose a reason for hiding this comment

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

LGTM, I have some optional suggestions inline.


namespace PlaidMLPlugin {

edsl::Tensor clip_activation(const std::string& func_name, bool should_clip, float clip, const edsl::Tensor& T) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

As discussed in #102, let's follow your suggestion and move this to plaidml_util

Comment on lines +50 to +51
auto activations_alpha = layer->get_activations_alpha();
auto activations_beta = layer->get_activations_beta();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I believe these are unused with current activations

@tzerrell tzerrell mentioned this pull request Nov 10, 2020
@tzerrell
Copy link
Copy Markdown

Obsolete -- this same code (plus merge conflict fixes) was landed as #109

@tzerrell tzerrell closed this Nov 10, 2020
@YangleiZouIntel YangleiZouIntel deleted the liyang-rnncell branch January 15, 2021 05:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add RNNCell

2 participants