Conversation
pelesh
left a comment
There was a problem hiding this comment.
Looks good to me and tests pass.
@abirchfield and @lukelowry: Please review smoothing functions docs and implementation. I would appreciate your approval before we merge this.
lukelowry
left a comment
There was a problem hiding this comment.
Great work this is great step forward for cases
| * @return Scalar value indicating limit activation | ||
| */ | ||
| template <class ScalarT, typename RealT> | ||
| __attribute__((always_inline)) inline ScalarT indicator( |
There was a problem hiding this comment.
The indicator_low and indicator_high functions make sense, but I am confused by indicator_zero. Can this be brought into indicator? If Enzyme needs it this way, then that is okay by me.
There was a problem hiding this comment.
It's not needed by Enzyme, and it could be brought into indicator, I'm not sure what difference that would make. It made sense to me to write it this way, but I'm not particularly attached to the implementation.
| \phi(V_R,f) &= \left[1-\phi_L\right]\left[1-\phi_U\right]\\ | ||
| \phi_L(V_R)&= \sigma(V_R-V_{rmin}) \\ | ||
| \phi_U(V_R)&= \sigma(V_{rmax}-V_R) \\ | ||
| \phi_0(V_R)&= \phi_L + \phi_U - 1 \\ |
There was a problem hiding this comment.
This is cleaner than my original approach! Can you explain why it is called
There was a problem hiding this comment.
It turns out that it corresponds to f=0, so I called it f=0, f<0 and f>0 cases. I'm not particularly attached to that notation, though.
Description
This is a functional implementation of the Hawaii case
Closes #336
Proposed changes
Checklist
-Wall -Wpedantic -Wconversion -Wextra.Further comments