[CodeStyle] replace `assert np.allclose` with `np.testing.assert_allclose` and `assert np.array_equal` with `np.testing.assert_array_equal` #55385

zrr1999 · 2023-07-12T18:03:52Z

PR types

Others

PR changes

Others

Description

#55348
替换了下面四种写法

assert np.allclose(...)
assert np.isclose(...).all()
assert np.array_equal(...)
assert not np.array_equal(...)

np.testing.assert_array_compare(operator.__ne__, $$$A) 这种写法参考了stackoverflow

paddle-bot · 2023-07-12T18:03:57Z

你的PR提交成功，感谢你对开源项目的贡献!
请关注后续CI自动化测试结果，详情请参考Paddle-CI手册。
Your PR has been submitted. Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

SigureMo

这次挂掉的单测还挺多的，可以这个 PR 把相关单测 revert 掉，尽快合入，然后在新的 PR 慢慢解决剩下的几个单测

根据经验（#44988、#45213），应该有 shape 不对齐、精度误差等问题，可参考之前的 PR 中的方案来解决～

SigureMo · 2023-07-13T01:28:17Z

test/auto_parallel/random_control_unittest.py

                        mask_tensor_remote.numpy(), mask_tensor_local.numpy()
                    )
                else:
-                    assert not np.array_equal(
+                    import operator


import operator 放在文件开始吧

好的，已修改，这个其实

SigureMo · 2023-07-13T01:38:21Z

test/legacy_test/test_adaptive_max_pool3d.py


-            assert np.allclose(res_3, self.res_3_np)
+            np.testing.assert_allclose(res_3, self.res_3_np)

            # assert np.allclose(res_4, self.res_4_np)


这种注释里的可以简单拿正则搜一下，手动替换一下～

好的，已修复

SigureMo · 2023-07-13T01:58:57Z

另外有一个个人的建议

https://github.com/PaddlePaddle/Paddle/blob/4c5ce835d62eb156a74eb80f851034cb8624d2c0/test/dygraph_to_static/test_cycle_gan.py#L697C1-L711

希望把这里的 assert_func 换成 np.testing 下的函数～可以在下一个 PR 一起做，因为目前 PaddleSOT 发现部分情况该函数会出现精度问题，因此在 https://github.com/PaddlePaddle/PaddleSOT/blob/be49db8f688c455794204241d3a5c45f698ab6c3/tests/run_all_paddle_ci.sh#L19 暂时禁掉了，但从 log 上只知道 False is not true，具体差异多少根本看不到，希望这里也改一下～

tips: 如果需要传递 atol 或者 rtol 参数的话，可以用 functools.partial

zrr1999 · 2023-07-13T03:31:59Z

另外有一个个人的建议

https://github.com/PaddlePaddle/Paddle/blob/4c5ce835d62eb156a74eb80f851034cb8624d2c0/test/dygraph_to_static/test_cycle_gan.py#L697C1-L711

希望把这里的 assert_func 换成 np.testing 下的函数～可以在下一个 PR 一起做，因为目前 PaddleSOT 发现部分情况该函数会出现精度问题，因此在 https://github.com/PaddlePaddle/PaddleSOT/blob/be49db8f688c455794204241d3a5c45f698ab6c3/tests/run_all_paddle_ci.sh#L19 暂时禁掉了，但从 log 上只知道 False is not true，具体差异多少根本看不到，希望这里也改一下～

tips: 如果需要传递 atol 或者 rtol 参数的话，可以用 functools.partial

好的，我改一下

zrr1999 · 2023-07-13T07:00:35Z

PR types

Others

PR changes

Others

Description

#55348 替换了下面四种写法

assert np.allclose(...)

assert np.isclose(...).all()

assert np.array_equal(...)

assert not np.array_equal(...)

np.testing.assert_array_compare(operator.__ne__, $$$A) 这种写法参考了stackoverflow

这个 assert not np.array_equal(...)我给revert了，因为stackoverflow 提到的这个方法是解决所有值均不同，这里应该是存在值不同，所以可能要后面再研究下，或者其实没必要修稿

SigureMo · 2023-07-13T07:04:17Z

这个 assert not np.array_equal(...)我给revert了，因为stackoverflow 提到的这个方法是解决所有值均不同，这里应该是存在值不同，所以可能要后面再研究下，或者其实没必要修稿

嗯嗯，好的

SigureMo · 2023-07-13T15:26:38Z

test/legacy_test/test_activation_op.py

-            assert np.allclose(res_2, np.power(input, 3))
-            assert np.allclose(res_6, np.power(input, 3))
+            np.testing.assert_allclose(
+                res_1, np.power(input, 2), rtol=1e-6, atol=1e-6


这里 atol 使用 1e-6 的原因是？

#44988 对比过两者的默认值

np.allclose(a, b, rtol=1e-05, atol=1e-08, equal_nan=False)

np.testing.assert_allclose(actual, desired, rtol=1e-07, atol=0, equal_nan=True, err_msg='', verbose=True)

因此理论上 rtol=1e-05, atol=1e-08 所有单测都能通过的，因此原来默认值的情况下建议 rtol 不要超过 1e-5，atol 不要超过 1e-8，一种非常简单的方式是将所有出现精度问题的单测设置成这俩默认值即可，应该都能过

因为这个当时看到很多都是1e-7+，然后就为方便起见全都统一成了1e-6的阈值了，这部分我也不知道为什么换成np.testing.assert_allclose会导致精度出现不同，看起来或许这个是一个概率问题，大部分情况只会略微比1e-7大一点

这部分我也不知道为什么换成np.testing.assert_allclose会导致精度出现不同，看起来或许这个是一个概率问题

不是的呀，我们分析一下哈：

原代码 assert np.allclose(res_2, np.power(input, 3)) 没有指定 rtol 和 atol，所以它等价于 assert np.allclose(res_2, np.power(input, 3), rtol=1e-05, atol=1e-08)，而直接转写后的代码 np.testing.assert_allclose(res_1, np.power(input, 2)) 同样没有默认值，等价于 np.testing.assert_allclose(res_1, np.power(input, 2), rtol=1e-07, atol=0)，同样没有设置默认值的情况下，后者明显是比前者更「严格」的，因此会出现部分单测过不去的问题，这些单测的精度应该在 rtol: 1e-07~1e-05, atol: 0~1e-08 之间

因此为了能达到同样的效果，最便捷的方式是改成 np.testing.assert_allclose(res_1, np.power(input, 2), rtol=1e-05, atol=1e-08)

有道理，我已经全都改过来啦

zrr1999 · 2023-07-14T03:07:29Z

@SigureMo 现在应该ok了，剩下的那个应该还是之前那个奇怪的问题

SigureMo · 2023-07-14T03:09:57Z

@SigureMo 现在应该ok了，剩下的那个应该还是之前那个奇怪的问题

revert 吧 😂

SigureMo

其余 LGTM～

注意一下 https://github.com/PaddlePaddle/community/blob/master/pfcc/call-for-contributions/code_style/code_style_improvement_for_unittest.md#%E9%81%97%E7%95%99%E9%97%AE%E9%A2%98 提到的遗留问题，如果这几个能确定修改的是对的的话是没问题的，如果不确定的话 revert 就好了～

test/collective/fleet/test_mixed_precision.py

test/dygraph_to_static/test_cinn_prim.py

test/dygraph_to_static/test_cycle_gan.py

test/legacy_test/test_egr_python_api.py

test/legacy_test/test_layers.py

test/legacy_test/test_nan_to_num_op.py

test/legacy_test/test_pixel_shuffle_op.py

test/legacy_test/test_prune.py

SigureMo · 2023-07-23T08:11:52Z

test/collective/fleet/test_mixed_precision.py

+
+                    assert not np.array_equal(moment1_, pre_moment1_)
+
+                    assert not np.array_equal(beta_pow1_, pre_beta_pow1_)


这个文件看起来还没改回去？

另外，针对某几个文件，可以尝试 git checkout develop -- file1 file2 直接 revert

SigureMo · 2023-07-23T08:13:05Z

test/legacy_test/test_imperative_auto_prune.py

-                np.array_equal(linear_origin, linear.weight.numpy())
-            )
+
+            assert not np.array_equal(linear_origin, linear.weight.numpy())


这里貌似也？

SigureMo · 2023-07-23T08:13:33Z

test/legacy_test/test_layers.py

-                np.array_equal(conv2d1_weight_np, conv2d2.weight.numpy())
-            )
+
+            assert not np.array_equal(conv2d1_weight_np, conv2d2.weight.numpy())


这个文件也有多处

SigureMo · 2023-07-23T08:14:54Z

test/legacy_test/test_prune.py

@@ -610,7 +612,8 @@ def test_prune_with_multi_optimizers(self):
            )

        np.testing.assert_array_equal(weight_with_prune, weight_expected)
-        self.assertFalse(np.array_equal(weight_without_prune, weight_expected))
+
+        assert not np.array_equal(weight_without_prune, weight_expected)


这个文件也

SigureMo · 2023-07-23T08:16:28Z

test/legacy_test/test_pixel_shuffle_op.py

@@ -272,9 +272,12 @@ def test_static_graph_layer(self):
                fetch_list=out_2,
                use_prune=True,
            )
+            # exe.run output maybe squeeze.
+            res_1 = res_1[0] if len(res_1) == 1 else res_1
+            res_2 = res_2[0] if len(res_2) == 1 else res_2


我看很多文件有这样的修改，maybe squeeze 具体是指？

这块是因为很多与res_1 对应的变量，例如 out_1_np，当第一个维度为1的时候都进行了类似squeeze的操作，所以这里我任务可能需要修改一些 exe.run 的代码，使其会自动进行squeeze操作，从而不需要在测试用例中修改内容。

话说这里的问题会不会是 exe.run 用法错了呢？

https://www.paddlepaddle.org.cn/documentation/docs/zh/develop/api/paddle/static/Executor_cn.html#run-program-none-feed-none-fetch-list-none-feed-var-name-feed-fetch-var-name-fetch-scope-none-return-numpy-true-use-program-cache-false-use-prune-false

应该是

a = data('a') b = data('b') c = a + b d = c * a c_data, d_data = exe.run( feed={'a': a, 'b': b}, fetch_list=[c.name, d.name] )

但这里直接将一个结果（比如 265 行的 out_1）当成 fetch_list 是什么鬼呀，因为这个才导致维度啥的出现问题吧

啊，好像还真就支持

Paddle/python/paddle/fluid/executor.py

Lines 1709 to 1715 in dc96ebc

is_fetch_var = lambda var: isinstance(var, (Variable, str))

is_tuple_list = lambda var: isinstance(var, (tuple, list))

if fetch_list is None:

return []

if is_fetch_var(fetch_list):

return [fetch_list]

paddle-ci-bot · 2023-07-30T03:07:39Z

Sorry to inform you that 6aa1e32's CIs have passed for more than 7 days. To prevent PR conflicts, you need to re-run all CIs manually.

SigureMo · 2023-07-31T11:24:15Z

test/legacy_test/test_pixel_shuffle_op.py

@@ -272,9 +272,11 @@ def test_static_graph_layer(self):
                fetch_list=out_2,
                use_prune=True,
            )
+            res_1 = res_1[0]


res_1[0] 直接写在下面或者上面吧，这样看着很奇怪

SigureMo

还有这四处～

test/legacy_test/test_group_norm_op_v2.py

test/legacy_test/test_number_count_op.py

SigureMo · 2023-07-31T11:26:38Z

test/xpu/test_masked_select_op_xpu.py

@@ -106,7 +106,8 @@ def test_static_mode(self):
            feed={"x": np_x, "mask": np_mask},
            fetch_list=[out],
        )
-        self.assertEqual(np.allclose(res, np_out), True)
+        res = res[0] if len(res) == 1 else res


SigureMo

LGTM~~~

paddle-bot bot added contributor External developers status: proposed labels Jul 12, 2023

SigureMo reviewed Jul 13, 2023

View reviewed changes

SigureMo mentioned this pull request Jul 13, 2023

单测报错信息优化（二期） #55348

Closed

SigureMo reviewed Jul 13, 2023

View reviewed changes

zrr1999 added 12 commits July 13, 2023 15:55

replace assert

8e8e6b4

replace assert

2a13a57

replace comments

808a826

move operator

09d3e25

fix shape

b890f61

fix group_norm_naive_for_general_dimension

d8ce156

revert assert not

326343f

fix rtol and atol

b5daac4

fix tol

1d1b158

fix

60b65ac

replace assert_func

eaae7c5

fix tol

25374ec

zrr1999 force-pushed the replace branch from 500397c to 25374ec Compare July 13, 2023 16:34

SigureMo reviewed Jul 14, 2023

View reviewed changes

SigureMo self-assigned this Jul 18, 2023

fix tol

47dcf38

zrr1999 added 6 commits July 18, 2023 03:57

fix bug

b0f0768

fix

45b119b

fix bug

545ab61

fix

4a7cd3b

fix

0ce1584

fix

6aa1e32

zrr1999 requested a review from SigureMo July 23, 2023 05:53

SigureMo reviewed Jul 23, 2023

View reviewed changes

zrr1999 added 5 commits July 31, 2023 08:30

fix bug

7ebd53e

fix

2882e03

fix

d7c722b

fix

592f4cd

fix

b6465d5

SigureMo reviewed Jul 31, 2023

View reviewed changes

zrr1999 added 3 commits July 31, 2023 11:45

fix

81b7919

fix

6c3baea

fix

09adbb7

SigureMo approved these changes Jul 31, 2023

View reviewed changes

luotao1 merged commit 744e1ea into PaddlePaddle:develop Aug 1, 2023


		assert not np.array_equal(moment1_, pre_moment1_)

		assert not np.array_equal(beta_pow1_, pre_beta_pow1_)

	is_fetch_var = lambda var: isinstance(var, (Variable, str))
	is_tuple_list = lambda var: isinstance(var, (tuple, list))

	if fetch_list is None:
	return []
	if is_fetch_var(fetch_list):
	return [fetch_list]

[CodeStyle] replace assert np.allclose with np.testing.assert_allclose and assert np.array_equal with np.testing.assert_array_equal #55385

[CodeStyle] replace assert np.allclose with np.testing.assert_allclose and assert np.array_equal with np.testing.assert_array_equal #55385

Conversation

zrr1999 commented Jul 12, 2023 • edited by SigureMo Loading

PR types

PR changes

Description

paddle-bot bot commented Jul 12, 2023

SigureMo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SigureMo commented Jul 13, 2023

zrr1999 commented Jul 13, 2023

zrr1999 commented Jul 13, 2023

PR types

PR changes

Description

SigureMo commented Jul 13, 2023

Choose a reason for hiding this comment

zrr1999 Jul 13, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zrr1999 commented Jul 14, 2023

SigureMo commented Jul 14, 2023

SigureMo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paddle-ci-bot bot commented Jul 30, 2023

Choose a reason for hiding this comment

SigureMo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SigureMo left a comment

Choose a reason for hiding this comment

[CodeStyle] replace `assert np.allclose` with `np.testing.assert_allclose` and `assert np.array_equal` with `np.testing.assert_array_equal` #55385

[CodeStyle] replace `assert np.allclose` with `np.testing.assert_allclose` and `assert np.array_equal` with `np.testing.assert_array_equal` #55385

zrr1999 commented Jul 12, 2023 •

edited by SigureMo

Loading

zrr1999 Jul 13, 2023 •

edited

Loading