Add SELU activation function #2559
Conversation
kimhan0515
requested review from
myungjoo,
jijoongmoon,
again4you,
jaeyun-jung,
leemgs,
wooksong,
helloahn,
kparichay and
gichan-jang
as code owners
|
📝 TAOS-CI Version: 1.5.20200925. Thank you for submitting PR #2559. Please a submit 1commit/1PR (one commit per one PR) policy to get comments quickly from reviewers. Your PR must pass all verificiation processes of cibot before starting a review process from reviewers. If you are new member to join this project, please read manuals in documentation folder and wiki page. In order to monitor a progress status of your PR in more detail, visit http://ci.nnstreamer.ai/. |
kimhan0515
requested review from
anyj0527,
zhoonit,
lhs8928,
songgot,
jihochu,
DonghakPark,
SeoHyungjun,
baek2sm,
skykongkong8,
djeong20,
EunjuYang and
a team
as code owners
There was a problem hiding this comment.
@kimhan0515, 💯 All CI checkers are successfully verified. Thanks.
nntrainer/layers/acti_func.h
Outdated
| template <typename T = float> static T selu(T x) { | ||
| return x > static_cast<T>(0.0) | ||
| ? selu_scale * x | ||
| : selu_scale * static_cast<T>(selu_alpha * (exp(x) - 1)); |
There was a problem hiding this comment.
This code would emit error like below when built with -Denable-fp16=true option:
Needs some trivial fixing.
../nntrainer/layers/acti_func.h:498:69: error: converting to ‘_Float16’ from ‘float’ with greater conversion rank [-Werror]
498 | : selu_scale * static_cast<T>(selu_alpha * (exp(x) - 1));
| ^
cc1plus: all warnings being treated as errorsTry below to reproduce above:
cd ~/nntrainer
meson build -Denable-fp16=true
cd ./build
ninjaThe same issue was pointed at #2545 and fixed with #2557
Please refer to them and reconsider afterwards.
There was a problem hiding this comment.
I think adding enable-fp16 could be an option to eliminate this kind of error. #2560 Let's talk about it!
- Now, user can use SELU activation function like torch or tensor flow. **Self evaluation:** 1. Build test: [X]Passed [ ]Failed [ ]Skipped 2. Run test: [X]Passed [ ]Failed [ ]Skipped Signed-off-by: kimhan0515 <[email protected]>
kimhan0515
force-pushed
the
activation-selu
branch
from
c74ff89 to
65a9eec
Compare
| EXPECT_NEAR(data[i], answer[i], tolerance); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Along with this, I would like to personally recommend implementing a half-precision TC as well.
You might need to add a new file like : unittest_nntrainer_activations_fp16.cpp
There was a problem hiding this comment.
I agree with the point!
Maybe it would be better to implement half-precision TCs for other types of activations as well as SELU, in separate PR!
There was a problem hiding this comment.
@kimhan0515, 💯 All CI checkers are successfully verified. Thanks.
| @@ -667,6 +694,8 @@ class ActiFunc { | |||
| private: | |||
| constexpr static inline float alpha = 1.0f; /**< alpha for elu */ | |||
| constexpr static inline float beta = 1.0f; /**< beta for Softplus */ | |||
| constexpr static inline float selu_alpha = 1.67326324f; /**< alpha for selu */ | |||
| constexpr static inline float selu_scale = 1.05070098f; /**< scale for selu */ | |||
There was a problem hiding this comment.
As you may already know, setting the parameters of the activation function in this way is not a good direction. I understand that existing codes, especially Leaky ReLU, have already been implemented in this manner. So I know why you implemented it this way and I understand you. As for the next task, rather than adding an activation function, how about improving the structure so that the parameters of the activation function can be processed by receiving the property as input? It is optional but not essential for you.
There was a problem hiding this comment.
Thanks for your opinion! I think that the task will be quite worthwhile.
Self evaluation:
Signed-off-by: kimhan0515 [email protected]