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

【PIR API adaptor No.45-47】Migrate some ops into pir #58682

Merged
merged 13 commits into from
Nov 20, 2023

Conversation

longranger2
Copy link
Contributor

@longranger2 longranger2 commented Nov 4, 2023

PR types

Others

PR changes

APIs

Description

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

  • crop(7/10):总计 10 个单测,打开7个单测,3个单测用来测试边界和error的

  • cross(5/5)

  • softmax_with_cross_entropy(43/44):总计 44 个单测,打开43个单测,1个单测用来测试error的,暂时关闭test_softmax_with_cross_entropy_op 的 check_grad 的相关单测因为windows ci会报错

  • 新IR Python API适配升级 #58067

Copy link

paddle-bot bot commented Nov 4, 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 Nov 4, 2023
@luotao1 luotao1 added the HappyOpenSource 快乐开源活动issue与PR label Nov 6, 2023
@MarioLulab
Copy link
Contributor

看 PR-CI-Py3 报错在了 test_crop_op.py 和 test_cross_op.py 里。

  1. test_crop_op.py 报错的原因是 TestCropOp 没有 python_api 属性。可以在 TestCropOp.setUp 函数里加上:
self.python_api = paddle.crop
  1. test_cross_op.py 报错的原因是当前场景下 OpResult 不支持在 python 端直接拿到 name 属性,故报错:
    image
    可以将 fetch_list=[z.name] 改为 fetch_list=[z]

@MarioLulab
Copy link
Contributor

PR-CI-Py3 挂掉的地方在:

  1. test_cross_api:解决方法是:把 Program 改成 paddle.static.Program 的形式吧
  2. test_crop_op: 错误和 kernel_signature 有关。我正在尝试本地复现

@YuanRisheng
Copy link
Contributor

YuanRisheng commented Nov 8, 2023

crop的问题是由于这里适配的单测是废弃算子的单测,这里由于算子的历史原因名字有误导性,真正需要适配的单测是test_crop_tensor_op.py

Copy link
Contributor

@MarioLulab MarioLulab left a comment

Choose a reason for hiding this comment

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

LGTM

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.

test_softmax_with_cross_entropy_op单测中的check_grad会在windows上失败。这里的check_grad先设置成check_pir=False,我们内部看下反向的问题

@MarioLulab
Copy link
Contributor

麻烦 pre-commit 一下

@longranger2
Copy link
Contributor Author

麻烦 pre-commit 一下

done

Copy link
Contributor

@MarioLulab MarioLulab left a comment

Choose a reason for hiding this comment

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

LGTM
请在 pr 描述里记录目前跳过 test_softmax_with_cross_entropy_op 的 check_grad 的相关单测,并更新单测覆盖率

@0x45f 0x45f merged commit d963050 into PaddlePaddle:develop Nov 20, 2023
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor External developers HappyOpenSource 快乐开源活动issue与PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants