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

[Typing] 修改一些错误的类型标注以及示例代码 #65452

Merged
merged 6 commits into from
Jun 26, 2024

Conversation

megemini
Copy link
Contributor

PR Category

Others

PR Types

Others

Description

  • LayerDict 的 __iter__ 应该返回 Iterator[str]
  • meshgrid 应该返回 list[Tensor]
  • make_layers 的输入应该是 list[object]
  • RandomAffine 的输入参数类型
  • 其他示例代码的错误

关联 PR #65008 #65397

@SigureMo

Copy link

paddle-bot bot commented Jun 25, 2024

你的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 paddle-bot bot added the contributor External developers label Jun 25, 2024
@SigureMo
Copy link
Member

非必要的情况下,PR 最好不要超过 20 个文件啊 😂

@megemini
Copy link
Contributor Author

非必要的情况下,PR 最好不要超过 20 个文件啊 😂

嗯嗯 ~ 改着改着一看都这么多了 ~~~ 🤣

打算后面尽量每天都能修改一部分 #65397 发现的错误 ~ 小步快跑吧 ~

>>> if dist.get_rank() == 0:
... data = paddle.to_tensor([1, 2, 3])
... dist.gather(data, gather_list, dst=0)
>>> else:
... data = paddle.to_tensor([4, 5, 6])
... dist.gather(data1, gather_list, dst=0)
... dist.gather(data, gather_list, dst=0)
Copy link
Member

Choose a reason for hiding this comment

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

这个我的 PR 没改么?应该已经改了吧?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

? 没冲突啊 ~ 我再 merge 一下 ~ 刚好把这个 pr 的文件数量删减一下 ~

Copy link
Member

Choose a reason for hiding this comment

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

和待合入的 #65376 冲突了

Copy link
Member

Choose a reason for hiding this comment

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

喔喔,我说的是 loss 那个文件的改动冲突了

Copy link
Member

Choose a reason for hiding this comment

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

不过这个居然真的有多个,#65429 刚改了一个

@@ -149,7 +149,7 @@ def sample(self, shape):
>>> import paddle
>>> from paddle.distribution import Bernoulli

>>> rv = Bernoulli(paddle.full((1), 0.3))
>>> rv = Bernoulli(paddle.full([1,], 0.3))
Copy link
Member

Choose a reason for hiding this comment

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

list 不需要 ,

@@ -337,7 +337,8 @@ def npair_loss(anchor, positive, labels, l2_reg=0.002):
.. code-block:: python

>>> import paddle
>>> DATATYPE = "float32"
>>> from paddle._typing import DTypeLike
>>> DATATYPE: DTypeLike = "float32"
Copy link
Member

Choose a reason for hiding this comment

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

这个我是不是也改了?和另一个 PR 冲突了吧,revert 掉吧

@@ -87,7 +88,7 @@ def check_layer_numerics(func):
... # return 1/x * self._w + self._b open it you will see the error log
... return x @ self._w + self._b
...
>>> dtype = 'float32'
>>> dtype: DTypeLike = 'float32'
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
>>> dtype: DTypeLike = 'float32'
>>> dtype: Literal["float32"] = 'float32'

目前都是这样改的

@SigureMo
Copy link
Member

20 个以上文件需要大佬 review,合并周期会长很多……

一般 PR 不要超过 10 个文件吧……

@megemini
Copy link
Contributor Author

20 个以上文件需要大佬 review,合并周期会长很多……

一般 PR 不要超过 10 个文件吧……

嗯 我拆分一下 ~

Copy link
Member

@SigureMo SigureMo left a comment

Choose a reason for hiding this comment

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

LGTMeow 🐾

@SigureMo SigureMo merged commit d37cd92 into PaddlePaddle:develop Jun 26, 2024
32 of 34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor External developers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants