-
Notifications
You must be signed in to change notification settings - Fork 6.8k
Conversation
@eric-haibin-lin @pengzhao-intel @Caenorst Please help to review. I copied the UTs from test_operator_gpu.py to test_operator.py but didn't remove the original ones since it seems they have minimal cuda version requirement. Let me know if it's not appropriate. Thanks! |
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.
could you elaborate "it seems they have minimal cuda version requirement" and why we duplicate the test?
Because of this line: /~https://github.com/apache/incubator-mxnet/blob/master/tests/python/gpu/test_operator_gpu.py#L2863 |
Thanks for the reply. Look forward to the update |
Is this PR good to merge? @TaoLv @eric-haibin-lin |
@eric-haibin-lin Code is re-based. |
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.
comments addressed
* init qk attention * qk: fake backward * cpu selfatt and encdec; move tests to test_operator.py * coding style * fix lint * use random seed in tests * remove ut in test_operator_gpu.py * coding style * retrigger ci
* init qk attention * qk: fake backward * cpu selfatt and encdec; move tests to test_operator.py * coding style * fix lint * use random seed in tests * remove ut in test_operator_gpu.py * coding style * retrigger ci Co-authored-by: Tao Lv <tao.a.lv@intel.com>
@TaoLv This is not true for the CD pipeline for cu90. See this failure |
@szha Any suggestion for this case? It's shared for both CPU and GPU test, can we add the cuda requirement decorator to it? Also if we're still releasing cu90 flavor, why it was removed from CI? |
Description
This PR fills the CPU counterpart of PR #16408.
Only FP32 is supported yet.
Checklist
Essentials
Please feel free to remove inapplicable items for your PR.
Changes
Comments