Skip to content

Conversation

@zoooo0820
Copy link
Contributor

@zoooo0820 zoooo0820 commented Jan 19, 2024

PR types

Performance optimization

PR changes

Others

Description

Pcard-66985

For combined indexing setting case of dynamic mode, e.g. x[1, [0,1]] = value, we will deal it as following step:

  1. Getting a view of x, x'=x[1]
  2. change the data of x', since it is a view of x, the data of x will also be changed.
  3. overwrite the x' to x by set_value.

In our plan , if x has changed by value, the backward should contain it too.

But for now., in step 2, the x cannot know whether it has been changed by other operators. It will only backward to the OP which generated it instead of which modified it.

In past , the step 3 is aiming to build a backward edge between value and x. But the performance suffers because of unnecessary forward overwrite.

This PR is to optimize this case by remove the final set_value_with_tensor, only keeping the backward part , to build a edge between value and x.

@paddle-bot
Copy link

paddle-bot bot commented Jan 19, 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.

Copy link
Contributor

@jeff41404 jeff41404 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 jeff41404 merged commit 9bd5bef into PaddlePaddle:develop Jan 24, 2024
@zoooo0820 zoooo0820 deleted the no_setvalue_in_combined_indexing_set branch January 24, 2024 03:46
eee4017 pushed a commit to eee4017/Paddle that referenced this pull request Jan 30, 2024
…60983) * remove setvalue in combined indexing set * using combined-setting case to test
zoooo0820 added a commit to zoooo0820/Paddle that referenced this pull request Feb 27, 2024
…60983) * remove setvalue in combined indexing set * using combined-setting case to test
zoooo0820 added a commit to zoooo0820/Paddle that referenced this pull request Feb 27, 2024
…60983) * remove setvalue in combined indexing set * using combined-setting case to test
XiaoguangHu01 pushed a commit that referenced this pull request Feb 28, 2024
* tensor_array slice in PIR (#60503) * use slice_array, now will meet error of destory opresult still in use * disable the pir test until the bug fixed * Optimize advanced setting by remove the last set_value (#60771) * pure-advanced setitem will not set_value back * fix multi output in tensor_array_pir * only in dynamic mode * add only advanced-setting case to fix coverage * fast pass for bool setitem (#61021) * fast pass for bool setitem * fix 0-size value case * remove final set_value OP in combined-indexing setting (#60983) * remove setvalue in combined indexing set * using combined-setting case to test * Optimize index put preprocess (#61060) * reduce vector operations when no bool index * reduce vector in index_put * reduce vector operations * no change for value * fix shape error in combine-getitem (#61922)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants