-
Notifications
You must be signed in to change notification settings - Fork 6.8k
Conversation
src/operator/nn/mkldnn/mkldnn_act.cc
Outdated
@@ -45,11 +45,9 @@ namespace op { | |||
bool SupportMKLDNNAct(const ActivationParam& param) { | |||
// We only enable ReLU for now. It seems other activations have some precision |
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.
Remove comment
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.
Remove comment why?
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.
The comment explains why the operators have been disabled. This PR re-enables them and thus the comment is obsolete
How have the precision problems been resolved? Is there a test? |
it seems the precision problem hasn't been fixed in mkldnn. I'm notify them of this problem. hopefully, it can be fixed soon. |
I see, thanks a lot! So we'll wait to merge this until Intel fixed the problem or how would you propose to move forward? |
@zheng-da thanks a lot. I will follow up with our team :) |
I wanted to test if MKLDNN activation is working now. We can close the PR for now and reopen it after the bug in MKLDNN is fixed, or just keep it open. Either way is fine. |
Look at the code, two implementations are the difference for the soft_relu. So, we get the different results.
|
but the error happens here: https://github.com/apache/incubator-mxnet/blob/master/tests/python/gpu/test_operator_gpu.py#L1111 |
@zheng-da tests/python/unittest/test_loss.py fail too which used softrelu.
|
@zheng-da @pengzhao-intel ping |
@pengzhao-intel is there any update from the Intel MKLDNN team? |
@zheng-da @piiswrong sorry I missed the first ping. I will raise the priority for this issue and update to you soon. |
@zheng-da @piiswrong Fixed the issue in the local and PR on the road by @jinhuang415 |
Description
Previously, some activation types in MKLDNN aren't used because there was a precision problem.
This is to enable all activations in MKLDNN.
Checklist
Essentials
make lint
)