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

fix the bug in sigmoid_cross_entropy_with_logits_grad_kernel #64253

Merged
merged 5 commits into from
May 16, 2024

Conversation

zeroRains
Copy link
Contributor

@zeroRains zeroRains commented May 13, 2024

PR Category

Others

PR Types

Bug fixes

Description

sigmoid_cross_entropy_with_logits op的反向kernel计算实现,与自动微分求导计算出的反向结果不一致。现提交PR进行修复,具体分析以及反向kernel计算方式错误仍能通过单测的分析见下面的Issue:

Copy link

paddle-bot bot commented May 13, 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.

T up = -e_x * abs_grad * pos_weight;
T term3 = up / down;

T diff = term1 - label + term3;
Copy link
Contributor

Choose a reason for hiding this comment

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

建议调用其他kernel实现

@cyber-pioneer
Copy link
Contributor

补充相关单侧

@zeroRains
Copy link
Contributor Author

补充相关单侧

Done

Copy link
Contributor

@cyber-pioneer cyber-pioneer left a comment

Choose a reason for hiding this comment

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

LGTM

T diff = simoid_x * pos_weight_idx - label;
T term1 = (x > 0) ? static_cast<T>(1) : static_cast<T>(0);

T e_x = std::exp(-std::abs(x));
Copy link
Contributor

Choose a reason for hiding this comment

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

这里为什么要加 std::abs(x) ? e_x 变量不是指的e^(-x) 么?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这是根据前向计算的求导得到的,前向计算是$e^{-|X|}$,其导数是$-e^{-|x|} * where(x>=0,1,0)$

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

因为前向中的e^x是e^-abs(x),所以反向也需要保持e^-abs(x)

Copy link
Contributor

@DesmonDay DesmonDay left a comment

Choose a reason for hiding this comment

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

LGTM

@cyber-pioneer cyber-pioneer merged commit 8cef847 into PaddlePaddle:develop May 16, 2024
31 checks passed
@zeroRains zeroRains deleted the sigmoid branch May 16, 2024 10:59
co63oc pushed a commit to co63oc/Paddle that referenced this pull request May 18, 2024
…addle#64253)

* fix the bug in sigmoid_cross_entropy_with_logits_grad_kernel

* add the test for sigmoid_cross_entropy_with_logits_grad

* modify the test

* modify the grad  kernel

* modify the kernel
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor External developers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants