Skip to content

[tfjs-layers] Add SigmoidRange layer #5548

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

bibs2091
Copy link

@bibs2091 bibs2091 commented Aug 29, 2021

Hello,

This is an implementation of the SigmoidRange layer mentioned in #5452 (comment) .

The SigmoidRange takes two parameters min and max and forces the output to be between them. The default value of min and max are 0 and 1 respectively (a normal sigmoid)
The SigmoidRange equation goes this way:
SigmoidRange(x) = sigmoid(x) * (max - min) + min

Please review it and tell me if I should add anything


This change is Reviewable

@google-cla
Copy link

google-cla bot commented Aug 29, 2021

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.

@google-cla google-cla bot added the cla: no label Aug 29, 2021
@bibs2091
Copy link
Author

@googlebot I signed it!

@google-cla google-cla bot added cla: yes and removed cla: no labels Aug 29, 2021
@bibs2091 bibs2091 changed the title Add SigmoidRange layer [tfjs-layers] Add SigmoidRange layer Aug 29, 2021
Copy link
Collaborator

@pyu10055 pyu10055 left a comment

Choose a reason for hiding this comment

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

The PR looks good, thanks you for the contribution.
I did some googling, I did not see this layer is defined by either keras or tf.keras
I am trying to understand the use case, can you provide any links to the definition of this activation layers?
Also if you can add tests would be great, thanks

Reviewable status: 0 of 1 approvals obtained

@bibs2091
Copy link
Author

bibs2091 commented Aug 30, 2021

@pyu10055 Thank you for taking the time to review my PR.
The SigmoidRange layer is inspired from the deep learning framework fastai which has more than 21k star in GitHub, you can check for the docs of the layer here and the code here.

The layer is used when you want to bound the output of your model in a certain range. For my exact use case, I use it for my recommendation system engine, I know for sure that the ratings are between 1 and 5 and my model can accept float values, and SigmoidRange makes a perfect sense to be used. I think other regression tasks can benefit from that if the output min and max are already known (example: when the model task it to output the axis (x,y) of the nose in an image, the SigmoidRange will assure that the coordinates are bigger than 0 and smaller than the size of the image)

One can argue that the model will learn that value is between min and max, but it will take some epochs to learn that. I ran a simple performance test on it, and it really helped accelerating the training speed in the first epochs:
image

Finally, if you think the feature is not essential enough or the name is too confusing, we can rename the layer into Sigmoid (since there's no sigmoid layer) and this layer optionally can receive two params, min and high, if the user does not set them they are automatically set to 0 and 1 respectively making a the normal sigmoid function.

I can start on the tests once you confirm that I can continue forward with the feature.

Copy link
Collaborator

@pyu10055 pyu10055 left a comment

Choose a reason for hiding this comment

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

@bibs2091 Thank you for the detailed explanation. I think keep it as SigmoidRange would be better, since it is different from the official Sigmoid definition.
Please let me know when you have the tests ready. Thanks!

Reviewable status: 0 of 1 approvals obtained

@bibs2091
Copy link
Author

Hello @pyu10055, sorry for taking long time to answer, I was waiting for the #5660 for the issue on testing in windows to be solved, but it did not yet.

Anyway I made some test, i think they work but i haven't got to test them because of the issue. and the GitHub action shows that theres an issue but i can't get the access to the log of it (it says i don't have permission). Can you check the tests for me, and tell me how to inspect the reason why the checks were not successful?

Thanks

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.

2 participants