Skip to content

Conversation

@NKNaN
Copy link
Contributor

@NKNaN NKNaN commented Jul 10, 2024

PR Category

User Experience

PR Types

Improvements

Description

解决的问题:

  1. F.pad 在输入 len(pad) == 2 * x.ndim 时,axis 左对齐,需改为 axis 右对齐
  2. F.pad 不支持除了 len(pad) == 2 * x.ndimlen(pad) == 2 * (x.ndim - 2) 以外的情况

修改方法:
添加 pad_from_first_axis 参数,默认为 True。只在 len(pad) == 2 * x.ndim 时起作用。

修改后 F.pad 的行为是:

  1. len(pad) == 2 * x.ndim 时,默认 axis 左对齐,可以通过设置 pad_from_first_axis 参数为 False 改为右对齐;
  2. len(pad) != 2 * (x.ndim - 2) 时,axis 右对齐;
  3. len(pad) == 2 * (x.ndim - 2) 时,x.ndim 只能是 3、4、5,从 [D, H, W]的最后一个维度开始填充,也是axis右对齐。

@paddle-bot
Copy link

paddle-bot bot commented Jul 10, 2024

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

@paddle-bot paddle-bot bot added the contributor External developers label Jul 10, 2024
@zhwesky2010
Copy link
Contributor

zhwesky2010 commented Jul 19, 2024

PR Category

User Experience

PR Types

Improvements

Description

解决的问题:

  1. F.pad 在输入 len(pad) == 2 * x.ndim 时,axis 左对齐,需改为 axis 右对齐
  2. F.pad 不支持除了 len(pad) == 2 * x.ndimlen(pad) == 2 * (x.ndim - 2) 以外的情况

修改方法: 添加 pad_from_first_axis 参数,默认为 True。修改后 F.pad 的行为是:当 len(pad) != 2 * (x.ndim - 2) 时,x.ndim 可以是 1-6,且可以通过 pad_from_first_axis 控制 axis 左对齐或右对齐;当 len(pad) == 2 * (x.ndim - 2) 时,x.ndim 只能是 3、4、5,填充的顺序由 data_format 来决定。

修改后可以认为 F.pad 能够支持 axis 右对齐:当 len(pad) != 2 * (x.ndim - 2) 时,取 pad_from_first_axis = False;或当 len(pad) == 2 * (x.ndim - 2) 时,取 data_format = 'NC...'

2是兼容性升级的,直接升级就行吧,无需捆绑pad_from_first_axis。兼容性升级你对齐torch。

只有不兼容升级,才用pad_from_first_axis这个参数降低影响,因此别弄那么复杂,这个参数不要捆绑那么多情况,只针对情况1时的不兼容升级,其他情况直接忽略该参数。

@zhwesky2010
Copy link
Contributor

@NKNaN 关注上述评论

@paddle-ci-bot
Copy link

paddle-ci-bot bot commented Jul 20, 2024

Sorry to inform you that 2feee53's CIs have passed for more than 7 days. To prevent PR conflicts, you need to re-run all CIs manually.

@NKNaN
Copy link
Contributor Author

NKNaN commented Jul 22, 2024

好的,我再改一下

@zhwesky2010
Copy link
Contributor

好的,我再改一下

更新一下PR描述

@NKNaN
Copy link
Contributor Author

NKNaN commented Jul 22, 2024

好的,我再改一下

更新一下PR描述

已更新

name (str, optional): For details, please refer to :ref:`api_guide_Name`. Generally, no setting is required. Default: ``'None'``.
the input data when: 1. mode is any of ``'reflect'``, ``'replicate'`` or ``'circular'``; or 2. the input ``'pad'`` is a tensor;
or 3. the length of ``'pad'`` is ``2*(x.ndim - 2)``. Default: ``'NCHW'``.
pad_from_first_axis (bool, optional): When mode is ``'constant'`` and the input ``'pad'`` is a list or tuple and the
Copy link
Contributor

Choose a reason for hiding this comment

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

先写一下,仅当什么情况下生效。

非constant其他mode下为何不生效?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已修改,仅当 mode 是 constant,且 pad 是 list/tuple,且 pad 长度是 2*x.ndim 时生效。

其他 mode 支持的 pad 长度是 2(N-2)。

paddings = pad
pad_value = value

padding_len = len(paddings)
Copy link
Contributor

Choose a reason for hiding this comment

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

kernel内部好修改吗

)

# since the kernel pad from first axis, if we want to pad from last axis, we need to reverse the paddings
if not (len(pad) == x_dim * 2 and pad_from_first_axis):
Copy link
Contributor

@zhwesky2010 zhwesky2010 Jul 22, 2024

Choose a reason for hiding this comment

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

在kernel内部好修改吗,目前主要是通过在Python层面组合适配的,导致kernel内部计算结果差异较大,可能导致推理等场景出现问题

Copy link
Contributor Author

Choose a reason for hiding this comment

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

我试一下吧

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.

这个改动看起来挺大的,能否先抽出一部分逻辑,将除 len(pad) == 2 * x.ndim 之外的情况修改先合入。

即:

F.pad 不支持除了 len(pad) == 2 * x.ndim 或 len(pad) == 2 * (x.ndim - 2) 以外的情况

然后pad_from_first_axis命名为pad_from_left_axis吧,之前的另外一种轻量实现你还在吗,也提一个PR吧,两个综合对比下看哪个更好,因为下放到c++ kernel来实现的影响确实比较大,之前疏忽了,没考虑到这一点抱歉。

@paddle-ci-bot
Copy link

paddle-ci-bot bot commented Aug 7, 2024

Sorry to inform you that d7457e5's CIs have passed for more than 7 days. To prevent PR conflicts, you need to re-run all CIs manually.

@NKNaN
Copy link
Contributor Author

NKNaN commented Aug 7, 2024

这个改动看起来挺大的,能否先抽出一部分逻辑,将除 len(pad) == 2 * x.ndim 之外的情况修改先合入。

即:

F.pad 不支持除了 len(pad) == 2 * x.ndim 或 len(pad) == 2 * (x.ndim - 2) 以外的情况

然后pad_from_first_axis命名为pad_from_left_axis吧,之前的另外一种轻量实现你还在吗,也提一个PR吧,两个综合对比下看哪个更好,因为下放到c++ kernel来实现的影响确实比较大,之前疏忽了,没考虑到这一点抱歉。

好的,没关系,我重新提PR

F.pad 不支持除了 len(pad) == 2 * x.ndim 或 len(pad) == 2 * (x.ndim - 2) 以外的情况
这个单独的改动如果放在 kernel 里面的话其实和这个PR要修改的地方是一样的
image

还是要先把 paddings 的长度补充成 2 * x.ndim,然后再反转 paddings。因为这个 kernel 底层用的是 eigen 库里 TensorMap 的 pad,又因为 tensor 是 rowmajor 的,所以 phi/kernel/func/eigen/pad.cc 这里调用的 pad 应该是左对齐的。所以要满足这里左对齐,但是输入希望是右对齐的话,还是需要把 paddings 补充成 2 * x.ndim,然后再反转 paddings。
image

所以这一点要改的话最好也是放在python端

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor External developers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants