Fix set value grad #59034
Merged
Fix set value grad #59034
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
zoooo0820
force-pushed
the
fix_set_value_grad
branch
from
0bc76a7
to
8a5f39f
Compare
Comment on lines
356
to
358
| switch (rank) { | ||
| case 1: | ||
| SetValueGradImpl<T, Context, 1>(dev_ctx, |
There was a problem hiding this comment.
这里是不是直接调SetValueGradKernel就可以了，value_grad传入nullptr。
Comment on lines
154
to
155
| op->SetType("set_value_grad"); | ||
| op->SetInput(framework::GradVarName("Out"), this->OutputGrad("Out")); |
There was a problem hiding this comment.
这里应该讨论，如果有ValueTensor调用set_value_grad，没有ValueTensor调用set_value_with_scalar_grad
kangguangli
reviewed
Nov 20, 2023
| op->SetType("assign"); | ||
| op->SetInput("X", this->OutputGrad("Out")); | ||
| op->SetOutput("Out", this->InputGrad("Input")); | ||
| op->SetType("set_value_with_scalar_grad"); |
There was a problem hiding this comment.
这行看起来依然没有被覆盖到，是不是需要注册一个新Op set_value_with_scalar_grad？
|
Sorry to inform you that 6cc1f71's CIs have passed for more than 7 days. To prevent PR conflicts, you need to re-run all CIs manually. |
Wanglongzhi2001
pushed a commit
to Wanglongzhi2001/Paddle
that referenced
this pull request
Jan 7, 2024
* first fix the UT * fix set value grad * polish code * add static mode backward test * always has input valuetensor * add dygraph test
zoooo0820
added a commit
to zoooo0820/Paddle
that referenced
this pull request
Jan 18, 2024
* first fix the UT * fix set value grad * polish code * add static mode backward test * always has input valuetensor * add dygraph test
XiaoguangHu01
pushed a commit
that referenced
this pull request
Jan 19, 2024
* Fix set value grad (#59034) * first fix the UT * fix set value grad * polish code * add static mode backward test * always has input valuetensor * add dygraph test * Fix shape error in combined-indexing setitem (#60447) * add ut * fix shape error in combine-indexing * fix ut * Set value with scalar (#60452) * set_value with scalar * fix ut * remove test_pir * remove one test since 2.6 not support uint8-add
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR types
Bug fixes
PR changes
OPs
Description
Pcard-66985
在此前，为了适配分布式动半模式，
set_value算子及其反向算子迁移到phi下。在此之前，set_value算子可同时处理value 为scalar或tensor两种场景，由inputValueTensor是否存在来决定。相应的，其反向set_value_grad也是如此。由于phi的要求，需要显式区分
set_value（对应value 为scalar) 和set_value_with_tensor（对应value 为tensor)两个算子。因此，反向也需要对应区分。在phi之前的算子历史定义fluid/operator中，将前者的反向行为错误地设置为assign，这使得在迁移phi时 #58893 的行为参考有误。导致目前value 为scalar时，赋值的反向结果与预期不符，需要修复。本PR 中新增kernel
set_value_with_scalar_grad，用于该场景的计算，替代此前错误的assign行为，其底层仍然复用SetValueGradImpl。