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

activation intrinsics for neural networks #860

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

Conversation

jalvesz
Copy link
Contributor

@jalvesz jalvesz commented Aug 13, 2024

@jalvesz jalvesz mentioned this pull request Aug 17, 2024
@jalvesz
Copy link
Contributor Author

jalvesz commented Aug 23, 2024

Before moving forward, any opinions on

  • putting these functions within the "specialfunctions" category?
  • naming of the derivative/gradient version with <name>_grad?
  • any other remark?

@jalvesz jalvesz reopened this Dec 23, 2024
@jalvesz jalvesz added the topic: mathematics linear algebra, sparse matrices, special functions, FFT, random numbers, statistics, ... label Dec 23, 2024
@jalvesz jalvesz marked this pull request as ready for review December 23, 2024 19:44
Copy link
Member

@perazz perazz left a comment

Choose a reason for hiding this comment

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

Thank you for this implementation @jalvesz, I have left my comments. I am no machine learning expert, so I focused on the code rather than on the actual math of each function.

interface tanh_grad
!! Version: experimental
!!
!! gradient of the gaussian function
Copy link
Member

Choose a reason for hiding this comment

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

Please add a specs description of this function.

!!
!! Fast approximation of the tanh function
!! Source: https://fortran-lang.discourse.group/t/fastgpt-faster-than-pytorch-in-300-lines-of-fortran/5385/31
#:for rk, rt in REAL_KINDS_TYPES
Copy link
Member

Choose a reason for hiding this comment

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

Please add a specs description for ftanh and ferf in the docs.

Copy link
Member

Choose a reason for hiding this comment

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

Also: I believe a discussion would be needed for fast, approximate versions of intrinsic functions. Different from gamma (that returns similar accuracy as the intrinsics), these are fast approximations. I'm fine with the f prefix (or fast_ would also be OK). I believe that should be standardized somewhere, for example in the library style guide?

integer, parameter :: n = 10
real(sp) :: x(n), y(n), y_ref(n)

y_ref = [-0.0455002784729 , -0.093188509345055, -0.148066952824593,&
Copy link
Member

Choose a reason for hiding this comment

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

Should the tests be run for all accuracies?

@jalvesz
Copy link
Contributor Author

jalvesz commented Apr 7, 2025

Thanks @perazz for your review! regarding tanh and erf, I wonder if I should actually remove reference to the intrinsic names and simply leave a reference to the fast approximation as that is what makes sense for NNs. Also, should these functions stay here or moved to the intrinsics module?

Regarding

I'm fine with the f prefix (or fast_ would also be OK). I believe that should be standardized somewhere, for example in the library style guide?

I agree with you and also wonder which one would be preferable, I don't have strong oppinions on that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
idea Proposition of an idea and opening an issue to discuss it topic: mathematics linear algebra, sparse matrices, special functions, FFT, random numbers, statistics, ...
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants