Skip to content

Conversation

@Triforcey
Copy link
Contributor

@Triforcey Triforcey commented Sep 4, 2020

Body:
FEATURE

Created Swish activation function class and unit tests

To see the logs from the Cloud Build CI, please join either our discussion or announcement mailing list.


This change is Reviewable

Body:
FEATURE

Created Swish activation function class and unit tests
@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

Copy link
Contributor

@caisq caisq left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 1 approvals obtained (waiting on @caisq and @Triforcey)


tfjs-layers/src/activations.ts, line 224 at r2 (raw file):

alpha = 1

Is this necessary? Can you provide links to related document or code? I haven't seen the Swish activation with this parameter before. Also it appears to be not used anywhere or exposed to the public API.

@Triforcey
Copy link
Contributor Author

Yes, that variable is a part of the function's definition. It is referenced as β here: https://en.wikipedia.org/wiki/Swish_function. I still need to expose this to the API as a trainable parameter. I'm going to do that. I would appreciate any pointers you may have on how to go about doing that.

@Triforcey Triforcey marked this pull request as draft September 4, 2020 18:43
Copy link
Contributor

@caisq caisq left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! 1 of 1 approvals obtained (waiting on @Triforcey)


tfjs-layers/src/activations.ts, line 224 at r2 (raw file):

Previously, caisq (Shanqing Cai) wrote…
alpha = 1

Is this necessary? Can you provide links to related document or code? I haven't seen the Swish activation with this parameter before. Also it appears to be not used anywhere or exposed to the public API.

@Triforcey OK. I'n fine with leaving this argument in for the time being. However, for exposing it to public API, we generally strive to be consistent with our Python counterpart (tf.keras). In the case of swish, it seems that the Python counter part doesn't expose this as an argument or a trainable variable yet. See:

  1. https://www.tensorflow.org/api_docs/python/tf/keras/activations/swish
  2. https://github.com/tensorflow/tensorflow/blob/master/tensorflow/python/ops/nn_impl.py#L551

@Triforcey Triforcey marked this pull request as ready for review September 4, 2020 19:06
@Triforcey
Copy link
Contributor Author

Okay, sounds good. Thank you for your assistance.

@Triforcey
Copy link
Contributor Author

@caisq the branch is ready to be merged. 😄

@caisq
Copy link
Contributor

caisq commented Sep 4, 2020

@rthadur @pyu10055 feel free to merge the PR when you're comfortable.

@pyu10055 pyu10055 merged commit 402cbc9 into tensorflow:master Oct 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants