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

为什么前向和后向 kernel 必须写在不同的文件里 #43701

Closed
tiancaishaonvjituizi opened this issue Jun 21, 2022 · 14 comments
Closed
Assignees
Labels

Comments

@tiancaishaonvjituizi
Copy link
Contributor

tiancaishaonvjituizi commented Jun 21, 2022

请提出你的问题 Please ask your question

原始链接:#42469 (comment)

把前向和后向 kernel 写在不同的文件里有什么好处呢,在我看来这只是盲目地遵守了一套八股文规则,对代码复用是有损害的,前后向 kernel 之间经常存在一些可以复用的逻辑,且这些逻辑是 specific to 当前的 kernel、没有普适性的。如果拆成两个文件,这些逻辑

  1. 要么不复用,在两个文件里各写一份。这增加了维护成本
  2. 要么 public 在头文件里。这增加了开发者要写的 boilerplate code,也带来了命名冲突/抽象泄漏的问题。

而且新增一个 op 需要在 8 个文件里跳转(以我正在实现的 nan_to_num 为例,是 nan_to_num_kernel.cc/nan_to_num_kernel.cu/nan_to_num_kernel.h/nan_to_num_kernel_impl.h 和 nan_to_num_grad_kernel.cc/nan_to_num_grad_kernel.cu/nan_to_num_grad_kernel.h/nan_to_num_grad_kernel_impl.h 八个),这让人哭笑不得,把这八个文件的文件名一个一个列出来的时候,心里也会觉得很心累吧。

我在 这里 提到,paddle 的很多 “反软件工程” 的做法反映了 “鼓励流水线式批量产出代码,不在意可维护性” 的潜意识,我觉得本 issue 提出的问题是又一个例子。

@paddle-bot-old
Copy link

您好,我们已经收到了您的问题,会安排技术人员尽快解答您的问题,请耐心等待。请您再次检查是否提供了清晰的问题描述、复现代码、环境&版本、报错信息等。同时,您也可以通过查看官网API文档常见问题历史IssueAI社区来寻求解答。祝您生活愉快~

Hi! We've received your issue and please be patient to get responded. We will arrange technicians to answer your questions as soon as possible. Please make sure that you have posted enough message to demo your request. You may also check out the APIFAQGithub Issue and AI community to get the answer.Have a nice day!

@chenwhql
Copy link
Contributor

  1. 首先解释下为什么前反向kernel会分开放置?
  • 如您所述,前反向拆开一方面文件数增多了,一些原本能组织到一起的代码也需要分开放置,这确实不好,但这个当时是因为要解决推理编包裁剪的问题。在之前paddle fluid中的算子,前反向实现是写到一起的,但是推理场景仅需要使用前向的kernel,为了减小体积,推理在编译时,采用了你最看不过去的正则匹配,将反向的代码匹配删掉生成一个新的文件,再去编译。因此这次在重新实现时,推理前置强调这个需求的重要性,要求能够用相对自然的,非ugly的方式选择不编译所有反向kernel,所以就拆分成两个文件了,推理编译的时候就不编译反向文件了
  • 关于这一点,我理解关键点其实在于,反向的kernel更多地应该复用前向的kernel去实现,而不是每个反向都单独编写,这样的话,会减少很多不必要的实现,这个我们正在做了 ,但历史代码很多,重建成本也比较高,周期会比较长
  1. 新增op需要8个文件?
  • 这个其实并不是我们期望的算子实现方式,impl.h这个东西的存在是因为老的算子库有很多历史算子是cpu和gpu实现公用的,但又不是完全设备无关的,比如有其他设备的时候,就又创建了op_xpu.cc, op_npu.cc等文件。我们迁移的时候没有足够的时间去重写所有的逻辑,所以临时创建了impl这个目录,用于放置这种cpu和gpu共用代码,但又不能给其他设备使用的情况,这个目录及里面的代码后续会删除的,可以的话,也请您不要在新增此类case了
  • 那nan_to_num这个case来说,其实我们推荐在kernels根目录写一份设备无关的实现,目前来说的话,在kernels目录是创建以下几个文件
./paddle/phi/kernels/nan_to_num_kernel.h  // 前向kernel声明
./paddle/phi/kernels/nan_to_num_kernel.cc // 前向kernel设备无关实现
./paddle/phi/kernels/nan_to_num_grad_kernel.h // 反向kernel声明
./paddle/phi/kernels/nan_to_num_grad_kernel.cc // 反向kernel设备无关实现

然后在kernels/funcs目录下的某个已有.h文件,或者新建的.h文件中,假设就叫nan_to_num.h吧,声明您前反向计算的核心函数,比如是NumToNanFunctor,然后在对应的.cc或者cu中去实现这个函数,这样nan_to_num_kernel.cc需要include fucns下面的nan_to_num.h,并调用NumToNanFunctor,这里的NumToNanFunctor只是声明和具体设备无关

kernels根目录下的kernel实现需要是设备无关的,这个是考虑到后续新增硬件的成本,假如现在您只写了cpu和gpu,但我们其实还有xpu,npu,mlu等硬件,如果按照我刚刚说的方式,我们新增这些硬件的时候,仅需要去实现核心的nan_to_num.h中的那个函数或者functor在xpu,npu,mlu的实现,就能够使之前kernels/nan_to_num_kernel.cc支持这几种硬件

其实本质上可能并没有减少文件数,但是后续硬件扩展的时候可以少写一些代码,而如果是现在这样在impl/xxx_impl.h下写的话,后续还需要新增kernels/xpu/nan_to_num_kernel.cc, kernels/npu/nan_to_num_kernel.cc, kernels/mlu/nan_to_num_kernel.cc这些文件,kernel声明也都是重复的

目前这里确实还是在建设中的状态,上下层的复用关系其实不是很明确,总之后续impl这个定位说不清楚的目录是要移除的

@tiancaishaonvjituizi
Copy link
Contributor Author

tiancaishaonvjituizi commented Jun 21, 2022

新增op需要8个文件?

这个其实并不是我们期望的算子实现方式

是不是这样理解:这个不是期望的算子实现方式,但期望的算子实现方式也是一样需要创建 8 个文件

比如是NumToNanFunctor,然后在对应的.cc或者cu中去实现这个函数

按照这种实现,原来 .cc 和 .cu 之间可以通过 impl 复用的代码现在也不能复用了吗

@chenwhql
Copy link
Contributor

然后目前的kernel实现主要是从原来的fluid/operators下面迁移过来的,迁移的时候为了不引入不确定性,逻辑基本都是尽可能保持不变的,所以现在kernel太散了,其实能复用的也没怎么复用起来。

我们正在看怎么调整体系,后续应该是少量基础算子保持kernels/xxx_kernel.h, kernels/cpu/xxx_kernel.cc, kernels/gpu/xxx_kernel.cu, kernels/xpu/xxx_kernel.cc, ...这样,大部分算子在上层只有kernels/xxx_kernel.h, kernels/xxx_kernel.cc这样的形式,这个需要一定周期

@chenwhql
Copy link
Contributor

这个目前确实是的,但是funcs下面的文件不需要严格一一对应,同性质的工具函数可以尽可能放到同一个文件中,当然改动的文件数是一致的。impl这个目录的定位是混乱的,只能放cpu和gpu的,下沉是必要的,否则可能有很多组合,cpu和xpu,xpu和gpu等等,硬件数是比较多的

cc和cu本就是两种实现,通过头文件中统一的函数声明去复用即可,大量带模板参数的函数体都放到头文件中,编译时效率也是比较低的

@chenwhql
Copy link
Contributor

比如像kernels/funcs下面的math_function.h,这种是比较合理的分层实现。

@tiancaishaonvjituizi
Copy link
Contributor Author

tiancaishaonvjituizi commented Jun 21, 2022

比如像kernels/funcs下面的math_function.h,这种是比较合理的分层实现。

这样是合理的

然而这样还是要改 8 个文件,这个 8 是这样组成的:(1 + 1 + 2) * (1 + 1)
也就是 (可选的 func 文件 + kernel 头文件 + 每种设备各一个实现文件) * (前向 + 后向)

所以能看出如果前向后向能写在一个文件里,8 就会变成 4,如果必须要写在两个文件,那后续也会有 xpu1/some_op_kernel.cc、xpu1/some_op_grad_kernel.cc、xpu2/some_op_kernel.cc、xpu2/some_op_grad_kernel.cc 等等,那个时候新增一个 op 就总共需要新增 12 个文件(更加哭笑不得了)而不是通常期望的 6 个文件,这意味着后续的删除、修改可能都需要一个一个操作这 12 个文件,如您所说 “硬件数是比较多的”,“考虑到后续新增硬件的成本”,这种组合爆炸我感觉并不是合理的,您觉得呢

@tiancaishaonvjituizi
Copy link
Contributor Author

我有些不知道该怎么说,这些奇怪的情况(每个 kernel 写八个文件、每个 python op api 里有三个 if 分支)好像说起来都有不得不这样的原因,但又没听说过其它框架有这些情况。关于每个 python op api 里有三个 if 分支的事情,我在这里写了看法,后来没有得到回复,我现在仍然持有这个看法 —— 从一开始的某个不起眼的决策开始就埋下了错误的种子 😂

@chenwhql
Copy link
Contributor

是的,肯定是不好的,所以现在我们的建设方向是这样:

  1. 提升复用程度,避免每个反向kernel都需要手写实现,而是通过声明配置的方式去复现前向实现
  2. 建设元算子体系,上层算子更多地复用实现,减少从0开始实现kernel的绝对数量
  3. 编译器建设,前反向整图元算子拆解,将复杂的前反向都拆分正小的基础运算,再通过fuse策略去加速

所以后续我们会将kernel开发的文件变成 1 + 1,即 头文件 + 前向kernel

@chenwhql
Copy link
Contributor

chenwhql commented Jun 21, 2022

我有些不知道该怎么说,这些奇怪的情况(每个 kernel 写八个文件、每个 python op api 里有三个 if 分支)好像说起来都有不得不这样的原因,但又没听说过其它框架有这些情况。关于每个 python op api 里有三个 if 分支的事情,我在这里写了看法,后来没有得到回复,我现在仍然持有这个看法 —— 从一开始的某个不起眼的决策开始就埋下了错误的种子 😂

说实话,我非常认同你的看法,特别是:

通俗的讲就是只想着赶紧做业务,忽视了对底层代码的维护和迭代,所以屎山代码越堆越大,直到某一天爆雷了,才意识到要花十倍的时间去弥补

所以现在我收拾这个烂摊子也极其痛苦且无奈😂,但我对于你说的这个整体的决策趋势无力改变,软件是有生命周期的,规范性问题积累多了不去处理,软件就会死去,paddle毕竟是开源代码,至少目前还是能推动去投入弥补之前的错误,不至于完全绝望

@chenwhql
Copy link
Contributor

麻烦再多给一些耐心吧,你前前后后讨论的多个问题,作为局外人我和你感同身受,但目前我是局内人,我们还需要去一步步解决这些无论是历史的还是新增的问题

@tiancaishaonvjituizi
Copy link
Contributor Author

🤝🤝 理解,祝 paddle 越来越成功

@chenwhql
Copy link
Contributor

🤝🤝 理解,祝 paddle 越来越成功

感谢理解

@paddle-bot paddle-bot bot closed this as completed Jun 27, 2023
@paddle-bot
Copy link

paddle-bot bot commented Jun 27, 2023

Since you haven't replied for more than a year, we have closed this issue/pr.
If the problem is not solved or there is a follow-up one, please reopen it at any time and we will continue to follow up.
由于您超过一年未回复,我们将关闭这个issue/pr。
若问题未解决或有后续问题,请随时重新打开,我们会继续跟进。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants