Skip to content
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

[xdoctest] reformat example code with google style in paddle/tensor/math #55768

Merged
merged 10 commits into from
Aug 7, 2023

Conversation

SigureMo
Copy link
Member

@SigureMo SigureMo commented Jul 28, 2023

@paddle-bot
Copy link

paddle-bot bot commented Jul 28, 2023

你的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.

@paddle-bot
Copy link

paddle-bot bot commented Jul 28, 2023

❌ The PR is not created using PR's template. You can refer to this Demo.
Please use PR's template, it helps save our maintainers' time so that more developers get helped.

@SigureMo SigureMo changed the title [xdoctest] reformat example code with google style in `paddle/tensor/… [WIP][xdoctest] reformat example code with google style in paddle/tensor/math Jul 28, 2023
@SigureMo
Copy link
Member Author

SigureMo commented Jul 28, 2023

本 PR 用来测试新版的 convert-doctest

另外看看在不同设备是否有「精度」问题

@SigureMo
Copy link
Member Author

@megemini

一些小的优化点

  • 最后增加 error summary,不然不太清楚都有哪些挂了,比如这个文件 6000 行,在一堆 SUCCESS 里很难看到哪里有报错,包括运行失败、输出对比失败以及我看有时候还会有 parse error

    image image

    其实是有的不应该用 .. code-block: python 的错误写成了 python

  • 其他的问题之后再看

@megemini
Copy link
Contributor

megemini commented Jul 28, 2023

  • 最后增加 error summary

image
image

另外,输出信息里面有 line number 可以做个简单的参考吧 ~
image

不过,这都是小事儿,刚发现个问题,xdoctest 会把 DoctestParseError 直接 pass 掉 ... ...

        # Always warn when something bad is happening.
        # However, dont error if the docstr simply has bad syntax
        print('msg = {}'.format(msg))
        warnings.warn(msg)
        if isinstance(ex, exceptions.MalformedDocstr):
            pass
        elif isinstance(ex, exceptions.DoctestParseError):
            pass
        else:
            raise

从而导致咱们的 Xdoctester 捕获不到结果,返回的是 nocode=True,也就是没结果:

[TestResult(name='Test docstring from: file *test_math_target.py* line number *591*.', nocode=True, passed=False, skipped=False, failed=False, time=-1, test_msg='', extra_info=None)]

这个咱们在 review 的时候要注意一下... ...

@SigureMo
Copy link
Member Author

不过,这都是小事儿,刚发现个问题,xdoctest 会把 DoctestParseError 直接 pass 掉 ... ...

啊这,从 TestResult 好像完全看不出来是解析失败啊……xdoctest 好歹返回一个状态啊……

@SigureMo
Copy link
Member Author

另外,输出信息里面有 line number 可以做个简单的参考吧 ~

嗯嗯,稍后我用新版再尝试几个,看是否还有别的优化点~

@SigureMo
Copy link
Member Author

image

@megemini 果然有部分在本地和 CI 上有部分精度 diff 的,请问精度 diff 有什么比较好的处理方式嘛?直接忽略数值?前后包裹 # doctest: +SKIP # doctest: -SKIP?还是有什么比较好的方式嘛?

@megemini
Copy link
Contributor

还是有什么比较好的方式嘛?

不建议 skip ~

这个问题其实有多种解决办法:

pytorch 的 round 已经可以设置保留小数了,paddle 的好像还不行?提个 PR ?

回到此次任务本身,可以考虑转为 numpy 进行 round 输出:

image

这是在什么设备上导致的问题?影响面大吗?我个人觉得,如果影响面不大,或者只是个别设备的话,可以针对这种设备赦免测试~

你看咋整?!

@SigureMo
Copy link
Member Author

pytorch 的 round 已经可以设置保留小数了,paddle 的好像还不行?提个 PR ?

涉及到 API 的变动还是挺麻烦的

xdoctest 如果能引入 float 比对的方法是最好的,比如 pytest 就有 pypi.org/project/pytest-doctestplus/#floating-point-comparison

这个看起来确实是最好的,directive 可以自定义嘛,可以自己写一个嘛?有什么接口嘛?如果可以自定义我觉得是最好的~

这是在什么设备上导致的问题?影响面大吗?我个人觉得,如果影响面不大,或者只是个别设备的话,可以针对这种设备赦免测试~

也可以从 CI 上 copy 结果,但我觉得不是很好

这个设备的话,我是 M1 mac,和你都是 1.94591010,而 CI 上是 1.94591022

@megemini
Copy link
Contributor

这个看起来确实是最好的,directive 可以自定义嘛,可以自己写一个嘛?有什么接口嘛?如果可以自定义我觉得是最好的~

这个 ... ... 还得再看看 ~

我觉得目前可能还真得手动 copy ,其他的几个办法(用 ... 或者用 round)都不太好 ... ...

要么在 issue 里面写个注意事项?

@SigureMo
Copy link
Member Author

关于 float 输出匹配,我大致看了 https://stackoverflow.com/questions/2428618/how-to-test-floats-results-with-doctest 里的几个方案,其中 https://github.com/boisgera/numtest 是比较合适的方案,看起来是利用 monkey patch 将 doctest 的 OutputChecker 替换掉了,我们是否也能将 xdoctest 的检查代码(也许是这里?https://github.com/Erotemic/xdoctest/blob/main/src/xdoctest/checker.py#L201 )替换一下以实现 float 的匹配呢?

大致看了下 PyTorch,没发现使用什么特殊的 directive,不太清楚他们是怎么避免这个问题的,还是说他们没有设备之间计算精度的差异?

@megemini
Copy link
Contributor

megemini commented Jul 30, 2023

#55811

已修改~

不过,这种打补丁的方式真的不太好... ...

numtest 相对好一点,应为 python 的 doctestdoctest.register_optionflag("NUMBER") 接口,而 xdoctest 没有,只能硬打补丁,从而导致一个问题:

  • 这里只是 patch checker.check_output 方法,如果这个方法出错了,外面错误信息不会显示 patch 之后的字符串,给调试带来不方便,如:

    checker.check_output 方法中将 1.23456 改为了 1.23457,一旦出错,错误信息还是显示 1.23456 而不是修改后的 1.23457

@SigureMo SigureMo changed the title [WIP][xdoctest] reformat example code with google style in paddle/tensor/math [xdoctest] reformat example code with google style in paddle/tensor/math Aug 5, 2023
Copy link
Contributor

@megemini megemini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM ~

修改本身没啥问题,但是,好像 renorm 的中文文档没有~
英文有:
http://preview-paddle-pr-55768.paddle-docs-preview.paddlepaddle.org.cn/documentation/docs/en/api/paddle/renorm_en.html

中文没有~

另外,中英文文档好像不少对不上,英文的:
image

中文的:
image

这只是一少部分~

@luotao1 luotao1 merged commit ae88111 into PaddlePaddle:develop Aug 7, 2023
@SigureMo SigureMo deleted the xdoctest/tensor-math branch August 7, 2023 08:26
Copy link
Contributor

@sunzhongkai588 sunzhongkai588 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

示例代码存在问题:

@sunzhongkai588 文档本身存在问题:

  • scale
  • logaddexp
  • renorm(无中文文档)
  • increment(中文typo)
  • frexp(英文return格式)

Comment on lines +246 to +259
>>> # scale with parameter scale as a Tensor
>>> import paddle

data = paddle.randn(shape=[2, 3], dtype='float32')
factor = paddle.to_tensor([2], dtype='float32')
res = paddle.scale(data, scale=factor, bias=1.0)
>>> data = paddle.arange(6).astype("float32").reshape([2, 3])
>>> print(data)
Tensor(shape=[2, 3], dtype=float32, place=Place(cpu), stop_gradient=True,
[[0., 1., 2.],
[3., 4., 5.]])
>>> factor = paddle.to_tensor([2], dtype='float32')
>>> res = paddle.scale(data, scale=factor, bias=1.0)
>>> print(res)
Tensor(shape=[2, 3], dtype=float32, place=Place(cpu), stop_gradient=True,
[[1. , 3. , 5. ],
[7. , 9. , 11.]])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这部分中文好像没有copy-from来着,我记一下叭
image

@@ -703,7 +726,7 @@ def logaddexp(x, y, name=None):

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

中文公式好像出了点问题..我记下
image

@@ -919,14 +948,16 @@ def remainder(x, y, name=None):

Examples:

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remainder 这个api文档在预览里没找着..但我看__all__是有的呀

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我记得 remainder 好像是另一个叫啥 API 的一个 alias(忘了是哪个了),也许和这个有关系

Comment on lines +1616 to +1618
>>> import paddle

>>> # x is a Tensor with following elements:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • 缩进有些问题,改一下吧
Suggested change
>>> import paddle
>>> # x is a Tensor with following elements:
>>> import paddle
>>> # x is a Tensor with following elements:
  • 而且中文的好像也没copy from成功,也是因为缩进么?
image

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

就这一个的话我借一个 PR 改一下~

@@ -2156,17 +2290,19 @@ def renorm(x, p, axis, max_norm):
Tensor: the renorm Tensor.

Examples:
.. code-block:: python
.. code-block:: python
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

记一下:补充中文文档

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants