-
Notifications
You must be signed in to change notification settings - Fork 5.8k
[API compatibility] add new API paddle.tensor and save paddle.tensor module #74540
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提交成功,感谢你对开源项目的贡献! |
tools/windows/run_unittests.sh
Outdated
@@ -335,6 +335,7 @@ disable_win_inference_test="^trt_quant_int8_yolov3_r50_test$|\ | |||
^trt_split_converter_test$|\ | |||
^paddle_infer_api_copy_tensor_tester$|\ | |||
^test_eager_tensor$|\ | |||
^test_new_eager_tensor$|\ |
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.
CI这些应该不用改吧
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/paddle/tensor/creation.py
Outdated
_warned_in_to_tensor = True | ||
tensor = _to_tensor_non_static(data, dtype, place, stop_gradient) | ||
if pin_memory: | ||
tensor = tensor.pin_memory() |
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.
最终仅返回一次tensor,将pin_memory这些统一处理
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.
Done
python/paddle/tensor/creation.py
Outdated
@@ -957,6 +1075,9 @@ def to_tensor( | |||
[[(1+1j), (2+0j)], | |||
[(3+2j), (4+0j)]]) | |||
""" | |||
warnings.warn( |
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.
参考下_warned_in_to_tensor,这里仅第一次打印warning,避免刷屏
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.
Done
|
||
|
||
tensor = _TensorMethodOrModule() # noqa: F811 | ||
|
||
# CINN has to set a flag to include a lib |
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.
_TensorMethodOrModule 这个class的其他一些方法也需要实现下,结合python原生方法,使体验上更好。比如type、print、repr应该看起来是一个方法,而dir() 看起来是模块
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.
Done
@@ -2061,6 +2061,20 @@ def test_to_tensor(self): | |||
self.assertEqual(out2.shape, []) | |||
self.assertEqual(out2, 2.5) | |||
|
|||
def test_tensor(self): |
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.
这里测试位置不对,建议这样测试吧,to_tensor直接alias到tensor,仅在前面加一个warning
这样历史所有的 paddle.to_tensor 都切换到 paddle.tensor
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.
是指在to_tensor的方法中删除所有的实现,然后改成warning+调用paddle.tensor(xxx)的方式么?
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.
诶,这样修改的话,是不是就不用新增单测了,直接利用to_tensor的单测测试tensor
|
||
|
||
tensor = _TensorMethodOrModule() # noqa: F811 | ||
|
||
# CINN has to set a flag to include a lib |
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.
new_tensor_api -> tensor_api
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.
Done
aa0b964
to
ec05c8e
Compare
Codecov Report❌ Patch coverage is
❌ Your patch status has failed because the patch coverage (88.00%) 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 #74540 +/- ##
==========================================
Coverage ? 88.00%
==========================================
Files ? 2
Lines ? 50
Branches ? 0
==========================================
Hits ? 44
Misses ? 6
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
python/paddle/tensor/creation.py
Outdated
@@ -65,6 +65,7 @@ | |||
__all__ = [] | |||
|
|||
_warned_in_to_tensor = False | |||
_warned_in_use_to_tensor = False |
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.
warning可以优化下名字,
_warned_in_tensor = False
_warned_in_to_tensor = False
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.
Done
python/paddle/tensor/creation.py
Outdated
tensor = tensor.pin_memory() | ||
return tensor | ||
|
||
|
||
@ParamAliasDecorator({"place": ["device"]}) |
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.
这个装饰器可以去掉了,这个是为了兼容 paddle.to_tensor
调用device的情况,但现在有了严格对齐的 paddle.tensor
,这个装饰器就不需要了,直接用paddle.tensor
就行,单测如果挂了也一并修改过来
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.
Done
python/paddle/tensor/creation.py
Outdated
place_str = re.findall(re_exp, str(place))[0] | ||
with paddle.static.device_guard(place_str): | ||
tensor = _to_tensor_static(data, dtype, stop_gradient) | ||
if pin_memory: |
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.
pin_memory直接的实现是不是能优化下,根据pin_memory,修改place为 CudaPinnedPlace
,避免二次拷贝的成本
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.
除了Cuda能pin_memory外,XPU也可以,所以这里多加了一些判断,另外是有个分支通过tensor_from_cuda_array_interface创建tensor,这个接口没有place参数,所以这里显式调用了一次pin_memory
place = _get_paddle_place(device) | ||
if place is None: | ||
place = _current_expected_place_() | ||
if pin_memory and not isinstance( |
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.
这里是not_instance,下面是isinstance?
paddle.tensor
写单测 测试一下吧,虽然 to_tensor
会调用 tensor
,但这两个API还是有些不同,比如 pin_memory、device这些参数
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.
这里not isinstance是想确定place是不是已经是PinnedPlace了,是的话就没必要继续判断了,下面的isinstance是确定当前的Place是GPU或XPU。
好的我补充一下单测
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.
这个API benchmark挂了,可能是to_tensor加的warning装饰器影响到了其他API,你需要把框架其他API用到to_tensor
的地方改成新的 tensor
试试能不能过
这个改动是不是太大了(搜了一下大概涉及1409个文件,10152个项),替换除了把paddle.to_tensor改成paddle.tensor之外,因为paddle.tensor和paddle.to_tensor的参数名(place->device, stop_gradient->requires_grad)不一样,有的情况还要一并修改。现在paddle.to_tensor是直接访问相对地址到API,然后在调用同级目录下的tensor方法。但改成paddle.tensor之后还需要走那个转发类,我感觉两个性能差距应该差不多。
这几个API也没用到paddle.to_tensor啊,这合理吗? |
我看大多数 PR 的 API benchmark 里 |
python/paddle/tensor/creation.py
Outdated
return tensor | ||
|
||
|
||
@deprecated( |
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.
如果影响到了性能,这个装饰器也可以不加,长时间应该还会是 paddle.tensor
与paddle.to_tensor
并存
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.
这个性能影响应该不大,现在API Benchmark的CI更新后API Benchmark是pass的
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
/re-run all-failed |
…module (PaddlePaddle#74540) * use paddle.tensor to instead paddle.to_tensor * update pin_memory * fix bug * fix the test * add a unit test * update test * add deprecated * update target api * add test * remove the deprecated * update test case * remove deprecated
PR Category
User Experience
PR Types
New features
Description
维持paddle.tensor作为module的原本功能的同时,新增API,paddle.tensor()实现tensor的创建。
在PaConvert下自测,paddle.tensor()与torch.tensor()对齐。
pcard-71500