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

为 paddle.linalg.norm 进行功能升级与对齐 - 增加matrix_norm #60070

Merged
merged 41 commits into from
Feb 2, 2024

Conversation

zbt78
Copy link
Contributor

@zbt78 zbt78 commented Dec 15, 2023

PR types

Function optimization

PR changes

OPs

Description

为 paddle.linalg.norm 进行功能对齐与功能增强

def norm(x, p=None, axis=None, keepdim=False, name=None):

paddle缺少 矩阵范数 功能,本次进行了功能升级。其中不兼容升级的分支为:

  • len(axis) == 2 && p == ±inf 时,原先求的是向量范数(由axis构成的矩阵中的最大(小)值),改成矩阵范数(由axis构成的矩阵的行和范数,即矩阵沿行方向取绝对值求和,再取最大(小)值)
  • len(axis) == 2 && p == ±1,±2 时,原先求的是向量范数(由axis构成的矩阵每个元素先取绝对值,在pow(p)后求和,最后再pow(1/p)),改成矩阵范数

被移出的 向量范数 功能,新增了paddle.linalg.vector_norm 来支持:

def vector_norm(x, p=2.0, axis=None, keepdim=False, name=None):

其他前期修改:

Copy link

paddle-bot bot commented Dec 15, 2023

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

@PaddlePaddle PaddlePaddle locked and limited conversation to collaborators Jan 2, 2024
@PaddlePaddle PaddlePaddle unlocked this conversation Jan 2, 2024
@zbt78
Copy link
Contributor Author

zbt78 commented Jan 4, 2024

涛姐,请您帮忙review一下吗~

@luotao1
Copy link
Contributor

luotao1 commented Jan 4, 2024

先把CI给过了吧~

@zbt78
Copy link
Contributor Author

zbt78 commented Jan 5, 2024

涛姐,PR-CE-Framework出错是因为里面使用的是PaddleTest仓库里面的test求的还是向量范数,而这个PR修改成了矩阵范数的逻辑,这里要去修改PaddleTest里的test吗?
还有,这里第二条中的负实数指的是-1,-2,-3这种负数吗?

@zhwesky2010
Copy link
Contributor

为 paddle.linalg.norm 进行功能对齐与功能增强

@zbt78 这个是因为有不兼容升级,可以看下这个矩阵范数功能,是不是通过设置axis也能实现与torch一致?那这样话可能就不要修改算子实现了。
负实数就是指p为负数,可以看下torch是可以这样计算的

@zbt78
Copy link
Contributor Author

zbt78 commented Jan 6, 2024

为 paddle.linalg.norm 进行功能对齐与功能增强

@zbt78 这个是因为有不兼容升级,可以看下这个矩阵范数功能,是不是通过设置axis也能实现与torch一致?那这样话可能就不要修改算子实现了。 负实数就是指p为负数,可以看下torch是可以这样计算的

晚上好~,pytorch matrix_norm也是通过组合算子来实现的,当p=[1,-1,inf,-inf]时先对axis操作然后再调用的vector_norm。如果不修改算子实现直接在python端进行组合好像也行。

p为负数时paddle好像也支持,我还是没太明白什么意思,您可以再详细说下吗

@zhwesky2010
Copy link
Contributor

这个是之前的调研结论:

1)求解p范数时,torch是对矩阵求p范数,paddle是将矩阵直接展平为向量求p范数,两者计算结果不一致
2)torch 支持负实数的范数,paddle 不支持
3)torch支持p='nuc',paddle不支持

如果结论如之不同,或者有简易的方法可以绕过,那么按照绕过方法来实现就可以了,不用修改API

@zbt78
Copy link
Contributor Author

zbt78 commented Jan 9, 2024

把用c++实现的api删除了,目前是用已有的api组装实现的。
另外paddle应该是支持负实数的范数的。

@ZHUI ZHUI self-requested a review January 9, 2024 08:34
@zhwesky2010
Copy link
Contributor

@ZHUI 辛苦看一下目前的修改是否对齐了Pytorch的这几点差异

@zbt78
Copy link
Contributor Author

zbt78 commented Jan 25, 2024

@zbt78 paddle.linalg.vector_norm 抽出来了吗

刚刚提了pr:
【Hackathon 5th No.115】为 paddle.linalg.norm 进行功能升级与对齐- 添加 vector_norm #61155

@luotao1 luotao1 changed the title 【Hackathon 5th No.115】为 paddle.linalg.norm 进行功能升级与对齐 为 paddle.linalg.norm 进行功能升级与对齐 Jan 29, 2024
@zhwesky2010
Copy link
Contributor

zhwesky2010 commented Jan 29, 2024

@zbt78 合入下最新develop,然后提交修改最后一部分吧,主要是 matrix_normnorm的不兼容升级

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.

一些文档小问题

than the :attr:`input` unless :attr:`keepdim` is true, default
value is False.
x (Tensor): Tensor, data type float32, float64.
p (int|float|string, optional): None for porder='fro'. Default None.
Copy link
Contributor

Choose a reason for hiding this comment

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

这个默认是 fro,不是None吧

def norm(x, p='fro', axis=None, keepdim=False, name=None):
"""

Returns the matrix norm (Frobenius) or vector norm (the 1-norm, the Euclidean
Copy link
Contributor

Choose a reason for hiding this comment

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

请问英文文档里可以画表格吗,我没有找到相应的参考资料,仅仅在中文文档里画了个表格。

@sunzhongkai588 这个可以吗

def norm(x, p=None, axis=None, keepdim=False, name=None):
"""

Returns the matrix norm (Frobenius) or vector norm (the 1-norm, the Euclidean
Copy link
Contributor

Choose a reason for hiding this comment

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

这个不止一种吧,有fro、nuc、p-norm

def norm(x, p='fro', axis=None, keepdim=False, name=None):
"""

Returns the matrix norm (Frobenius) or vector norm (the 1-norm, the Euclidean
Copy link
Contributor

Choose a reason for hiding this comment

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

请问英文文档里可以画表格吗,我没有找到相应的参考资料,仅仅在中文文档里画了个表格。

@sunzhongkai588 这个可以吗

@zbt78 是可以的,中文和英文都遵循 sphinx 语法,两者写法是一样的。英文文档是自动抽取源码里的 docstring 并转化成 .rst 文档,而中文文档只是少了抽取这一步。

.rst 的表格写法可以参考下述文档中的表格

需要注意,.rst 文档写表格时,表格内的 | 是需要对齐的(中文文档看起来没对齐是因为表格的文字包含中文和英文,而中文的占位符比英文占位符多导致的)


>>> # compute 2-order norm along [0,1] dimension.
>>> out_pnorm = paddle.linalg.norm(x, p=2, axis=[0,1])
>>> print(out_pnorm)
Copy link
Contributor

Choose a reason for hiding this comment

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

检查下示例代码的输出,CI-Static-Check 里面显示和CI里的输出不一致
image

>>> out_fro = paddle.linalg.norm(x, p='fro', axis=[0,1])
>>> print(out_fro)
>>> out_matrix_norm = paddle.linalg.matrix_norm(x=x,p=2,axis=[0,1],keepdim=False)
>>> print(out_matrix_norm)
Copy link
Contributor

Choose a reason for hiding this comment

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

检查下示例代码的输出,CI-Static-Check 里面显示和CI里的输出不一致
image

Copy link
Contributor

Choose a reason for hiding this comment

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

建议检查一下代码示例是否符合规范,参考Python 文档示例代码书写规范,比如检查是否 norm 带有随机性等,如有随机性得设置seed。


Paddle supports the following norms:
+----------------+--------------------------------+--------------------------------+
| porder | norm for matrices | norm for vectors |
Copy link
Contributor

@zhwesky2010 zhwesky2010 Feb 1, 2024

Choose a reason for hiding this comment

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

porder还需要写一个None的情况,把p在矩阵和向量两种情况下的默认值写上去

p (int|float|string, optional): Order of the norm. Supported values are `fro`, `nuc`, `0`, `±1`, `±2`,
`±inf` and any real number yielding the corresponding p-norm.
Default value is None.
axis (int|list|tuple, optional): The axis on which to apply norm operation. If axis is int
Copy link
Contributor

Choose a reason for hiding this comment

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

需要说明一下axis is None时,是展平后计算vector norm

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.

LGTM,后面注意更新中文文档

@zbt78
Copy link
Contributor Author

zbt78 commented Feb 1, 2024

LGTM,后面注意更新中文文档

我还有个问题哈,就是前两天答辩的时候有个研发大哥提了个建议,就是这里为什么调用_C_ops.x下的api而不是直接调用的paddle.下面的。这个地方要如何处置呀。如果全都用paddle.这种方式的话我可能需要再改改。
image

@zhwesky2010
Copy link
Contributor

zhwesky2010 commented Feb 1, 2024

LGTM,后面注意更新中文文档

我还有个问题哈,就是前两天答辩的时候有个研发大哥提了个建议,就是这里为什么调用_C_ops.x下的api而不是直接调用的paddle.下面的。这个地方要如何处置呀。如果全都用paddle.这种方式的话我可能需要再改改。 image

不用改了就这么写吧

@PaddlePaddle PaddlePaddle deleted a comment from zbt78 Feb 2, 2024
Copy link
Contributor

@sunzhongkai588 sunzhongkai588 left a comment

Choose a reason for hiding this comment

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

LGTM

@luotao1 luotao1 merged commit 4589cdb into PaddlePaddle:develop Feb 2, 2024
32 checks passed
@zhwesky2010
Copy link
Contributor

zhwesky2010 commented Feb 4, 2024

@zbt78 你好,由于此次涉及到不兼容升级,扫描了paddle github组织下的所有repo,本次不兼容升级分支【len(axis)=2且p!=None的情况】使用3次,位于https://github.com/PaddlePaddle Github组织下的repo中3个文件里:

  • PaddleDetection/ppdet/modeling/assigners/uniform_assigner.py
  • PaddleRS/paddlers/models/ppdet/modeling/assigners/uniform_assigner.py
  • PaddleYOLO/ppdet/modeling/assigners/uniform_assigner.py

麻烦你这边统一修改一下

@zhwesky2010 zhwesky2010 changed the title 为 paddle.linalg.norm 进行功能升级与对齐 为 paddle.linalg.norm 进行功能升级与对齐 - 增加matrix_norm Feb 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants