Skip to content
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

[PaddleHackathon No.14] #41183

Merged
merged 21 commits into from
Jun 17, 2022
Merged

[PaddleHackathon No.14] #41183

merged 21 commits into from
Jun 17, 2022

Conversation

yangguohao
Copy link
Contributor

@yangguohao yangguohao commented Mar 30, 2022 •
edited
Loading

PR types

New features

PR changes

APIs

Describe

解决了issues：#40319
添加了paddle.nn.MultiLabelSoftMarginLoss以及paddle.nn.functional.multi_label_soft_margin_loss的函数
设计文档：PaddlePaddle/community#53
中文文档PR链接:PaddlePaddle/docs#4651

@CLAassistant
Copy link

CLAassistant commented Mar 30, 2022 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@paddle-bot-old paddle-bot-old bot added contributor External developers status: proposed labels Mar 30, 2022
@paddle-bot-old
Copy link

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

@dingjiaweiww
Copy link
Contributor

在PR被review前请先通过CI噢~

@TCChenlong TCChenlong mentioned this pull request Apr 6, 2022
Closed
@paddle-bot-old
Copy link

Sorry to inform you that e2763d5's CIs have passed for more than 7 days. To prevent PR conflicts, you need to re-run all CIs manually.

@paddle-bot-old
Copy link

PR格式检查通过，你的PR将接受Paddle专家以及开源社区的review，请及时关注PR动态。
The format inspection passed. Your PR will be reviewed by experts of Paddle and developers from the open-source community. Stay tuned.

betterpig
betterpig requested changes Apr 19, 2022
View reviewed changes
Copy link
Contributor

@betterpig betterpig left a comment

Choose a reason for hiding this comment

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

主要问题：

  1. 两个单测文件要去掉，如果没修改的话。
  2. 尽量将paddle.fluid改为paddle.static

python/paddle/fluid/tests/unittests/test_frac_api.py Outdated Show resolved Hide resolved
python/paddle/fluid/tests/unittests/test_quantile_and_nanquantile.py Outdated Show resolved Hide resolved
python/paddle/nn/functional/loss.py Outdated Show resolved Hide resolved
python/paddle/nn/functional/loss.py Outdated Show resolved Hide resolved
python/paddle/fluid/tests/unittests/test_multilabel_margin_loss.py Outdated Show resolved Hide resolved
python/paddle/fluid/tests/unittests/test_multilabel_margin_loss.py Outdated Show resolved Hide resolved
python/paddle/fluid/tests/unittests/test_multilabel_margin_loss.py Outdated Show resolved Hide resolved
python/paddle/fluid/tests/unittests/test_multilabel_margin_loss.py Outdated Show resolved Hide resolved
python/paddle/fluid/tests/unittests/test_multilabel_margin_loss.py Outdated

places = [fluid.CPUPlace()]
if fluid.core.is_compiled_with_cuda():
places.append(fluid.CUDAPlace(0))
Copy link
Contributor

Choose a reason for hiding this comment

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

看能否改为 paddle.static.cpu_places() 和 paddle.static.cuda_places(0)

TCChenlong
TCChenlong reviewed May 6, 2022
View reviewed changes
python/paddle/nn/layer/loss.py Outdated
return F.multi_label_soft_margin_loss(
input,
label,
reduction=self.reduction,
Copy link
Contributor

Choose a reason for hiding this comment

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

这里建议严谨一点， weight 在前，reduction 在后吧

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已修改

python/paddle/nn/layer/loss.py Outdated Show resolved Hide resolved
python/paddle/nn/layer/loss.py Outdated Show resolved Hide resolved
python/paddle/nn/layer/loss.py Outdated Show resolved Hide resolved
python/paddle/nn/layer/loss.py Outdated Show resolved Hide resolved
@TCChenlong TCChenlong requested a review from DDDivano May 9, 2022 06:52
DDDivano
DDDivano previously approved these changes May 12, 2022
View reviewed changes
TCChenlong
TCChenlong previously approved these changes May 30, 2022
View reviewed changes
Copy link
Contributor

@TCChenlong TCChenlong left a comment

Choose a reason for hiding this comment

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

LGTM

@yangguohao yangguohao dismissed stale reviews from TCChenlong and DDDivano via 4b7aee7 June 1, 2022 14:37
@TCChenlong
Copy link
Contributor

@yangguohao 请解决一下 Code Style 问题

TCChenlong
TCChenlong approved these changes Jun 17, 2022
View reviewed changes
Copy link
Contributor

@TCChenlong TCChenlong left a comment

Choose a reason for hiding this comment

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

LGTM

XiaoguangHu01
XiaoguangHu01 approved these changes Jun 17, 2022
View reviewed changes
Copy link
Contributor

@XiaoguangHu01 XiaoguangHu01 left a comment

Choose a reason for hiding this comment

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

LGTM

@jeff41404
Copy link
Contributor

code is fine, but in RFC the description of parameters of paddle.nn.MultiLabelSoftMarginLoss is not correct which do not have input and target

@jeff41404 jeff41404 merged commit 74c74b0 into PaddlePaddle:develop Jun 17, 2022
sneaxiy pushed a commit to sneaxiy/Paddle that referenced this pull request Jun 27, 2022
@yangguohao @sneaxiy
[PaddleHackathon No.14] (PaddlePaddle#41183)
c360750 
* 2022-04-28

* 2022-05-04

* 2022-05-05_V1

* 2022-05-05_V1

* Update loss.py

* Update loss.py

* 2022-06-01_hook

* 2022-06-05

* 2022-06-07

* 2022-06-07_V2

* 2022-06-07_V2

* 2022-06-17_codestyle
@yangguohao yangguohao deleted the multilabelmarginloss branch September 18, 2023 05:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@XiaoguangHu01 XiaoguangHu01 XiaoguangHu01 approved these changes

@TCChenlong TCChenlong TCChenlong approved these changes

@jeff41404 jeff41404 jeff41404 approved these changes

@betterpig betterpig betterpig left review comments

@DDDivano DDDivano DDDivano left review comments

Assignees

@DDDivano DDDivano

@betterpig betterpig

Labels
contributor External developers PaddlePaddle Hackathon
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@yangguohao @CLAassistant @dingjiaweiww @luotao1 @TCChenlong @jeff41404 @DDDivano @XiaoguangHu01 @betterpig