-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
[CodeStyle] Add CI for self.assertTrue(np.allclose(...)) #45126
Conversation
你的PR提交成功,感谢你对开源项目的贡献! |
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.
待测试并完善新增的CI脚本后,删除测试样例文件(python/paddle/fluid/tests/unittests/CI_test文件夹及其下的文件)。
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.
grep -zoEnr "self\.assert(True|Equal)\(\s\s*(np|numpy)\.(allclose|array_equal)" ./
./python/paddle/fluid/contrib/slim/tests/test_weight_quantization_mobilenetv1.py:1:self.assertTrue( np.allclose./python/paddle/fluid/contrib/tests/test_model_cast_to_bf16.py:1:self.assertTrue( np.allclose./python/paddle/fluid/contrib/tests/test_model_cast_to_bf16.py:1:self.assertTrue( np.allclose./python/paddle/fluid/contrib/tests/test_weight_decay_extend.py:1:self.assertTrue( np.allclose./python/paddle/fluid/tests/custom_op/test_custom_tanh_double_grad.py:1:self.assertTrue( np.allclose./python/paddle/fluid/tests/custom_op/test_custom_tanh_double_grad.py:1:self.assertTrue( np.allclose./python/paddle/fluid/tests/custom_op/test_custom_tanh_double_grad.py:1:self.assertTrue( np.allclose./python/paddle/fluid/tests/test_if_else_op.py:1:self.assertTrue( np.allclose./python/paddle/fluid/tests/unittests/benchmark.py:1:self.assertTrue( np.allclose./python/paddle/fluid/tests/unittests/CI_test/test_dropout_op.py:1:self.assertTrue( np.allclose
...
./python/paddle/fluid/tests/unittests/xpu/test_batch_norm_op_xpu.py:1:self.assertEqual( np.allclose./python/paddle/tests/test_async_read_write.py:1:self.assertTrue( np.allclose
20a46a0
to
1932eae
Compare
bd0c6b2
to
67909e6
Compare
7ecdf8b
to
173fbf4
Compare
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.
删除几个测试文件即可。
@@ -259,6 +259,12 @@ if [ "${EMPTY_GRAD_OP_REGISTERED}" != "" ] && [ "${GIT_PT_ID}" != "" ]; then | |||
check_approval 1 43953930 46782768 22165420 22361972 | |||
fi | |||
|
|||
INVALID_UNITTEST_ASSERT_CHECK=`echo "$ALL_ADDED_LINES" | grep -zoE '\+\s+self\.assert(True|Equal)\((\s*\+\s*)?(np|numpy)\.(allclose|array_equal)[^+]*' || true` |
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.
照理说,这行应该会触发PR-CI-Approval的
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.
因为修改后的正则为了显示更加友好,把整行都给匹配出来。它要求以 +
开头且后接一系列空格之后再接 self.assertXxx
,相对于原来是更加严格的。
+ self.assertTrue( <---- 这一行前面要求必须是空格开头
+ np.allclose(...
因此它现在是不会将这个脚本自身内容视为关键词触发,因为这个脚本里 self.assertTrue(
前面并不是空格
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.
LGTM +++
PR types
Others
PR changes
Others
Describe
添加阻止使用 self.assertTrue(np.allclose(...)) 或 self.assertEqual(np.allclose(...)) 的 CI。
问题来源于 Recommend to use np.testing.assert_allclose instead of assertTrue(np.allclose(...)) #44641
RFC:[WIP, Don't review][CodeStyle] 单测报错信息优化 RFC community#198