Skip to content
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

Bipolar sigmoid added #3506

Merged
merged 9 commits into from Aug 24, 2023
Merged

Bipolar sigmoid added #3506

merged 9 commits into from Aug 24, 2023

Conversation

IWNMWE
Copy link
Contributor

@IWNMWE IWNMWE commented Jun 25, 2023

Implemented the bipolar sigmoid activation function
Fixed invertible activation functions
Added jacobian testing for all of the above mentioned ones

Tested and used in various research papers:

https://www.scitepress.org/Papers/2018/100372/100372.pdf

https://iopscience.iop.org/article/10.1088/1742-6596/1255/1/012023/meta

Copy link
Member

@zoq zoq left a comment

Choose a reason for hiding this comment

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

Let's add the same tests mentioned here - #3498 (review)

@IWNMWE
Copy link
Contributor Author

IWNMWE commented Jul 3, 2023

Hey @zoq I had checked out the issue you had mentioned, I just wanted to ask about how we can tackle functions which do require both the x and f(x) value for the calculation of their derivatives ,a great example for this type of activation function is the EliSH function whose derivative can't be expressed solely in terms of f(x).(I have even checked other libraries and tried my self to solve for f'(x) in terms of f(x) but was not able to find any implementation that uses only f(x) values.)

A possible fix for these kinds of layers is to store the x values as a member of the class and use them in the deriv() method pls do let me know if there is any problem with this implementation or if there are any other fixes possible I will look into it.

@zoq
Copy link
Member

zoq commented Jul 4, 2023

Yes, you have ti implement it as a layer and store the activation function in the layer.

@IWNMWE
Copy link
Contributor Author

IWNMWE commented Jul 6, 2023

@zoq Ok I will get to implementing all the required ones as layers. Then in this PR, I have fixed all activation functions which do have an inverse or can be brought into terms of their derivatives and add the jacobian test for all of them and in the next few PRs I will add the other ones as a layer . Please do let me know if there are any problem in this.

@IWNMWE
Copy link
Contributor Author

IWNMWE commented Jul 6, 2023

The jacobian testing implemented is as same as given in #3478

@IWNMWE IWNMWE requested a review from zoq July 7, 2023 12:13
@IWNMWE
Copy link
Contributor Author

IWNMWE commented Jul 19, 2023

Hey @zoq can you please review the changes and check if the above mentioned plan is fine so that I can get to implementing the non invertible functions as layers

@zoq
Copy link
Member

zoq commented Jul 19, 2023

Hey @zoq can you please review the changes and check if the above mentioned plan is fine so that I can get to implementing the non invertible functions as layers

Looking at it right now.

Copy link
Member

@zoq zoq left a comment

Choose a reason for hiding this comment

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

Minor style issues, the test and implementation looks correct.

@IWNMWE IWNMWE requested a review from zoq July 21, 2023 16:00
Copy link
Member

@zoq zoq left a comment

Choose a reason for hiding this comment

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

@IWNMWE IWNMWE requested a review from zoq July 22, 2023 20:45
Copy link
Member

@zoq zoq left a comment

Choose a reason for hiding this comment

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

No more comments from my side, thanks for putting this together.

Copy link

@mlpack-bot mlpack-bot bot left a comment

Choose a reason for hiding this comment

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

Second approval provided automatically after 24 hours. 👍

@mlpack-bot
Copy link

mlpack-bot bot commented Jul 25, 2023

Hello there! Thanks for your contribution. I see that this is your first contribution to mlpack. If you'd like to add your name to the list of contributors in COPYRIGHT.txt and you haven't already, please feel free to push a change to this PR---or, if it gets merged before you can, feel free to open another PR.

In addition, if you'd like some stickers to put on your laptop, I'd be happy to help get them in the mail for you. Just send an email with your physical mailing address to stickers@mlpack.org, and then one of the mlpack maintainers will put some stickers in an envelope for you. It may take a few weeks to get them, depending on your location. 👍

@zoq
Copy link
Member

zoq commented Jul 28, 2023

Let's rerun the CI.

@IWNMWE
Copy link
Contributor Author

IWNMWE commented Aug 10, 2023

Hey @zoq can this be merged so that I can create the subsequent pull requests required

@zoq zoq merged commit b8bc415 into mlpack:master Aug 24, 2023
12 of 19 checks passed
@zoq
Copy link
Member

zoq commented Aug 24, 2023

Sorry for the delay, thanks again for the contribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants