-
Notifications
You must be signed in to change notification settings - Fork 129
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
Add new custom derivatives for math functions #1237
base: master
Are you sure you want to change the base?
Conversation
clang-tidy review says "All clean, LGTM! 👍" |
clang-tidy review says "All clean, LGTM! 👍" |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1237 +/- ##
==========================================
- Coverage 94.64% 94.61% -0.04%
==========================================
Files 51 51
Lines 8890 8890
==========================================
- Hits 8414 8411 -3
- Misses 476 479 +3 |
This looks quite good. What are the remaining functions from math.h and cmath that still need to be supported? |
These are some funcitons from https://en.cppreference.com/w/c/numeric/math Basic Functions:
Exponential Functions:
Logarithmic Functions:
Power Functions:
Trigonometric Functions:
Hyperbolic Functions:
Error and Gamma Functions:
I'm not sure if I should include the remaining functions if they don't have derivatives. |
Probably returning 0 is fine. @parth-07 what do you think?
Yes. You can still fall back to a single templated implementation function. |
I mean functions like floor and ceil (they are implemented to return 0), and other functions after Nearest integer floating-point operations here: https://en.cppreference.com/w/c/numeric/math |
Oh, I see. Yes, I think we should return 0 as the derivative for such functions. |
} | ||
|
||
template <typename T, typename dT> | ||
CUDA_HOST_DEVICE void tan_pullback(T x, T d_y, T* d_x) { |
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.
dT
template type is not being used.
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.
yeah, sorry I fixed that in the pullback functions. I was waiting to finish adding the remaining functions before pushing it.
@a-elrawy, is there anything else that needs to be done here? |
Contributing to #871
Added support for the following functions:
tan
cosh
sinh
asin
atan
log10
log2
cbrt
hypot