Skip to content

Conversation

@winter-wang
Copy link
Contributor

@winter-wang winter-wang commented Nov 26, 2023

PR types

New features

PR changes

Others

Description

  • 支持在Op注册完后以后,给OpInfo添加新的Interface.
  • 在注册Paddle的OpDilacet的时候,给cf.TuplePushOp添加新的Vjp接口。

Other

Pcard-67164

@paddle-bot
Copy link

paddle-bot bot commented Nov 26, 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
Copy link

paddle-bot bot commented Nov 26, 2023

❌ The PR is not created using PR's template. You can refer to this Demo.
Please use PR's template, it helps save our maintainers' time so that more developers get helped.

@winter-wang winter-wang force-pushed the cf_develop branch 8 times, most recently from 5a33c1c to 5ef4bfa Compare November 26, 2023 10:30
Copy link
Contributor

@zhangbo9674 zhangbo9674 left a comment

Choose a reason for hiding this comment

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

LGTM overrall

const std::vector<std::vector<pir::OpResult>> &outputs,
const std::vector<std::vector<pir::Value>> &out_grads,
const std::vector<std::vector<bool>> &stop_gradients) {
PADDLE_ENFORCE_EQ(inputs.size() == 1u && inputs[0].size() >= 1u,
Copy link
Contributor

Choose a reason for hiding this comment

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

这个判断有必要么?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

我觉得判断一下比较好,不然后面inputs[0][0]可能会直接崩溃,不好发现原因

(void)std::initializer_list<int>{
0, (PlacementConstrctInterface<Args>(p_interface), 0)...};
return p_interface;
0, (PlacementConstrctInterface<Args>(interface_set), 0)...};
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
0, (PlacementConstrctInterface<Args>(interface_set), 0)...};
0, (ConstrctInterface<Args>(interface_set), 0)...};

Copy link
Contributor Author

Choose a reason for hiding this comment

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

好的,这个名字我在下个pr中修改。

const char **p_attributes,
VerifyPtr verify_sig,
VerifyPtr verify_region)
: interface_set_(std::move(interface_set)),
Copy link
Contributor

Choose a reason for hiding this comment

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

将interface_set_调整到num_traits后面或前面会更好?

Copy link
Contributor Author

@winter-wang winter-wang Nov 27, 2023

Choose a reason for hiding this comment

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

个人觉得占用空间最大的成员变量放在最前面会比较好。我们可以再讨论一下。

@winter-wang winter-wang merged commit d93061c into PaddlePaddle:develop Nov 27, 2023
SecretXV pushed a commit to SecretXV/Paddle that referenced this pull request Nov 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants