Skip to content

Conversation

@aquagull
Copy link
Contributor

@aquagull aquagull commented Aug 18, 2025

PR Category

User Experience

PR Types

Others

Description

maximum minimum下沉到C++层,并删除了相关老IR的单测

pcard-71500

@paddle-bot
Copy link

paddle-bot bot commented Aug 18, 2025

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

zhwesky2010
zhwesky2010 previously approved these changes Aug 18, 2025
@zhwesky2010
Copy link
Contributor

@aquagull 冲突需要处理下

@aquagull aquagull closed this Aug 18, 2025
@aquagull aquagull marked this pull request as draft August 19, 2025 02:29
@aquagull aquagull marked this pull request as ready for review August 19, 2025 07:44
zhwesky2010
zhwesky2010 previously approved these changes Aug 19, 2025
Copy link
Contributor

@zhwesky2010 zhwesky2010 left a comment

Choose a reason for hiding this comment

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

LGTM

@zhwesky2010 zhwesky2010 requested a review from SigureMo August 19, 2025 12:53
(integer types are autocasted into float32).
Examples:
.. code-block:: python
>>> import paddle
Copy link
Member

Choose a reason for hiding this comment

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

如果我没记错的话,code-block 下面不加空行英文文档渲染是会出问题的

这段文档不应该直接从原来 API 文档里直接 copy 么?还会进行修改么?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这个迁移的时候从Github Files Changedcopy的,没注意到没有空行。之后会恢复

"""
Compare two tensors and returns a new tensor containing the element-wise maxima. The equation is:
.. math::
Copy link
Member

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.

同上



def maximum(x: Tensor, y: Tensor, name: str | None = None) -> Tensor:
@ParamAliasDecorator({"x": ["input"], "y": ["other"]})
Copy link
Contributor

@zhwesky2010 zhwesky2010 Aug 20, 2025

Choose a reason for hiding this comment

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

API-bechmark没过,试试改成函数式的装饰器 param_two_alias,这个性能好一些

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@codecov-commenter
Copy link

codecov-commenter commented Aug 20, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (develop@348fa91). Learn more about missing BASE report.

Additional details and impacted files
@@             Coverage Diff             @@
##             develop    #74683   +/-   ##
===========================================
  Coverage           ?   100.00%           
===========================================
  Files              ?         2           
  Lines              ?         6           
  Branches           ?         0           
===========================================
  Hits               ?         6           
  Misses             ?         0           
  Partials           ?         0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@aquagull
Copy link
Contributor Author

/re-run all-failed

@aquagull aquagull dismissed stale reviews from SigureMo and zhwesky2010 via a6089ef August 26, 2025 03:43
@aquagull aquagull changed the title [API Compatibility] add out parameter for maximum minimum topk [API Compatibility] add out parameter for maximum minimum Aug 26, 2025
@aquagull aquagull changed the title [API Compatibility] add out parameter for maximum minimum [API Compatibility] add out parameter for maximum minimum topk Aug 27, 2025
@zhwesky2010 zhwesky2010 marked this pull request as ready for review August 27, 2025 03:29
@aquagull aquagull changed the title [API Compatibility] add out parameter for maximum minimum topk [API Compatibility] add out parameter for maximum minimum Aug 27, 2025
zhwesky2010
zhwesky2010 previously approved these changes Aug 27, 2025
@DanielSun11
Copy link
Contributor

test/amp下的单测报什么错啊?删掉的原因是什么?

@SigureMo
Copy link
Member

SigureMo commented Aug 27, 2025

test/amp下的单测报什么错啊?删掉的原因是什么?

@DanielSun11 我当时看到另一个 PR 也简单调研了下,#74795 (comment)

这部分应该是针对老 IR 写的 AMP case,使用了 OldIrGuard,PIR AMP 写法与动态图一致,实现了动静统一,因此应该是单独的单测,这些应该确实可以删掉

这个 @wanghuancoder 可以确认下

Copy link
Contributor

@zhwesky2010 zhwesky2010 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

@XiaoguangHu01 XiaoguangHu01 left a comment

Choose a reason for hiding this comment

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

LGTM

@aquagull
Copy link
Contributor Author

/re-run all-failed

@wanghuancoder wanghuancoder merged commit 69d6153 into PaddlePaddle:develop Aug 28, 2025
72 of 73 checks passed
@aquagull aquagull deleted the api_out branch September 12, 2025 03:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants