Skip to content

[Typing][A-20,A-61] Add type annotations for paddle/nn/{layer,functional}/loss.py #65376

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 13 commits into from
Jun 26, 2024
Merged

[Typing][A-20,A-61] Add type annotations for paddle/nn/{layer,functional}/loss.py #65376

merged 13 commits into from
Jun 26, 2024

Conversation

Asthestarsfalll
Copy link
Contributor

PR Category

User Experience

PR Types

Improvements

Description

@paddle-bot Paddle-bot
Copy link

paddle-bot bot commented Jun 22, 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 22, 2024
@megemini megemini mentioned this pull request Jun 22, 2024
Closed
SigureMo
SigureMo reviewed Jun 24, 2024
View reviewed changes
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.

比较建议 functional 和相关 layer 一并推进，或者先完成 functional 再完成 layer，以确保两者代码风格的一致性，并且推荐在 functional 定义 Alias，并在 layer 中 import 复用，以保持合理的依赖关系

@Asthestarsfalll 方便的话这个 PR 里推进 loss 的 functional 和 layer 吧

cc @gsq7474741

@SigureMo
Copy link
Member

#65379 这种大 PR 也是建议拆分成 functional-layer pair 的形式的，PR 越大越不容易推进合入，合理地合并 PR 更利于 review

@luotao1 luotao1 added the HappyOpenSource 快乐开源活动issue与PR label Jun 24, 2024
@Asthestarsfalll
Copy link
Contributor Author

#65379 这种大 PR 也是建议拆分成 functional-layer pair 的形式的，PR 越大越不容易推进合入，合理地合并 PR 更利于 review

好的

@SigureMo SigureMo changed the title [Typing][A-20] Add type annotations for paddle/nn/layer/loss.py [Typing][A-20] Add type annotations for paddle/nn/{layer,functional}/loss.py Jun 24, 2024
@SigureMo SigureMo changed the title [Typing][A-20] Add type annotations for paddle/nn/{layer,functional}/loss.py [Typing][A-20,A-61] Add type annotations for paddle/nn/{layer,functional}/loss.py Jun 24, 2024
SigureMo
SigureMo previously approved these changes Jun 25, 2024
View reviewed changes
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 mentioned this pull request Jun 25, 2024
Merged
@SigureMo
Copy link
Member 

2024-06-25 21:27:02 **************************************************************
2024-06-25 21:27:02 Please find RD for approval first, and then find TPM for approval.
2024-06-25 21:27:02 0. You must have one RD (XiaoguangHu01, jeff41404 or qingqing01) approval for API change.
2024-06-25 21:27:02 
2024-06-25 21:27:02 There are 1 approved errors.
2024-06-25 21:27:02 **************************************************************

怎么会有 API diff 呢？

@Asthestarsfalll
Copy link
Contributor Author 

2024-06-25 21:27:02 **************************************************************
2024-06-25 21:27:02 Please find RD for approval first, and then find TPM for approval.
2024-06-25 21:27:02 0. You must have one RD (XiaoguangHu01, jeff41404 or qingqing01) approval for API change.
2024-06-25 21:27:02 
2024-06-25 21:27:02 There are 1 approved errors.
2024-06-25 21:27:02 **************************************************************

怎么会有 API diff 呢？

是因为参数默认值改了吗？我使用代码生成0.00001会变成1e-5

@SigureMo
Copy link
Member

是因为参数默认值改了吗？我使用代码生成0.00001会变成1e-5

哪个 API 呢？

@SigureMo
Copy link
Member

利用最新 commit 打印一下 diff 再看一下吧，现在看不出来

@SigureMo
Copy link
Member 

2024-06-26 00:03:13 api_spec_diff: API Difference is: 
2024-06-26 00:03:13 - paddle.nn.functional.triplet_margin_loss (ArgSpec(args=['input', 'positive', 'negative', 'margin', 'p', 'epsilon', 'swap', 'reduction', 'name'], varargs=None, varkw=None, defaults=(1.0, 2, 1e-06, False, 'mean', None), kwonlyargs=[], kwonlydefaults=None), ('document', '**********'))
2024-06-26 00:03:13 + paddle.nn.functional.triplet_margin_loss (ArgSpec(args=['input', 'positive', 'negative', 'margin', 'p', 'epsilon', 'swap', 'reduction', 'name'], varargs=None, varkw=None, defaults=(1.0, 2.0, 1e-06, False, 'mean', None), kwonlyargs=[], kwonlydefaults=None), ('document', '**********'))
2024-06-26 00:03:13 ?                                                                                                                                                                                            ++

找到了

@SigureMo
Revert "show api_spec_diff"
33d7660 
This reverts commit 8fa764f.
SigureMo
SigureMo previously approved these changes Jun 25, 2024
View reviewed changes
@SigureMo SigureMo merged commit 8d2b0d2 into PaddlePaddle:develop Jun 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SigureMo SigureMo SigureMo approved these changes

Assignees

@luotao1 luotao1

@SigureMo SigureMo

Labels
contributor External developers HappyOpenSource 快乐开源活动issue与PR
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Asthestarsfalll @SigureMo @luotao1