[Typing][C-18,C-68,C-77] Add type annotations for `distributed/communication/group.py` and `incubate/nn/functional/{block_multihead_attention,fused_transformer}.py` #67677

Lans1ot · 2024-08-23T08:33:27Z

PR Category

User Experience

PR Types

Improvements

Description

ProcessGroup是从这里找到的，不知道对不对
fused_multi_transformer中，未找到norm_type的取值范围，就先只允许layernorm了
compute_dtype也不知道取值范围

paddle-bot · 2024-08-23T08:33:32Z

你的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 · 2024-08-23T10:35:19Z

python/paddle/distributed/communication/group.py

@@ -154,7 +167,7 @@ def is_initialized():
    return _GroupManager.global_group_id in _GroupManager.group_map_by_id


-def destroy_process_group(group=None):
+def destroy_process_group(group=None) -> None:


Suggested change

def destroy_process_group(group=None) -> None:

def destroy_process_group(group: Group | None = None) -> None:

megemini · 2024-08-23T10:45:14Z

python/paddle/distributed/communication/group.py

        if self.rank < 0:
            return False
        if self.nranks < 2:
            return False
        return True

-    def get_group_rank(self, rank):
+    def get_group_rank(self, rank) -> int:


Suggested change

def get_group_rank(self, rank) -> int:

def get_group_rank(self, rank: int) -> int | Literal[-1]:

megemini · 2024-08-23T10:46:03Z

python/paddle/distributed/communication/group.py

@@ -196,7 +209,7 @@ def destroy_process_group(group=None):
        del _GroupManager.group_map_by_id[group.id]


-def get_group(id=0):
+def get_group(id=0) -> Group:


Suggested change

def get_group(id=0) -> Group:

def get_group(id: int=0) -> Group:

megemini · 2024-08-23T10:46:35Z

python/paddle/distributed/communication/group.py

@@ -255,7 +268,7 @@ def _sync_comm_stream(tensor, ring_id=0):
        )


-def wait(tensor, group=None, use_calc_stream=True):
+def wait(tensor, group=None, use_calc_stream=True) -> None:


Suggested change

def wait(tensor, group=None, use_calc_stream=True) -> None:

def wait(tensor: Tensor, group: Group | None=None, use_calc_stream:bool=True) -> None:

megemini · 2024-08-23T10:46:50Z

python/paddle/distributed/communication/group.py

@@ -291,7 +304,7 @@ def wait(tensor, group=None, use_calc_stream=True):
        _sync_comm_stream(tensor, ring_id)


-def barrier(group=None):
+def barrier(group=None) -> None:


Suggested change

def barrier(group=None) -> None:

def barrier(group:Group|None=None) -> None:

megemini · 2024-08-23T12:08:12Z

python/paddle/incubate/nn/functional/fused_transformer.py

+    _activation_function = Literal["relu", "gelu"]
+    _mode = Literal["upscale_in_train", "downscale_in_infer"]
+    _norm_type = Literal["layernorm"]


CamelCase 方式， _mode 改为 _Mode

megemini · 2024-08-23T12:08:34Z

python/paddle/incubate/nn/functional/fused_transformer.py

+    ln2_bias: Tensor | None = None,
+    dropout1_rate: float = 0.5,
+    dropout2_rate: float = 0.5,
+    activation: _activation_function = "relu",


这里为什么只能是 "relu", "gelu" ？

抱歉这个武断了，看fused_linear_activation有这个限制，以为就都有这个限制了

megemini · 2024-08-23T12:12:01Z

python/paddle/incubate/nn/functional/fused_transformer.py

+    ffn2_biases: Sequence[Tensor],
+    pre_layer_norm: bool = True,
+    epsilon: float = 1e-05,
+    residual_alpha: int = 1.0,


Suggested change

residual_alpha: int = 1.0,

residual_alpha: float = 1.0,

megemini · 2024-08-23T12:13:39Z

python/paddle/incubate/nn/functional/fused_transformer.py

+    mode: _mode = 'upscale_in_train',
+    trans_qkvw: bool = True,
+    ring_id: int = -1,
+    norm_type: _norm_type = "layernorm",


为什么只能是 "layernorm ？

megemini · 2024-08-23T12:32:31Z

python/paddle/incubate/nn/functional/fused_transformer.py

+    use_neox_rotary_style: bool = False,
+    gqa_group_size: int = -1,
+    name: str | None = None,
+) -> Tensor | tuple(Tensor, Tensor):


需要用 overload ，如果 cache_kvs 是 None 则输出 Tensor ，否则输出 tuple[Tensor, Sequence[Tensor]]

megemini · 2024-08-23T12:41:10Z

fused_multi_transformer中，未找到norm_type的取值范围，就先只允许layernorm了
compute_dtype也不知道取值范围

用 str 吧～

关联 #65008

megemini · 2024-08-24T04:47:34Z

python/paddle/incubate/nn/functional/block_multihead_attention.py

+if TYPE_CHECKING:
+    from paddle import Tensor
+
+    _quant_round_type = Literal[0, 1]


Suggested change

_quant_round_type = Literal[0, 1]

_QuantRoundType: TypeAlias = Literal[0, 1]

megemini · 2024-08-24T04:48:14Z

python/paddle/incubate/nn/functional/fused_transformer.py

@@ -12,6 +12,10 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.

+from __future__ import annotations
+
+from typing import TYPE_CHECKING, Literal, Sequence, overload


从 collections.abc 导入 Sequence

megemini · 2024-08-24T04:48:58Z

python/paddle/incubate/nn/functional/fused_transformer.py

+    ffn1_biases: Sequence[Tensor],
+    ffn2_weights: Sequence[Tensor],
+    ffn2_biases: Sequence[Tensor],
+    pre_layer_norm: bool = True,


Suggested change

pre_layer_norm: bool = True,

pre_layer_norm: bool = ...,

注意 overload 中默认值的写法

megemini · 2024-08-24T04:49:39Z

python/paddle/incubate/nn/functional/fused_transformer.py

+    use_neox_rotary_style: bool = False,
+    gqa_group_size: int = -1,
+    name: str | None = None,
+) -> tuple(Tensor, Sequence[Tensor]): ...


Suggested change

) -> tuple(Tensor, Sequence[Tensor]): ...

) -> tuple[Tensor, Sequence[Tensor]]: ...

megemini · 2024-08-24T04:49:59Z

python/paddle/incubate/nn/functional/fused_transformer.py

+    name: str | None = None,
+) -> tuple(Tensor, Sequence[Tensor]): ...
+
+
 def fused_multi_transformer(


原函数保持不变，不要动默认值～ overload 中写默认值是没有意义的，运行时获取不到～

megemini

TypeAlias 从 typing_extensions 导入

megemini

LGTM ～

SigureMo

LGTMeow

参数类型信息补全

0eff3f9

luotao1 assigned luotao1 and SigureMo Aug 23, 2024

修改语法错误

32d24dc

luotao1 added contributor External developers HappyOpenSource Pro 进阶版快乐开源活动，更具挑战性的任务 labels Aug 23, 2024

megemini mentioned this pull request Aug 23, 2024

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

Closed

megemini reviewed Aug 23, 2024

View reviewed changes

根据review修改错误

1dcba51

megemini reviewed Aug 24, 2024

View reviewed changes

根据review进行修改

2331aad

megemini reviewed Aug 26, 2024

View reviewed changes

修改引用信息

2b0e9b2

megemini approved these changes Aug 26, 2024

View reviewed changes

SigureMo approved these changes Aug 26, 2024

View reviewed changes

luotao1 merged commit f3c6db7 into PaddlePaddle:develop Aug 26, 2024
28 of 29 checks passed

Lans1ot mentioned this pull request Sep 6, 2024

[WeeklyReports] 2024.08.24~2024.09.06 周报收集 PFCCLab/Starter#381

Closed

33 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Typing][C-18,C-68,C-77] Add type annotations for `distributed/communication/group.py` and `incubate/nn/functional/{block_multihead_attention,fused_transformer}.py` #67677

[Typing][C-18,C-68,C-77] Add type annotations for `distributed/communication/group.py` and `incubate/nn/functional/{block_multihead_attention,fused_transformer}.py` #67677

Lans1ot commented Aug 23, 2024 •

edited

Loading

paddle-bot bot commented 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

megemini Aug 23, 2024

Lans1ot Aug 24, 2024

megemini Aug 23, 2024

megemini Aug 23, 2024

megemini Aug 23, 2024

megemini commented Aug 23, 2024

megemini Aug 24, 2024

megemini Aug 24, 2024

megemini Aug 24, 2024

megemini Aug 24, 2024

megemini Aug 24, 2024

megemini left a comment

megemini left a comment

SigureMo left a comment

	def destroy_process_group(group=None) -> None:
	def destroy_process_group(group: Group \| None = None) -> None:

	def get_group_rank(self, rank) -> int:
	def get_group_rank(self, rank: int) -> int \| Literal[-1]:

	def get_group(id=0) -> Group:
	def get_group(id: int=0) -> Group:

	def wait(tensor, group=None, use_calc_stream=True) -> None:
	def wait(tensor: Tensor, group: Group \| None=None, use_calc_stream:bool=True) -> None:

	def barrier(group=None) -> None:
	def barrier(group:Group\|None=None) -> None:

	_quant_round_type = Literal[0, 1]
	_QuantRoundType: TypeAlias = Literal[0, 1]

	) -> tuple(Tensor, Sequence[Tensor]): ...
	) -> tuple[Tensor, Sequence[Tensor]]: ...

[Typing][C-18,C-68,C-77] Add type annotations for distributed/communication/group.py and incubate/nn/functional/{block_multihead_attention,fused_transformer}.py #67677

[Typing][C-18,C-68,C-77] Add type annotations for distributed/communication/group.py and incubate/nn/functional/{block_multihead_attention,fused_transformer}.py #67677

Conversation

Lans1ot commented Aug 23, 2024 • edited Loading

PR Category

PR Types

Description

paddle-bot bot commented Aug 23, 2024

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 commented Aug 23, 2024

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

megemini left a comment

Choose a reason for hiding this comment

SigureMo left a comment

Choose a reason for hiding this comment

[Typing][C-18,C-68,C-77] Add type annotations for `distributed/communication/group.py` and `incubate/nn/functional/{block_multihead_attention,fused_transformer}.py` #67677

[Typing][C-18,C-68,C-77] Add type annotations for `distributed/communication/group.py` and `incubate/nn/functional/{block_multihead_attention,fused_transformer}.py` #67677

Lans1ot commented Aug 23, 2024 •

edited

Loading