Skip to content

[Typing] 修改 core.pyi 及一些示例代码中的类型错误 #65496

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

Merged
merged 10 commits into from
Jun 28, 2024

Conversation

megemini
Copy link
Contributor

@megemini megemini commented Jun 26, 2024

PR Category

User Experience

PR Types

Bug fixes

Description

  • 修改 core.pyiXXXPlace 不再继承 Place
  • 添加 core.pyi 一些类及方法
  • 其他示例错误

关联 PR #65008 #65397

@SigureMo

Copy link

paddle-bot bot commented Jun 26, 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 26, 2024
@@ -26,23 +28,39 @@ class Place:
def set_place(self, place: Place) -> None: ...
def xpu_device_id(self) -> int: ...

class CPUPlace(Place): ...
class CPUPlace: ...
Copy link
Member

Choose a reason for hiding this comment

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

这里不「继承」,也不「实现」的话,怎么保证 CPUPlace() 可以传入 foo(p: Place) 呢?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

咱们有个

PlaceLike: TypeAlias = Union[
    "CPUPlace",
    "CUDAPlace",
    "CUDAPinnedPlace",
    "IPUPlace",
    "CustomPlace",
    "XPUPlace",
    str,  # some string like "cpu", "gpu:0", etc.
]

是不是应该用这个?

Copy link
Member

Choose a reason for hiding this comment

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

那……Place 的意义是啥啊 😂

Copy link
Contributor Author

@megemini megemini Jun 26, 2024

Choose a reason for hiding this comment

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

不知道 ... ... 可能有地方单独需要 Place 的一些方法?

class Place(Protocol):
    def custom_device_id(self) -> int: ...
    def custom_device_type(self) -> str: ...
    def gpu_device_id(self) -> int: ...
    def ipu_device_id(self) -> int: ...
    def is_cpu_place(self) -> bool: ...
    def is_cuda_pinned_place(self) -> bool: ...
    def is_custom_place(self) -> bool: ...
    def is_gpu_place(self) -> bool: ...
    def is_ipu_place(self) -> bool: ...
    def is_xpu_place(self) -> bool: ...
    def set_place(self, place: Place) -> None: ...
    def xpu_device_id(self) -> int: ...

Copy link
Member

Choose a reason for hiding this comment

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

所以为啥不实现呢?

所有 Place 实现这个 Place 不可以么?

然后我们是不是可以用 PlaceLike: TypeAlias = Union[Place, str]

@megemini
Copy link
Contributor Author

megemini commented Jun 26, 2024

Update 20240626

  • 修改 core.pyi 中的类型
  • mobilenetv2/v3

@@ -54,7 +54,7 @@ def __init__(
oup: int,
stride: int,
expand_ratio: float,
norm_layer: nn.Layer = nn.BatchNorm2D,
norm_layer: type[nn.Layer] = nn.BatchNorm2D,
Copy link
Member

Choose a reason for hiding this comment

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

是不是和 #65487 做重了?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

哦?好像当时还只看到 mobilenetv3 ~ 我撤掉就行 ~

@megemini
Copy link
Contributor Author

Update 20240627

  • 撤回 mobilenet v2/v3 部分重复标注

SigureMo
SigureMo previously approved these changes Jun 27, 2024
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
Copy link
Member

2024-06-27 14:22:12 paddle.Tensor.process_mesh:1
2024-06-27 14:22:12 <string>:7:12: error: Missing positional argument "placements" in call to "shard_tensor"  [call-arg]
2024-06-27 14:22:12 Found 1 error in 1 file (checked 1 source file)
2024-06-27 14:22:12 
2024-06-27 14:22:12 
2024-06-27 14:22:12 >>> Mistakes found in type checking!
2024-06-27 14:22:12 >>> Please recheck the type annotations. Run `tools/type_checking.py` to check the typing issues:

喔喔,还有点问题

@megemini
Copy link
Contributor Author

2024-06-27 14:22:12 paddle.Tensor.process_mesh:1
2024-06-27 14:22:12 <string>:7:12: error: Missing positional argument "placements" in call to "shard_tensor"  [call-arg]
2024-06-27 14:22:12 Found 1 error in 1 file (checked 1 source file)
2024-06-27 14:22:12 
2024-06-27 14:22:12 
2024-06-27 14:22:12 >>> Mistakes found in type checking!
2024-06-27 14:22:12 >>> Please recheck the type annotations. Run `tools/type_checking.py` to check the typing issues:

喔喔,还有点问题

居然影响到这里 ... ... 防不胜防 ~~~

Copy link
Contributor

@Aurelius84 Aurelius84 left a comment

Choose a reason for hiding this comment

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

Great work! LGTM

@SigureMo SigureMo merged commit 1575a1f into PaddlePaddle:develop Jun 28, 2024
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.

3 participants