-
Notifications
You must be signed in to change notification settings - Fork 2k
[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
base: master
Are you sure you want to change the base?
Conversation
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 What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
@googlebot I signed it! |
There was a problem hiding this 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
@pyu10055 Thank you for taking the time to review my PR. 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: 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. |
There was a problem hiding this 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
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 |
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