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.

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

【Hackathon No.10】AdaptiveLogSoftmaxWithLoss API #41886

Closed
wants to merge 10 commits into from

Conversation

gsq7474741
Copy link
Contributor

@gsq7474741 gsq7474741 commented Apr 17, 2022

PR types

New features

PR changes

APIs

Describe

在 Paddle 框架中,新增 AdaptiveLogSoftmaxWithLoss API
RFC:20200322_api_design_for_AdaptiveLogSoftmaxWithLoss.md
文档PR:PaddlePaddle/docs#4650

@paddle-bot-old paddle-bot-old bot added contributor External developers status: proposed labels Apr 17, 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.

@Ligoml
Copy link
Contributor

Ligoml commented Apr 18, 2022

需要在Describe中添加rfc链接和中文文档pr链接,待CI大部分通过后,开启review

@gsq7474741
Copy link
Contributor Author

需要在Describe中添加rfc链接和中文文档pr链接,待CI大部分通过后,开启review

已添加

@gsq7474741
Copy link
Contributor Author

@yingyibiao 已更新~

@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.

@yingyibiao
Copy link
Contributor

需要新增 paddle.nn.functional.adaptive_log_softmax_with_loss API

@gsq7474741
Copy link
Contributor Author

需要新增 paddle.nn.functional.adaptive_log_softmax_with_loss API

这个api里面需要构造一些对象,且需要保留,所以我觉得做成Layer比较合适,本任务提出的functional api可能不适用,不知您怎么看

@yingyibiao
Copy link
Contributor

需要新增 paddle.nn.functional.adaptive_log_softmax_with_loss API

这个api里面需要构造一些对象,且需要保留,所以我觉得做成Layer比较合适,本任务提出的functional api可能不适用,不知您怎么看

可以把具体的理由详细说明一下吗?adaptive_log_softmax_with_loss 和 paddle.nn.functional 目录下其他 loss 函数的差异点在哪里呢?

@paddle-bot-old
Copy link

paddle-bot-old bot commented May 1, 2022

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

import paddle.fluid as fluid
import paddle
from .. import functional as F
from .. import Layer, Sequential, LayerList
from paddle.nn.layer import Linear
from paddle.fluid.framework import _varbase_creator
from .. import Layer
Copy link
Contributor

Choose a reason for hiding this comment

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

Layer 重复导入

Copy link
Contributor

Choose a reason for hiding this comment

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

import 包的顺序按照 built-in、third-party、项目内部包这三类区分。

head_output[not_in_shortlist])
output[not_in_shortlist] = paddle.argmax(
log_prob, axis=1).cast('float32')
return output
Copy link
Contributor

Choose a reason for hiding this comment

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

AdaptiveLogSoftmaxWithLoss 这个类的代码&注释和 torch 一摸一样,原则上我们是禁止直接搬运 torch 代码的。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

注释里加了pytorch的license,不知是否可以,而且这个代码部分适配paddle做了一点修改

Copy link
Contributor

Choose a reason for hiding this comment

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

可以借鉴 pytorch 代码,但是不能一大段直接搬运。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

好的 我改一下

@Ligoml Ligoml assigned luotao1 and Ligoml and unassigned yingyibiao Jan 5, 2023
@luotao1 luotao1 added the API label May 9, 2023
@luotao1 luotao1 changed the title 【Hackathon No.10】 【Hackathon No.10】AdaptiveLogSoftmaxWithLoss API Sep 18, 2023
@luotao1 luotao1 closed this Oct 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants