[Typing][A-39,A-47,A-51] Add type annotations for `paddle/optimizer/optimizer.py` #65076

ooooo-create · 2024-06-12T10:46:31Z

PR Category

User Experience

PR Types

Improvements

Description

类型标注：
- paddle/optimizer/optimizer.py

Related links

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

@SigureMo @megemini

paddle-bot · 2024-06-12T10:46:35Z

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

python/paddle/optimizer/optimizer.py

megemini · 2024-06-12T12:38:24Z

python/paddle/optimizer/optimizer.py

-        name=None,
-    ):
+        learning_rate: float | LRScheduler,
+        parameters: list[Tensor] | tuple[Tensor] | None = None,


建议用 Sequence[Tensor] ～另外，如果用 tuple 的话，应该是 tuple[Tensor, ...]

python/paddle/optimizer/optimizer.py

megemini · 2024-06-12T12:54:21Z

python/paddle/optimizer/optimizer.py

+        startup_program: Program | None = None,
+        parameters: list[Tensor] | list[str] | None = None,
+        no_grad_set: set[Tensor] | set[str] | None = None,
+    ) -> tuple[list[tuple[Tensor, Tensor]], list[Any]]:


这里是不是写反了？optimize_ops 在前面， params_grads 在后面？

已修改~，后面的Any也改成了Operator

SigureMo · 2024-06-13T05:27:48Z

python/paddle/optimizer/optimizer.py

@@ -15,7 +15,7 @@
 import logging


没用 PEP 563 么？

PEP 563 是对于类中的注解如果需要使用未定义的类型不用转换成字符串嘛？这样好像没有使用到 PEP 563

不止，| 也必须要 PEP 563 才能在 3.8 用

SigureMo · 2024-06-13T06:46:01Z

python/paddle/optimizer/optimizer.py

+        Args:
+            None


这里需要确认下预览

是有点问题的，没有参数就不写Args:叭~

… pr/ooooo-create/65076

megemini · 2024-06-13T10:49:41Z

python/paddle/optimizer/optimizer.py

@@ -111,24 +122,24 @@ class Optimizer:
    Args:
        learning_rate (float|LRScheduler): The learning rate used to update ``Parameter``.
            It can be a float value or any subclass of ``LRScheduler`` .
-        parameters (list|tuple, optional): List/Tuple of ``Tensor`` names to update to minimize ``loss``. \
+        parameters (list[Tensor]|tuple[Tensor,...]|None, optional): List/Tuple of ``Tensor`` names to update to minimize ``loss``. \


这里只写 list|tuple|None 就可以了～因为 parameters 的输入其实还可以是类似 list[dict] 的形式，这里保持简洁，就不展开写了吧～

可以参考 https://github.com/cattidea/paddlepaddle-stubs/tree/main/paddle-stubs/optimizer ，看一下 parameters 在签名里面的写法～

对了，Optimizer 里面还有几个属性，这里需要补充一下～

https://github.com/cattidea/paddlepaddle-stubs/blob/e19bc56e035e56d5d6490c95d94570eb8eeb861f/paddle-stubs/optimizer/optimizer.pyi#L22-L25

python/paddle/_typing/shape.py

SigureMo · 2024-06-13T13:40:09Z

python/paddle/optimizer/optimizer.py

+    regularization: Any
+    helper: Any
+    clear_gradients: Any


这几个有更细致的 type 么？

SigureMo · 2024-06-14T02:31:57Z

python/paddle/optimizer/optimizer.py

-        name=None,
-    ):
+        learning_rate: float | LRScheduler,
+        parameters: Sequence[Tensor] | None = None,


这里支持分组的 parameter 吧？

https://github.com/cattidea/paddlepaddle-stubs/blob/e19bc56e035e56d5d6490c95d94570eb8eeb861f/paddle-stubs/optimizer/sgd.pyi#L17

SGD、Adam 等示例代码中用到的优化器在这个 PR 一起做了吧，不然检测不到问题

SigureMo · 2024-06-14T04:51:29Z

python/paddle/optimizer/adam.py

@@ -408,7 +426,7 @@ def _append_optimize_op(self, block, param_and_grad):

    @imperative_base.no_grad


这个装饰器没加类型提示吧，应该会丢失掉类型信息，可以参考 #64954 加一下

这个实现和其他的不太一样，需要自己看一下

SigureMo · 2024-06-14T04:52:44Z

python/paddle/optimizer/adam.py

@@ -30,6 +33,21 @@
 )
 from .optimizer import Optimizer

+if TYPE_CHECKING:
+    from typing_extensions import NotRequired


这个不用放这下面

SigureMo · 2024-06-14T04:54:07Z

python/paddle/optimizer/adam.py

+    from .lr import LRScheduler
+    from .optimizer import ParameterConfig
+
+    class AdamParameterConfig(ParameterConfig):


Suggested change

class AdamParameterConfig(ParameterConfig):

class _AdamParameterConfig(_ParameterConfig):

非暴露，加个下划线吧，可以不用放到 TYPE_CHECKING 下面

我把 ParameterConfig 定义在 optimizer.py 的 TYPE_CHECKING 下面（里面用到很多type_checking里导入的类），这里不放在 TYPE_CHECKING 下面是不是会找不到 ParameterConfig 啊

😂 肯定是要改一起改啊

SigureMo · 2024-06-14T06:55:06Z

Static Check 报错也需要看下

megemini

ci 中的错误

2024-06-14 13:19:52 ----------------Check results--------------------
2024-06-14 13:19:52 paddle.optimizer.Adam:code-example2
2024-06-14 13:19:52 <string>:24:16: error: Argument "parameters" to "Adam" has incompatible type "List[Dict[str, Any]]"; expected "Union[Sequence[Tensor], Sequence[AdamParameterConfig], None]"  [arg-type]
2024-06-14 13:19:52 Found 1 error in 1 file (checked 1 source file)
2024-06-14 13:19:52 
2024-06-14 13:19:52 
2024-06-14 13:19:52 paddle.optimizer.Optimizer:1
2024-06-14 13:19:52 <string>:19:16: error: Argument "parameters" to "SGD" has incompatible type "List[Dict[str, Any]]"; expected "Union[Sequence[Tensor], Sequence[ParameterConfig], None]"  [arg-type]
2024-06-14 13:19:52 Found 1 error in 1 file (checked 1 source file)

应该是依赖了 python/paddle/nn/layer/layers.py ，如：

            >>> import paddle
            >>> linear = paddle.nn.Linear(10, 10)
            >>> inp = paddle.uniform(shape=[10, 10], min=-0.1, max=0.1)
            >>> out = linear(inp)
            >>> loss = paddle.mean(out)
            >>> adam = paddle.optimizer.Adam(learning_rate=0.1,
            ...         parameters=linear.parameters()) # 此处需要优先标注 Layer 和 Linear

@liyongchao911 是否可以先做这个任务？

megemini · 2024-06-14T07:02:03Z

python/paddle/optimizer/optimizer.py

+        startup_program: Program | None = None,
+        parameters: list[Tensor] | list[str] | None = None,
+        no_grad_set: set[Tensor] | set[str] | None = None,
+        callbacks: list[Callback] | None = None,


这里是不是可以展开：

callbacks: list[Callback[[Block, dict[str, Tensor | paddle.core.OpDesc], None]] | None = None

def append_backward 里

callbacks(list[callable object]|tuple[callable object], optional): List/Tuple of callback functions. The callbacks are used for doing some custom jobs during backward part building. All callable objects in it will be invoked once each time a new gradient operator is added into the program. The callable object must have two input parameters: ``block`` and ``context`` . The ``block`` is the :ref:`api_guide_Block_en` which the new gradient operator will be added to. The ``context`` is a map, whose keys are gradient Tensor names and values are corresponding original :ref:`api_guide_tensor_en` . In addition to this, the ``context`` has another special key-value pair: the key is string ``__current_op_desc__`` and the value is the op_desc of the gradient operator who has just triggered the callable object. Default: None.

参考

def error_clip_callback(block, context):

另外，这里只能用 list，下面有一句判断 assert isinstance(callbacks, list)

@SigureMo 看看是否需要？

callbacks: list[Callback[[Block, dict[str, Tensor | paddle.core.OpDesc], None]] | None = None

Callback 不是泛型，需要改成泛型才能这样写

另外，这里只能用 list，下面有一句判断 assert isinstance(callbacks, list)

这个位置，必须有默认值吧

callbacks: list[Callback[[Block, dict[str, Tensor | paddle.core.OpDesc], None]] | None = None

Callback 不是泛型，需要改成泛型才能这样写

改成 Callable 就可以了叭，一开始标错了

这里应该是 Callable 还是 Callback 呢？

另外这里有 Block 和 OpDesc，大多是老 IR 的概念了，相关逻辑在 PIR 下是否有适配过呢？

这里应该是 Callable 还是 Callback 呢？

应该是 Callable，Callback 是模型训练等调用的

是 Callable !

当时在跟踪这个标注的时候，还说要改为 Callable，结果还是忘了～抱歉～～～

另外，这里只能用 list，下面有一句判断 assert isinstance(callbacks, list)

这个位置，必须有默认值吧

if callbacks is None: callbacks = [paddle.nn.clip.error_clip_callback] else: assert isinstance(callbacks, list)

代码里面这样处理的，默认是 None ，会手动创建～

megemini · 2024-06-14T07:31:42Z

@ooooo-create 那啥，这个 PR 里面做了好几个任务，能不能在 ISSUE #65008 里面报一下名，然后改一下这个 PR 的标题 [Typing][A-xx,A-xx,A-xx] ... ，以防跟其他同学做重了～ 🙏

SigureMo · 2024-06-14T13:40:23Z

@liyongchao911 是否可以先做这个任务？

还是我来吧……

SigureMo · 2024-06-17T02:08:41Z

#65190 Layer 已合，可以继续推进下了～

… pr/ooooo-create/65076

SigureMo · 2024-06-17T05:13:31Z

python/paddle/base/dygraph/base.py

+def no_grad(
+    func: Callable[_InputT, _RetT] | None = None
+) -> Callable[_InputT, _RetT] | ContextManager:


注意示例代码里的两种用法，应该对应两种签名，可以使用 overload～

Paddle 代码库搜 @overload 就可以找到很多参考

SigureMo · 2024-06-17T05:18:33Z

python/paddle/optimizer/optimizer.py

+        startup_program: Program | None = None,
+        parameters: list[Tensor] | list[str] | None = None,
+        no_grad_set: set[Tensor] | set[str] | None = None,
+        callbacks: list[Callback] | None = None,


应该是 Callable，Callback 是模型训练等调用的

所以这里是 Callable 还是 Callback 呢？是 Callable 的话，这里好像还没改？

SigureMo · 2024-06-17T05:19:27Z

python/paddle/optimizer/optimizer.py

                to be updated. The default value is None.
-            callbacks (list, optional): list of callable objects to run when appending backward
+            callbacks (list[Callback]|None, optional): list of callable objects to run when appending backward


如果是 Callable，这里是有问题的

我们不强求修改 docstring，因为按理说这里的内容是应该退场的，而且修改后会导致同时有多种奇怪的写法，用户会更加困惑

SigureMo · 2024-06-17T06:19:59Z

python/paddle/optimizer/optimizer.py

+                    paddle.framework.Block,
+                    dict[str, Tensor | paddle.core.OpDesc],


话说 PIR 是不是根本就没用这个 callbacks？这里直接 Callable[[...], Any] 吧，尚不清楚 PIR 下形态如何，没必要在这里暴露老 IR 的概念

返回值确定是 None 么？那就 Callable[[...], None]

ooooo-create · 2024-06-17T06:49:53Z

            >>> sgd = paddle.optimizer.SGD(
            ...     learning_rate=0.1,
            ...     parameters=[{
            ...         'params': linear_1.parameters()
            ...     }, {
            ...         'params': linear_2.parameters(),
            ...         'weight_decay': 0.001,
            ...         'learning_rate': 0.1
            ...     }],
            ...     weight_decay=0.01)

刚刚看 ci 里面显示 parameters 是 has incompatible type List[object]，这个要怎么修改呀～

SigureMo · 2024-06-17T07:00:18Z

刚刚看 ci 里面显示 parameters 是 has incompatible type List[object]，这个要怎么修改呀～

# type: ignore 吧，mypy 太拉了

SigureMo · 2024-06-17T07:41:00Z

python/paddle/optimizer/adam.py

+            >>> adam = paddle.optimizer.Adam(
            ...     learning_rate=0.1,
-            ...     parameters=[{
-            ...         'params': linear_1.parameters()
-            ...     }, {
-            ...         'params': linear_2.parameters(),
-            ...         'weight_decay': 0.001,
-            ...         'learning_rate': 0.1,
-            ...         'beta1': 0.8
-            ...     }],
+            ...     parameters=[{  # type: ignore
+            ...         'params': linear_1.parameters()  # type: ignore
+            ...     }, {  # type: ignore
+            ...         'params': linear_2.parameters(),  # type: ignore
+            ...         'weight_decay': 0.001,  # type: ignore
+            ...         'learning_rate': 0.1,  # type: ignore
+            ...         'beta1': 0.8  # type: ignore
+            ...     }],  # type: ignore


这个……离谱了吧……看看加在哪可以解决，这每行都加不太能接受……

本地跑 tools/type_checking.py 可以快速验证，如：

python tools/type_checking.py paddle.optimizer.lr.CosineAnnealingWarmRestarts

SigureMo

LGTMeow

…ptimizer.py` (PaddlePaddle#65076)

[Typing][A-47] Add typing annotations for paddle/optimizer/optimizer.py"

bf4b1ec

paddle-bot bot added the contributor External developers label Jun 12, 2024

zrr1999 reviewed Jun 12, 2024

View reviewed changes

python/paddle/optimizer/optimizer.py Outdated Show resolved Hide resolved

python/paddle/optimizer/optimizer.py Outdated Show resolved Hide resolved

megemini reviewed Jun 12, 2024

View reviewed changes

megemini mentioned this pull request Jun 12, 2024

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

Closed

SigureMo mentioned this pull request Jun 12, 2024

[Typing][A-48,A-49,A-50][debug] Add type annotations for 3 optimizers (RAdam, RMSProp, Rprop) #65085

Merged

luotao1 added the HappyOpenSource 快乐开源活动issue与PR label Jun 13, 2024

luotao1 assigned luotao1 and SigureMo Jun 13, 2024

update

3712142

SigureMo reviewed Jun 13, 2024

View reviewed changes

add PEP 563

bde8a8d

SigureMo reviewed Jun 13, 2024

View reviewed changes

ooooo-create added 2 commits June 13, 2024 18:11

add type hint for paddle.static.data

3977d61

Merge branch 'develop' of https://github.com/PaddlePaddle/Paddle into…

dbd7fdc

… pr/ooooo-create/65076

megemini reviewed Jun 13, 2024

View reviewed changes

update

5b40e80

ooooo-create requested a review from gouzil as a code owner June 13, 2024 10:51

SigureMo reviewed Jun 13, 2024

View reviewed changes

python/paddle/_typing/shape.py Outdated Show resolved Hide resolved

update

22658fd

SigureMo reviewed Jun 13, 2024

View reviewed changes

python/paddle/_typing/shape.py Outdated Show resolved Hide resolved

ooooo-create added 3 commits June 13, 2024 19:19

update

3c4b045

update

e6bba9f

update

4bff18e

SigureMo reviewed Jun 13, 2024

View reviewed changes

ooooo-create added 3 commits June 13, 2024 22:16

update

e492299

update

0333fa6

update

2612591

SigureMo reviewed Jun 14, 2024

View reviewed changes

ooooo-create added 4 commits June 14, 2024 12:04

add type annotations for Adam, SGD

d5bf9ef

update import

c9c17e3

update

c8ab7ee

update

050e10b

SigureMo reviewed Jun 14, 2024

View reviewed changes

update

160e808

megemini reviewed Jun 14, 2024

View reviewed changes

ooooo-create changed the title ~~[Typing][A-47] Add type annotations for paddle/optimizer/optimizer.py~~ [Typing][A-39,A-47,A-51] Add type annotations for paddle/optimizer/optimizer.py Jun 14, 2024

SigureMo mentioned this pull request Jun 16, 2024

[Typing][A-69] Add type annotations for LR Schedulers #65209

Merged

Merge branch 'develop' of https://github.com/PaddlePaddle/Paddle into…

011966d

… pr/ooooo-create/65076

SigureMo reviewed Jun 17, 2024

View reviewed changes

ooooo-create added 2 commits June 17, 2024 14:10

update for review

9789477

update

2e7ecee

SigureMo reviewed Jun 17, 2024

View reviewed changes

update

2c790ac

ooooo-create added 2 commits June 17, 2024 15:16

add # type: ignore

e728415

add return None

851f46f

SigureMo reviewed Jun 17, 2024

View reviewed changes

update

1c294ea

SigureMo approved these changes Jun 17, 2024

View reviewed changes

SigureMo merged commit 417117e into PaddlePaddle:develop Jun 17, 2024
32 of 33 checks passed

megemini mentioned this pull request Jun 18, 2024

[Typing] not render type ignore PaddlePaddle/docs#6680

Merged

co63oc pushed a commit to co63oc/Paddle that referenced this pull request Jun 25, 2024

[Typing][A-39,A-47,A-51] Add type annotations for `paddle/optimizer/o…

91cf787

…ptimizer.py` (PaddlePaddle#65076)

		@@ -408,7 +426,7 @@ def _append_optimize_op(self, block, param_and_grad):

		@imperative_base.no_grad

	class AdamParameterConfig(ParameterConfig):
	class _AdamParameterConfig(_ParameterConfig):

		paddle.framework.Block,
		dict[str, Tensor \| paddle.core.OpDesc],

[Typing][A-39,A-47,A-51] Add type annotations for paddle/optimizer/optimizer.py #65076

[Typing][A-39,A-47,A-51] Add type annotations for paddle/optimizer/optimizer.py #65076

Conversation

ooooo-create commented Jun 12, 2024

PR Category

PR Types

Description

Related links

paddle-bot bot commented Jun 12, 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

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

SigureMo Jun 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SigureMo commented Jun 14, 2024

megemini left a comment • edited Loading

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

ooooo-create Jun 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

megemini commented Jun 14, 2024

SigureMo commented Jun 14, 2024

SigureMo commented Jun 17, 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

ooooo-create commented Jun 17, 2024

SigureMo commented Jun 17, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SigureMo left a comment

Choose a reason for hiding this comment

[Typing][A-39,A-47,A-51] Add type annotations for `paddle/optimizer/optimizer.py` #65076

[Typing][A-39,A-47,A-51] Add type annotations for `paddle/optimizer/optimizer.py` #65076

SigureMo Jun 14, 2024 •

edited

Loading

megemini left a comment •

edited

Loading

ooooo-create Jun 14, 2024 •

edited

Loading