Skip to content
This repository was archived by the owner on Jul 1, 2023. It is now read-only.

Conversation

@saeta
Copy link
Contributor

@saeta saeta commented Feb 23, 2019

This change follows along with the defaulted values for Conv and other layers.

This change follows along with the defaulted values for Conv and other layers.
Copy link
Contributor

@rxwei rxwei left a comment

Choose a reason for hiding this comment

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

I was intentionally not doing so because I wanted the initializer with the activation parameter to be the first candidate in code completion. But for consistency with Keras, I think this change is fine.

Copy link
Member

@dan-zheng dan-zheng left a comment

Choose a reason for hiding this comment

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

LGTM. Perhaps identity activation would be better represented as an optional (no activation) in the future.

(Side note: related issue about identity activation and debuggability of gradient tensor seed shape mismatch.)

@rxwei
Copy link
Contributor

rxwei commented Feb 23, 2019

(Side note: related issue about identity activation and debuggability of gradient tensor seed shape mismatch.)

You can check these pretty easily by inserting a precondition or assert in each primitive VJP's pullback to make sure the seed matches the shape of the output. However, that would cause the pullback to capture the original result, which is bad for memory usage. The proper way is to make the differentiation transform insert these checks based on the optimization level.

@saeta saeta merged commit 39114bb into master Feb 24, 2019
@saeta saeta deleted the default-activation branch February 24, 2019 02:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants