-
Notifications
You must be signed in to change notification settings - Fork 5.9k
[API Compatible] Support add signature and default mapping when Python API sink to the C++ layer #74676
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
Conversation
|
你的PR提交成功,感谢你对开源项目的贡献! |
Codecov Report❌ Patch coverage is
❌ Your patch status has failed because the patch coverage (64.22%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## develop #74676 +/- ##
==========================================
Coverage ? 64.22%
==========================================
Files ? 2
Lines ? 109
Branches ? 0
==========================================
Hits ? 70
Misses ? 39
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
wanghuancoder
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
jeff41404
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
暂时 approve,本 PR 只能保证 paddle.Tensor.amax 有类型提示效果,因为它是根据运行时自动生成的 tensor.pyi 生效的,但对 paddle.amax 是无效的,因为这个 API 之前是 Python 原生编写的,type checker 能根据源码分析得到,这些 API 一旦下沉,无非两个方案:
- 提供 python 包装,包装签名为
fn(*args, **kwargs),直接传参给下沉后的 API,包装同时提供@overload用于类型提示(没试过,需要调研) - 继续按照当前方案走,但是为对应的 pybind API 提供
.pyi文件,类似tensor.pyi,这样才能让 type checker 能够自动发现
一些其他问题可以下个 PR 修改,主要是降低实现难度和维护成本
|
|
||
| EAGER_CATCH_AND_THROW_RETURN_NULL | ||
| } | ||
| PyObject* eager__add_doc_str(PyObject* self, PyObject* args) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
只是好奇,这段代码下沉的必要是什么?使用 Python 直接编写清晰得多,而且这里应该不涉及计算逻辑,应该没什么加速效果
另一方面,这段逻辑只在 import paddle 时候执行一次,不会造成运行时开销
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这段代码本来计划是为C++ 编译出的builtin类型的function添加signature和doc的,但是C++ builtin类型的function加signature有问题,暂时保留了C++接口。
| def _ast_unparse(node: ast.AST) -> str: | ||
| if isinstance(node, ast.Name): | ||
| return node.id | ||
| elif isinstance(node, ast.Subscript): | ||
| value = _ast_unparse(node.value) | ||
| slice_str = _ast_unparse(node.slice) | ||
| return f"{value}[{slice_str}]" | ||
| elif isinstance(node, ast.Index): | ||
| return _ast_unparse(node.value) | ||
| elif isinstance(node, ast.Constant): | ||
| # process string | ||
| if isinstance(node.value, str): | ||
| return f"'{node.value}'" | ||
| return str(node.value) | ||
| elif isinstance(node, ast.BinOp) and isinstance(node.op, ast.BitOr): | ||
| left = _ast_unparse(node.left) | ||
| right = _ast_unparse(node.right) | ||
| return f"{left} | {right}" | ||
| elif isinstance(node, ast.Attribute): | ||
| return f"{_ast_unparse(node.value)}.{node.attr}" | ||
| elif isinstance(node, ast.Tuple): | ||
| return ", ".join(_ast_unparse(el) for el in node.elts) | ||
| else: | ||
| return ast.dump(node) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
直接使用 ast.unparse,因为我们现在最低支持是 Python 3.9,没有理由手写,而且 else 用的 ast.dump 效果完全不一样
| from .base.dygraph.generated_tensor_methods_patch import methods_map | ||
|
|
||
|
|
||
| def _parse_function_signature( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这里的实现不够可靠,不如直接:
import inspect
code_template = """
from __future__ import annotations
{}
"""
code = """
def fn(
x: Tensor,
axis: int | Sequence[int] | None = None,
keepdim: bool = False,
name: str | None = None,
) -> Tensor:
...
"""
def extract_fn_signature(code: str) -> inspect.Signature:
code = code_template.format(code)
code_obj = compile(code, "<string>", "exec")
globals = {}
eval(code_obj, globals)
return inspect.signature(globals["fn"])
extract_fn_signature(code)简单可靠,直接利用 python 解释器自身的解析能力
…n API sink to the C++ layer (PaddlePaddle#74676) * support add signature and default mapping * temp disable signature for builtin function * warp the _C_ops api
PR Category
Operator Mechanism
PR Types
New features
Description
Python API C++ 下沉机制支持添加signature以及支持使用默认的args mapping
签名测试:
pcard-67164