Closed zbt78 closed 3 months ago
你的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.
涛姐,请您帮忙review一下吗~
先把CI给过了吧~
涛姐,PR-CE-Framework出错是因为里面使用的是PaddleTest
仓库里面的test求的还是向量范数
,而这个PR修改成了矩阵范数
的逻辑,这里要去修改PaddleTest
里的test吗?
还有,这里第二条中的负实数指的是-1,-2,-3这种负数吗?
为 paddle.linalg.norm 进行功能对齐与功能增强
@zbt78 这个是因为有不兼容升级,可以看下这个矩阵范数功能,是不是通过设置axis也能实现与torch一致?那这样话可能就不要修改算子实现了。 负实数就是指p为负数,可以看下torch是可以这样计算的
为 paddle.linalg.norm 进行功能对齐与功能增强
@zbt78 这个是因为有不兼容升级,可以看下这个矩阵范数功能,是不是通过设置axis也能实现与torch一致?那这样话可能就不要修改算子实现了。 负实数就是指p为负数,可以看下torch是可以这样计算的
晚上好~,pytorch matrix_norm也是通过组合算子来实现的,当p=[1,-1,inf,-inf]时先对axis操作然后再调用的vector_norm
。如果不修改算子实现直接在python端进行组合好像也行。
p为负数时paddle好像也支持,我还是没太明白什么意思,您可以再详细说下吗
这个是之前的调研结论:
1)求解p范数时,torch是对矩阵求p范数,paddle是将矩阵直接展平为向量求p范数,两者计算结果不一致
2)torch 支持负实数的范数,paddle 不支持
3)torch支持p='nuc',paddle不支持
如果结论如之不同,或者有简易的方法可以绕过,那么按照绕过方法来实现就可以了,不用修改API
把用c++实现的api删除了,目前是用已有的api组装实现的。 另外paddle应该是支持负实数的范数的。
@ZHUI 辛苦看一下目前的修改是否对齐了Pytorch的这几点差异
PR-CE-Framework出错是因为里面使用的是PaddleTest仓库里面的test求的还是向量范数,而这个PR修改成了矩阵范数的逻辑,这里要去修改PaddleTest里的test吗?
对的
https://pytorch.org/docs/stable/generated/torch.norm.html
有个疑问就是,我们的功能对齐了torch.linalg.norm()
. 那之前norm
的功能,我们有 torch.linalg.vector_norm()
when computing vector norms and torch.linalg.matrix_norm()
类似的api吗?
之前的 向量范数功能,用户需要如何兼容使用?
是否也需要像torch 一样 保留 paddle.norm()
作为兼容?
PR-CE-Framework出错是因为里面使用的是PaddleTest仓库里面的test求的还是向量范数,而这个PR修改成了矩阵范数的逻辑,这里要去修改PaddleTest里的test吗?
对的
这个需要现在修改吗,还是等这个pr能够merge后呀
这个估计得同步改,不然两边CI都过不去
@zbt78 麻烦在PR改动里写清楚目前的改动点,列成每一条的功能改动点,以及是否兼容
@zbt78 麻烦在PR改动里写清楚目前的改动点,列成每一条的功能改动点,以及是否兼容
写在PR的Description里了。
https://pytorch.org/docs/stable/generated/torch.norm.html
有个疑问就是,我们的功能对齐了
torch.linalg.norm()
. 那之前norm
的功能,我们有torch.linalg.vector_norm()
when computing vector norms andtorch.linalg.matrix_norm()
类似的api吗?之前的 向量范数功能,用户需要如何兼容使用?
是否也需要像torch 一样 保留
paddle.norm()
作为兼容?
这里的将矩阵范数当作向量范数来计算我认为是完全错误的,与numpy和torch都不同,也许是之前实现时赶时间或者没有认真推导公式。
这里的不兼容主要发生在len(axis)=2且p为[-1, -2, -inf, 1, 2, inf]时,将原来的向量范数方式切换为正确的矩阵范数公式。原来的这种强行作为向量范数的计算方式,在这个函数里https://github.com/PaddlePaddle/Paddle/blob/develop/python/paddle/tensor/linalg.py#L417C1-L465 有vector_norm的实现,目前这个API是放到内部使用,可以考虑将其公开出来,承接向量范数的功能。而paddle.linalg.norm
我们按照正确的方式来升级即可。
注:可以看出这里的函数命名起初是想实现p阶矩阵范数的,但不知道为何将p_matrix_norm
实现成了向量范数的公式。。。
@zbt78 还想确认两个地方: 1)axis=None时,paddle与torch的实现方式是否一致? 2)p为1与-1、2与-2、inf与-inf时,计算结果应该不同吧,这里按abs(p)实现的结果是一致的?
@zbt78 还想确认两个地方: 1)axis=None时,paddle与torch的实现方式是否一致? 2)p为1与-1、2与-2、inf与-inf时,计算结果应该不同吧,这里按abs(p)实现的结果是一致的?
p is not None and ((axis is not None and len(axis) == 2) or (axis is None and len(input.shape) == 2))
,其余情况下算的都是向量范数)max_min = _C_ops.max if porder > 0.0 else _C_ops.min
这种方式区分开了,所以实现的结果是正确的。https://pytorch.org/docs/stable/generated/torch.norm.html 有个疑问就是,我们的功能对齐了
torch.linalg.norm()
. 那之前norm
的功能,我们有torch.linalg.vector_norm()
when computing vector norms andtorch.linalg.matrix_norm()
类似的api吗? 之前的 向量范数功能,用户需要如何兼容使用? 是否也需要像torch 一样 保留paddle.norm()
作为兼容?这里的将矩阵范数当作向量范数来计算我认为是完全错误的,与numpy和torch都不同,也许是之前实现时赶时间或者没有认真推导公式。 这里的不兼容主要发生在len(axis)=2且p为[-1, -2, -inf, 1, 2, inf]时,将原来的向量范数方式切换为正确的矩阵范数公式。原来的这种强行作为向量范数的计算方式,在这个函数里https://github.com/PaddlePaddle/Paddle/blob/develop/python/paddle/tensor/linalg.py#L417C1-L465 有vector_norm的实现,目前这个API是放到内部使用,可以考虑将其公开出来,承接向量范数的功能。而
paddle.linalg.norm
我们按照正确的方式来升级即可。
这里的历史原因是paddle.norm
当时对齐的是 torch.norm
, 而不是 torch.linalg.norm
。我们是不是也要将 paddle.norm
和 paddle.linalg.norm
行为做区分
和torch一致是没问题的。兼容性问题解决好就没问题。
https://pytorch.org/docs/stable/generated/torch.norm.html 有个疑问就是,我们的功能对齐了
torch.linalg.norm()
. 那之前norm
的功能,我们有torch.linalg.vector_norm()
when computing vector norms andtorch.linalg.matrix_norm()
类似的api吗? 之前的 向量范数功能,用户需要如何兼容使用? 是否也需要像torch 一样 保留paddle.norm()
作为兼容?这里的将矩阵范数当作向量范数来计算我认为是完全错误的,与numpy和torch都不同,也许是之前实现时赶时间或者没有认真推导公式。 这里的不兼容主要发生在len(axis)=2且p为[-1, -2, -inf, 1, 2, inf]时,将原来的向量范数方式切换为正确的矩阵范数公式。原来的这种强行作为向量范数的计算方式,在这个函数里https://github.com/PaddlePaddle/Paddle/blob/develop/python/paddle/tensor/linalg.py#L417C1-L465 有vector_norm的实现,目前这个API是放到内部使用,可以考虑将其公开出来,承接向量范数的功能。而
paddle.linalg.norm
我们按照正确的方式来升级即可。这里的历史原因是
paddle.norm
当时对齐的是torch.norm
, 而不是torch.linalg.norm
。我们是不是也要将paddle.norm
和paddle.linalg.norm
行为做区分
了解了,torch的线性代数API最初很多在torch.*
,但后来都在torch.linalg.*
重新规范实现了,并计划废弃torch.*
下很多不规范线性代数API,所以最初设计paddle线性代数时,规范是统一在paddle.linalg.*
下,因此建议不增加paddle.norm
,而新增paddle.linalg.vector_norm
来补充这个将废弃的功能。
总结一下,这个PR的主要改动点在于:
np.linalg.norm
、torch.linalg.norm
均不对齐且不符合常理,这里更新为矩阵范数实现,同时新增一个 paddle.linalg.vector_norm
来承接原本的 向量范数 功能;在更正后,paddle.linalg.norm
与numpy、torch均对齐。@zbt78 请确认以上的修改点补充完全,务必补充完整相应的单测case。
@zbt78 将第一个改动点拆开吧,我先合入了;第二个改动点我们会内部讨论下,晚点告知结果。
@zbt78 将第一个改动点拆开吧,我先合入了;第二个改动点我们会内部讨论下,晚点告知结果。
好的。
@zbt78 将第一个改动点拆开吧,我先合入了;第二个改动点我们会内部讨论下,晚点告知结果。
好的。
paddle.linalg.vector_norm
这个API也暴露出来,承接被不兼容删掉的功能,在这里 https://github.com/PaddlePaddle/Paddle/blob/develop/python/paddle/tensor/linalg.py#L417
paddle.linalg.vector_norm
这个API也暴露出来,承接被不兼容删掉的功能,在这里 https://github.com/PaddlePaddle/Paddle/blob/develop/python/paddle/tensor/linalg.py#L417
我有个疑问哈~ 这里被删除的功能是len(axis)==2时畸形的p_matrix_norm中的逻辑。把paddle.linalg.vector_norm
暴露出来后好像也无法兼容这部分功能。因为paddle.linalg.vector_norm
只支持len(axis)==1的情况,并不支持被删掉的len(axis)==2那部分,这样做好像还是没办法做到兼容。
paddle.linalg.vector_norm
这个API也暴露出来,承接被不兼容删掉的功能,在这里 https://github.com/PaddlePaddle/Paddle/blob/develop/python/paddle/tensor/linalg.py#L417我有个疑问哈~ 这里被删除的功能是len(axis)==2时畸形的p_matrix_norm中的逻辑。把
paddle.linalg.vector_norm
暴露出来后好像也无法兼容这部分功能。因为paddle.linalg.vector_norm
只支持len(axis)==1的情况,并不支持被删掉的len(axis)==2那部分,这样做好像还是没办法做到兼容。
那这个已有的vector_norm相当于残缺的vector_norm吧,只是不支持axis为tuple的情况,是https://pytorch.org/docs/stable/generated/torch.linalg.vector_norm.html#torch.linalg.vector_norm 的子集,因此需要把paddle的vector_norm增强一下,对齐torch.linalg.vecotr_norm
然后暴露出来。
缺这个功能:
@zbt78 另外需要处理一下冲突
还有一个问题就是:当axis == None && len(input.shape) == 2
时,numpy和torch都是把input看成个矩阵,然后计算matrix_norm。之前的情况是把input拉成向量,然后计算vector_norm。这里需要对齐吗?
还有一个问题就是:当
axis == None && len(input.shape) == 2
时,numpy和torch都是把input看成个矩阵,然后计算matrix_norm。之前的情况是把input拉成向量,然后计算vector_norm。这里需要对齐吗?
因为我们已通过len(axis)==2支持了矩阵范数功能,这里就先继续维持现状吧
def norm(x, p='fro', axis=None, keepdim=False, name=None):
我想了想感觉这里把p的默认值设置为2.0比较好。因为p='fro'时要计算矩阵范数,而这与axis的默认值None是相冲突的,axis为None时要计算向量范数。
def norm(x, p='fro', axis=None, keepdim=False, name=None):
我想了想感觉这里把p的默认值设置为2.0比较好。因为p='fro'时要计算矩阵范数,而这与axis的默认值None是相冲突的,axis为None时要计算向量范数。
@zbt78 把p改成None比较好,这样可以自适应,适应规则可以这样:
pytorch:
numpy:
@zbt78 建议注意下代码风格
@zbt78 建议注意下代码风格
好的好的。(๑•̀ㅂ•́)و✧
@zbt78 本地需要装一下pre-commit,现在codestyle没有过
@zbt78 本地需要装一下pre-commit,现在codestyle没有过
这里是因为把matrix_norm
,vector_norm
接入paddle.linalg
下的时候少些了一部分,已添加。
@zbt78 这个PR目前太大了触发了CI拦截,把新增 paddle.linalg.vector_norm
这个功能先拆出来合入吧
@zbt78
https://github.com/PaddlePaddle/PaddleTest/pull/2626 这个PR修改了不兼容升级的测试test_norm.py,以使得PR-CE-Framework 能通过,你需要本地验证下修改后的test_norm.py能不能跑,运行方式 pytest test_norm.py
@zbt78 paddle.linalg.vector_norm
抽出来了吗
@zbt78
paddle.linalg.vector_norm
抽出来了吗
刚刚提了pr: 【Hackathon 5th No.115】为 paddle.linalg.norm 进行功能升级与对齐- 添加 vector_norm #61155
@zbt78 合入下最新develop,然后提交修改最后一部分吧,主要是 matrix_norm
、norm的不兼容升级
LGTM,后面注意更新中文文档
我还有个问题哈,就是前两天答辩的时候有个研发大哥提了个建议,就是这里为什么调用_C_ops.
x下的api而不是直接调用的paddle.
下面的。这个地方要如何处置呀。如果全都用paddle.
这种方式的话我可能需要再改改。
LGTM,后面注意更新中文文档
我还有个问题哈,就是前两天答辩的时候有个研发大哥提了个建议,就是这里为什么调用
_C_ops.
x下的api而不是直接调用的paddle.
下面的。这个地方要如何处置呀。如果全都用paddle.
这种方式的话我可能需要再改改。
不用改了就这么写吧
@zbt78 你好,由于此次涉及到不兼容升级,扫描了paddle github组织下的所有repo,本次不兼容升级分支【len(axis)=2且p!=None的情况】使用3次,位于https://github.com/PaddlePaddle Github组织下的repo中3个文件里:
麻烦你这边统一修改一下
PR types
Function optimization
PR changes
OPs
Description
为 paddle.linalg.norm 进行功能对齐与功能增强
paddle缺少 矩阵范数 功能,本次进行了功能升级。其中不兼容升级的分支为:
len(axis) == 2 && p == ±inf
时,原先求的是向量范数(由axis构成的矩阵中的最大(小)值),改成矩阵范数(由axis构成的矩阵的行和范数
,即矩阵沿行方向取绝对值求和,再取最大(小)值)len(axis) == 2 && p == ±1,±2
时,原先求的是向量范数(由axis构成的矩阵每个元素先取绝对值,在pow(p)
后求和,最后再pow(1/p)
),改成矩阵范数被移出的 向量范数 功能,新增了
paddle.linalg.vector_norm
来支持:其他前期修改: