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

Remove the declaration of using Tensor in framework/tensor.h #46432

Merged
merged 20 commits into from
Sep 28, 2022

Conversation

chenwhql
Copy link
Contributor

@chenwhql chenwhql commented Sep 23, 2022

PR types

Function optimization

PR changes

Others

Describe

Remove the declaration of using Tensor in framework/tensor.h

本RP完成年初基础框架Tensor统一的部分遗留工作,事实上现在框架中的基础Tensor即为phi::DenseTensor,原framework::Tensor和framework::LoDTensor都是采用using的方式作为phi::DenseTensor的别名

using Tensor = phi::DenseTensor;
using LoDTensor = phi::DenseTensor;

当初认为改动成本较高,因此搁置了,但是继续保留这样的状态,会有以下危害:

  1. 基础概念混乱,理解成本高:部分同学目前并不知道上述的别名关系,因此开发时会对框架的Tensor体系感到困惑
  2. 潜在命名空间冲突风险,阻塞后续C++ API experimental 命名空间去除:
    目前框架内 Tensor 主要指对外暴露的 paddle::experimental::Tensor,这是paddle对外C++ API的基础数据结构,内部即为DenseTensor,但experimental后续是需要去掉的,而由于paddle::framework::Tensor 通过using的关系仍然保留着,这样如果去掉experimental的话,paddle::Tensorpaddle::framework::Tensor在编译时就有冲突的风险,影响框架正确性和开发效率

因此,我们还是需要尽早将历史残留的using去掉,以确保后续框架的稳定发展

TODO:

  1. 部分代码中还遗留着 using Tensor = phi::DenseTensor 这样的声明,后续仍然需要继续去除,本PR改动过多,不严格保证均删除这样的声明
  2. 去掉 using LoDTensor = phi::DenseTensor 的声明
  3. 去掉 paddle::experimental::Tensorexperimental

@paddle-bot
Copy link

paddle-bot bot commented Sep 23, 2022

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

YuanRisheng
YuanRisheng previously approved these changes Sep 28, 2022
zyfncg
zyfncg previously approved these changes Sep 28, 2022
paddle/fluid/framework/device_worker.h Show resolved Hide resolved
paddle/fluid/framework/ir/fc_fuse_pass.cc Show resolved Hide resolved
@chenwhql chenwhql dismissed stale reviews from zyfncg and YuanRisheng via b384e8b September 28, 2022 05:10
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

Copy link
Contributor

@ZzSean ZzSean left a comment

Choose a reason for hiding this comment

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

LGTM for CI-OP-Benchmark

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.

6 participants