[Typing][C-34][BUAA] Add type annotations for `python/paddle/distributed/fleet/fleet.py` #67624

Luohongzhige · 2024-08-21T16:53:41Z

PR Category

User Experience

PR Types

Improvements

Description

paddle-bot · 2024-08-21T16:53:46Z

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

megemini

没有逐个 review，几个问题：

类型的标注尽量简洁，如 paddle.Tensor 改为 Tensor ，相应的导入语句放到 TYPE_CHECKING 里面
泛型的参数化，如 list 需要写为 list[xxx]，以及 set dict tuple 等
不是 callable ，是 Callable ，从 collections.abc 导入
注意 paddle._typing 中通用类型的使用，如 place: paddle.CUDAPlace, 是否可以使用 paddle._typing 中的类型
Literal 的使用，如 def all_reduce(self, input, mode="sum") 中的 mode
类中实例参数的标注，如 Fleet 的 strategy_compiler, user_defined_optimizer 等
返回的 Self 的使用
私有函数/方法不需要标注

megemini · 2024-08-22T13:18:01Z

python/paddle/distributed/fleet/fleet.py

+    def all_reduce(
+        self,
+        input: Tensor,
+        mode: str = mode_Literal,


Literal 的用法看一下其他 PR 是如何实现的～

megemini

为什么要删掉默认值？？？

megemini · 2024-08-23T04:29:23Z

python/paddle/distributed/fleet/fleet.py

+def apply_ir_passes(
+    main_program: Program,
+    startup_program: Program,
+    config: BuildStrategy,


Suggested change

config: BuildStrategy,

config: Fleet,

megemini · 2024-08-23T04:32:54Z

python/paddle/distributed/fleet/fleet.py

        self._role_maker = None
-        self.strategy_compiler = None
+        self.strategy_compiler: StrategyCompiler = None


Suggested change

self.strategy_compiler: StrategyCompiler = None

self.strategy_compiler: StrategyCompiler | None = None

megemini · 2024-08-23T04:33:45Z

python/paddle/distributed/fleet/fleet.py

+        is_collective: bool = False,
+        strategy: DistributedStrategy | None = None,
+        log_level: int | str = "INFO",
+    ) -> Fleet:


Suggested change

) -> Fleet:

) -> Self:

megemini · 2024-08-23T04:36:28Z

python/paddle/distributed/fleet/fleet.py

-    ):
+        iteration: int,
+        x: Tensor,
+        group: HybridCommunicateGroup,


Suggested change

group: HybridCommunicateGroup,

group: Group,

megemini · 2024-08-23T04:37:23Z

python/paddle/distributed/fleet/fleet.py

+        self,
+        iteration: int,
+        x: Tensor,
+        group: HybridCommunicateGroup,


Suggested change

group: HybridCommunicateGroup,

group: Group,

megemini · 2024-08-23T05:36:36Z

python/paddle/distributed/fleet/fleet.py

@@ -500,7 +546,7 @@ def reduce_scatter_perf(
                f"[Perf Warning] ReduceScatter Test Timeout! {ret} > {perf_threshold_time}"
            )

-    def _collective_perf_impl(self, round=50, context={}, hcg=None):
+    def _collective_perf_impl(self, round, context, hcg=None):


默认值呢?

megemini · 2024-08-23T05:38:38Z

python/paddle/distributed/fleet/fleet.py

+        self,
+        optimizer: Optimizer,
+        strategy: DistributedStrategy | None = None,
+    ) -> Fleet:


Suggested change

) -> Fleet:

) -> Self:

Self在python3.11才引入,此处使用TypeVar实现

嗯好问题～但是，为什么项目中都在用 Self 呢？

抱歉，资料没有找全，python3.11之前Self由typing_extensions引入

megemini · 2024-08-23T05:39:10Z

python/paddle/distributed/fleet/fleet.py

@@ -1319,10 +1394,14 @@ def set_date(self, table_id, day_id):

    @is_non_distributed_check
    @inited_runtime_handler
-    def shrink(self, threshold=None):
+    def shrink(self, threshold: int) -> None:


Suggested change

def shrink(self, threshold: int) -> None:

def shrink(self, threshold: int | None = None) -> None:

megemini · 2024-08-23T05:39:47Z

python/paddle/distributed/fleet/fleet.py

+        self,
+        loss,
+        startup_program,
+        parameter_list,
+        no_grad_set,


默认值呢？

megemini · 2024-08-23T05:39:59Z

python/paddle/distributed/fleet/fleet.py

+        startup_programs,
+        parameter_list,
+        no_grad_set,


默认值呢？

megemini

另外，

_inited_runtime_handler_
_is_non_distributed_check_

这两个函数也要标一下，参考：

https://www.paddlepaddle.org.cn/documentation/docs/zh/develop/dev_guides/style_guide_and_references/type_annotations_specification_cn.html#ruguohanshushiyonglezhuangshiqi-zhuangshiqitongyangyinggaibiaozhuleixing

megemini · 2024-08-24T06:29:53Z

python/paddle/distributed/fleet/fleet.py

+    Any,
+    Iterable,
+    Literal,
+    Sequence,


从 collections.abc 导入 Sequence Iterable

megemini · 2024-08-24T06:31:31Z

python/paddle/distributed/fleet/fleet.py

+    from paddle import (
+        Tensor,
+    )


Suggested change

from paddle import (

Tensor,

)

from paddle import Tensor

megemini · 2024-08-24T06:46:39Z

python/paddle/distributed/fleet/fleet.py

-    def all_reduce(self, input, mode="sum"):
+    def all_reduce(
+        self,
+        input: int,


Suggested change

input: int,

input: Any,

应该不只是 int

megemini · 2024-08-24T06:56:26Z

python/paddle/distributed/fleet/fleet.py

+        Operator,
+        Parameter,
+        Program,
+        Scope,


不是这个 Scope ，是 from paddle.base.core import _Scope

megemini · 2024-08-24T10:18:12Z

python/paddle/distributed/fleet/fleet.py

+    TypedDict,
+)
+
+if TYPE_CHECKING:


把 if TYPE_CHECKING 这部分放到 80 行 __all__ 上面吧（注意前后留一个空行～），其他应该没啥问题了～辛苦 🤟

megemini

LGTM ～

typing

eb498e4

paddle-bot bot added the contributor External developers label Aug 21, 2024

luotao1 assigned luotao1 and SigureMo Aug 22, 2024

luotao1 added the HappyOpenSource Pro 进阶版快乐开源活动，更具挑战性的任务 label Aug 22, 2024

megemini reviewed Aug 22, 2024

View reviewed changes

megemini mentioned this pull request Aug 22, 2024

[Type Hints] 为公开 API 标注类型提示信息 #65008

Closed

fixed

2d1df7c

megemini reviewed Aug 22, 2024

View reviewed changes

Luohongzhige added 3 commits August 22, 2024 21:42

fixed

59c67b8

fixed

a623d07

fixed all_reduce

0bb8b8f

megemini reviewed Aug 23, 2024

View reviewed changes

Luohongzhige added 2 commits August 23, 2024 20:36

fixed

65ff7f0

Self

a195bdc

megemini reviewed Aug 24, 2024

View reviewed changes

Luohongzhige added 2 commits August 24, 2024 13:05

Self

74f80e0

Self

16e7f44

megemini reviewed Aug 24, 2024

View reviewed changes

fixed

61614f5

megemini reviewed Aug 24, 2024

View reviewed changes

fixed codestyle

4be53e8

megemini approved these changes Aug 24, 2024

View reviewed changes

SigureMo approved these changes Aug 25, 2024

View reviewed changes

SigureMo merged commit cdf544d into PaddlePaddle:develop Aug 25, 2024
28 of 29 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Typing][C-34][BUAA] Add type annotations for `python/paddle/distributed/fleet/fleet.py` #67624

[Typing][C-34][BUAA] Add type annotations for `python/paddle/distributed/fleet/fleet.py` #67624

Luohongzhige commented Aug 21, 2024

paddle-bot bot commented Aug 21, 2024

megemini left a comment

megemini Aug 22, 2024

megemini left a comment

megemini Aug 23, 2024

megemini Aug 23, 2024

megemini Aug 23, 2024

megemini Aug 23, 2024

megemini Aug 23, 2024

megemini Aug 23, 2024

megemini Aug 23, 2024

Luohongzhige Aug 23, 2024

megemini Aug 24, 2024

Luohongzhige Aug 24, 2024

megemini Aug 23, 2024

megemini Aug 23, 2024

megemini Aug 23, 2024

megemini left a comment

megemini Aug 24, 2024

megemini Aug 24, 2024

megemini Aug 24, 2024

megemini Aug 24, 2024

megemini Aug 24, 2024

Luohongzhige Aug 24, 2024

megemini left a comment

	self.strategy_compiler: StrategyCompiler = None
	self.strategy_compiler: StrategyCompiler \| None = None

	def shrink(self, threshold: int) -> None:
	def shrink(self, threshold: int \| None = None) -> None:

[Typing][C-34][BUAA] Add type annotations for python/paddle/distributed/fleet/fleet.py #67624

[Typing][C-34][BUAA] Add type annotations for python/paddle/distributed/fleet/fleet.py #67624

Conversation

Luohongzhige commented Aug 21, 2024

PR Category

PR Types

Description

paddle-bot bot commented Aug 21, 2024

megemini left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

megemini 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

megemini 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

megemini left a comment

Choose a reason for hiding this comment

[Typing][C-34][BUAA] Add type annotations for `python/paddle/distributed/fleet/fleet.py` #67624

[Typing][C-34][BUAA] Add type annotations for `python/paddle/distributed/fleet/fleet.py` #67624