Skip to content

【PIR API adaptor No.53、111、112、116、208、229、230】Migrate some ops into pir #58384

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
Merged
merged 20 commits into from
Nov 6, 2023
Merged

【PIR API adaptor No.53、111、112、116、208、229、230】Migrate some ops into pir #58384

merged 20 commits into from
Nov 6, 2023

Conversation

zrr1999
Copy link
Member

@zrr1999 zrr1999 commented Oct 25, 2023 •
edited
Loading

PR types

Others

PR changes

APIs

Description

PIR API 推全升级
将如下算子迁移升级至 pir，并更新单测

  • expand/broadcast_to(1/2): base.backward.calc_gradient好像不支持 pir报错 AttributeError: 'paddle.base.libpaddle.pir.Program' object has no attribute '_appending_grad_times'
  • solve(20/20)
  • diag(4/4)
  • linspace(4/4)
  • std(2/2)
  • cond：依赖于 【PIR API adaptor No.219 、220】 Migrate pinv/svd into pir #58446
  • slogdet/det(6/6)
  • index_add(5/5)
  • index_put(0/34)：添加 PIR 测试遇到问题
  • inverse(7/7)：

遇到问题

std 测试 File "/home/linuxbrew/.linuxbrew/Cellar/python@3.10/3.10.13/lib/python3.10/site-packages/paddle/tensor/manipulation.py", line 187, in cast return _C_ops.cast(x, dtype) ValueError: (InvalidArgument) cast: argument (position 2) must be one of paddle::DataType, but got paddle.base.libpaddle.VarType (at /workspace/Paddle/paddle/fluid/pybind/eager_utils.cc:697)

index_put 测试

按照 https://github.com/PaddlePaddle/Paddle/pull/58496/files#diff-1ccaa1eaa0f1a4a582a62b7813e9d6d9c561d0579aea019074a39eba769be687R649
等参考内容修改后，依然会有如下报错
ValueError: (InvalidArgument) The Variable type must be N3phi11DenseTensorE, but the type it holds is N6paddle9framework9PhiVectorIPKNS0_8VariableEEE.
[Hint: Expected holder_->Type() == VarTypeTrait::kId, but received holder_->Type():64 != VarTypeTrait::kId:7.] (at /workspace/Paddle/paddle/fluid/framework/variable.h:52)

@paddle-bot
Copy link

paddle-bot bot commented Oct 25, 2023

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

@paddle-bot paddle-bot bot added the contributor External developers label Oct 25, 2023
@paddle-bot
Copy link

paddle-bot bot commented Oct 25, 2023 •
edited
Loading

✅ This PR's description meets the template requirements!
Please wait for other CI results.

@zrr1999 zrr1999 marked this pull request as draft October 25, 2023 10:57
@zrr1999 zrr1999 marked this pull request as ready for review October 26, 2023 07:37
@zrr1999 zrr1999 changed the title Migrate broadcast_to ops into pir Migrate some ops into pir Oct 26, 2023
@zrr1999 zrr1999 changed the title Migrate some ops into pir 【PIR API adaptor No.230】Migrate some ops into pir Oct 26, 2023
@Ligoml Ligoml mentioned this pull request Oct 27, 2023
Closed
@zrr1999 zrr1999 changed the title 【PIR API adaptor No.230】Migrate some ops into pir 【PIR API adaptor No.111、112、208、229、230】Migrate some ops into pir Oct 29, 2023
@0x45f
Copy link
Contributor

0x45f commented Oct 30, 2023

  • std添加单测遇到问题，尝试修改下stat.py文件中162、163两行中paddle.int64修改为'int64'
  • slogdet的对应单测在test_determinant_op.py文件中
  • t这个函数我看在develop上已经适配过pir了
  • index_put测试将dtype改为str会有什么报错呢？

@zrr1999
Copy link
Member Author

zrr1999 commented Oct 30, 2023

  • std添加单测遇到问题，尝试修改下stat.py文件中162、163两行中paddle.int64修改为'int64'
  • slogdet的对应单测在test_determinant_op.py文件中
  • t这个函数我看在develop上已经适配过pir了
  • index_put测试将dtype改为str会有什么报错呢？

std的问题修改后出现了报错
InvalidArgumentError: The type of data we are trying to retrieve (float32) does not match the type of data (int64) currently contained in the container.

image 看起来这里存在一些问题。

如果有继承 TestStdAPI 的类，
class TestStdAPI_dtype(TestStdAPI):
def set_attrs(self):
self.dtype = 'float32'

这一步的 x 类型可能就会出错
self.x = np.random.uniform(-1, 1, self.shape).astype(self.dtype)

我在 def static里修改成了 self.x.astype(self.dtype)，但是似乎还是同样的报错

@0x45f
Copy link
Contributor

0x45f commented Oct 30, 2023

  • std添加单测遇到问题，尝试修改下stat.py文件中162、163两行中paddle.int64修改为'int64'
  • slogdet的对应单测在test_determinant_op.py文件中
  • t这个函数我看在develop上已经适配过pir了
  • index_put测试将dtype改为str会有什么报错呢？

std的问题修改后出现了报错 InvalidArgumentError: The type of data we are trying to retrieve (float32) does not match the type of data (int64) currently contained in the container.

image 看起来这里存在一些问题。
如果有继承 TestStdAPI 的类， class TestStdAPI_dtype(TestStdAPI): def set_attrs(self): self.dtype = 'float32'

这一步的 x 类型可能就会出错 self.x = np.random.uniform(-1, 1, self.shape).astype(self.dtype)

我在 def static里修改成了 self.x.astype(self.dtype)，但是似乎还是同样的报错

下午看了一下std单测的问题，原因是OpResult的__div__方法的代码有bug，可以按如下进行修改：

  1. 按前面的描述将dtype修改为str
  2. 修改下python/paddle/pir/math_op_patch.py文件的292行，修改为other_var_opresult = paddle.cast(other_var_opresult, DataType.FLOAT32)
  3. 然后打开std对应的单测~

@0x45f
Copy link
Contributor

0x45f commented Oct 30, 2023

对于index_put的单测，将dtype修改为str之后，因为Python端Program错乱的缘故，需要将test_static_forward函数中的with paddle.static.program_guard(Program()):代码行修改为：with paddle.static.program_guard(paddle.static.Program()):这样index_put单测应该就没有问题了

@zrr1999 zrr1999 changed the title 【PIR API adaptor No.111、112、208、229、230】Migrate some ops into pir 【PIR API adaptor No.53、111、112、208、229、230】Migrate some ops into pir Oct 31, 2023
@zrr1999 zrr1999 changed the title 【PIR API adaptor No.53、111、112、208、229、230】Migrate some ops into pir 【PIR API adaptor No.53、111、112、116、208、229、230】Migrate some ops into pir Oct 31, 2023
@zrr1999 zrr1999 requested a review from 0x45f October 31, 2023 08:34
@0x45f
Copy link
Contributor

0x45f commented Nov 2, 2023

辛苦在PR描述中添加一下：

  • expand/broadcast_to另一个单测没有打开的原因
  • index_put打开单测的个数

另外cond在本pr中还没有适配对吗？

@zrr1999
Copy link
Member Author

zrr1999 commented Nov 2, 2023

辛苦在PR描述中添加一下：

  • expand/broadcast_to另一个单测没有打开的原因
  • index_put打开单测的个数

另外cond在本pr中还没有适配对吗？

  • expand 的原因已添加
  • index_put 的单测因为都是依赖的父类的test_static_forward，因此目前全部都没有开启。
  • cond 测试本 PR 都还没有开启，但是有部分修改

0x45f
0x45f reviewed Nov 2, 2023
View reviewed changes
@0x45f
Copy link
Contributor

0x45f commented Nov 2, 2023

辛苦在PR描述中添加一下：

  • expand/broadcast_to另一个单测没有打开的原因
  • index_put打开单测的个数

另外cond在本pr中还没有适配对吗？

  • expand 的原因已添加
  • index_put 的单测因为都是依赖的父类的test_static_forward，因此目前全部都没有开启。
  • cond 测试本 PR 都还没有开启，但是有部分修改

辛苦在pr描述中标明一下index_put打开了多少单测~

0x45f
0x45f reviewed Nov 3, 2023
View reviewed changes
zrr1999 and others added 2 commits November 3, 2023 12:44
@zrr1999 @0x45f
Update python/paddle/tensor/manipulation.py
6507f48 
Co-authored-by: WangZhen <23097963+0x45f@users.noreply.github.com>
@zrr1999
fix
f61f97c
@zrr1999 zrr1999 requested a review from 0x45f November 4, 2023 12:26
0x45f
0x45f approved these changes Nov 6, 2023
View reviewed changes
Copy link
Contributor

@0x45f 0x45f left a comment

Choose a reason for hiding this comment

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

LGTM

@0x45f
Copy link
Contributor

0x45f commented Nov 6, 2023

index_put的单测应该不是所有单测都会报错吧，之前我跑了一下有的单测是能够跑过的，辛苦下个pr尝试将index_put能跑过的单测打开~

Aurelius84
Aurelius84 approved these changes Nov 6, 2023
View reviewed changes
Copy link
Contributor

@Aurelius84 Aurelius84 left a comment

Choose a reason for hiding this comment

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

LGTM

@0x45f 0x45f merged commit dc5ae3b into PaddlePaddle:develop Nov 6, 2023
zeroRains pushed a commit to zeroRains/Paddle that referenced this pull request Nov 8, 2023
@zrr1999 @zeroRains
【PIR API adaptor No.53、111、112、116、208、229、230】Migrate some ops into …
fee07e1 
…pir (PaddlePaddle#58384)
@zrr1999 zrr1999 mentioned this pull request Nov 8, 2023
Closed
22 tasks
danleifeng pushed a commit to danleifeng/Paddle that referenced this pull request Nov 14, 2023
@zrr1999
【PIR API adaptor No.53、111、112、116、208、229、230】Migrate some ops into …
39fbd7e 
…pir (PaddlePaddle#58384)
SecretXV pushed a commit to SecretXV/Paddle that referenced this pull request Nov 28, 2023
@zrr1999 @SecretXV
【PIR API adaptor No.53、111、112、116、208、229、230】Migrate some ops into …
2fcdbd1 
…pir (PaddlePaddle#58384)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Aurelius84 Aurelius84 Aurelius84 approved these changes

@0x45f 0x45f 0x45f approved these changes

Assignees

@0x45f 0x45f

@zrr1999 zrr1999

Labels
contributor External developers
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@zrr1999 @0x45f @Aurelius84