Skip to content

Conversation

@NKNaN
Copy link
Contributor

@NKNaN NKNaN commented Nov 8, 2024

PR Category

User Experience

PR Types

New features

Description

@paddle-bot
Copy link

paddle-bot bot commented Nov 8, 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 Nov 8, 2024
self.cast_dtype = 'complex64'

def set_func(self):
self.func = paddle.jit.to_static(full_graph=True)(test_complex_cast)
Copy link
Member

Choose a reason for hiding this comment

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

这个 case 想跑通的话,应该需要修改 python/paddle/jit/dy2static/transformers/cast_transformer.pypython/paddle/jit/dy2static/convert_operators.pyconvert_var_dtype

现在不修改的情况下是能跑通的么?如果能跑通有点不符合预期

Copy link
Contributor Author

Choose a reason for hiding this comment

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

本地之前没有跑到。这个case需要支持吗?我看报错信息都是在 python/paddle/base/layers/math_op_patch.pypython/paddle/pir/math_op_patch.py 两处新增的地方。

Copy link
Member

Choose a reason for hiding this comment

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

支持一下吧,和 float、int 保持一致

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已修改

complex(Tensor) is not supported in static graph mode. Because it's value is not available during the static mode.
It's usually triggered by the logging implicitly, for example:
>>> logging.info("The value of x is: {complex(x)}")
^ `x` is Tensor, `complex(x)` triggers complex(Tensor)
Copy link
Member

Choose a reason for hiding this comment

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

^ 对齐到上面的 x

Copy link
Contributor Author

Choose a reason for hiding this comment

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

好的

Comment on lines 764 to 765
if dtype == 'complex':
return var.astype('complex64')
Copy link
Member

Choose a reason for hiding this comment

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

这个为啥要单独写而不是一起放到 cast_map?看起来 paddle.castTensor.astype 逻辑是一致的,都是走的 cast OP

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image
哦我看这里没有添加complex的类型,以为不行来着。
已修改

Copy link
Member

Choose a reason for hiding this comment

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

我看 PIR 分支是没有这些限制的,如果老 IR 有问题的话,可以在当前 case 上装饰 test_pir_only 装饰器跳过老 IR,新特性无需考虑老 IR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

好的,明白了

@PaddlePaddle PaddlePaddle locked and limited conversation to collaborators Nov 11, 2024
@PaddlePaddle PaddlePaddle unlocked this conversation Nov 11, 2024
@HydrogenSulfate
Copy link
Contributor

有几个单测挂了,
image
看起来还需要额外适配几处代码才行,有时间的话还麻烦看一下

@SigureMo
Copy link
Member

看起来还需要额外适配几处代码才行,有时间的话还麻烦看一下

按照上面说的,装饰 @test_pir_only 跳过老 IR 即可

@HydrogenSulfate
Copy link
Contributor

看起来还需要额外适配几处代码才行,有时间的话还麻烦看一下

按照上面说的,装饰 @test_pir_only 跳过老 IR 即可

好的

@NKNaN
Copy link
Contributor Author

NKNaN commented Nov 11, 2024

xpu似乎不支持cast到complex类型,需要跳过xpu上的检测吗
image

@SigureMo
Copy link
Member

xpu似乎不支持cast到complex类型,需要跳过xpu上的检测吗

嗯,先跳过下吧

SigureMo
SigureMo previously approved these changes Nov 11, 2024
Copy link
Member

@SigureMo SigureMo left a comment

Choose a reason for hiding this comment

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

LGTMeow 🐾

Coverage 中老 IR 代码和 SOT 没测到可以豁免

@HydrogenSulfate HydrogenSulfate changed the title 【Paddle Tensor No.10】新增 Tensor.__complex__ 【Paddle Tensor No.10】新增 Tensor.__complex__ Nov 12, 2024
@HydrogenSulfate
Copy link
Contributor

@NKNaN 覆盖率好像没过,还麻烦看下是否能补充几个单测

@luotao1 luotao1 added the HappyOpenSource 快乐开源活动issue与PR label Nov 13, 2024
@HydrogenSulfate HydrogenSulfate merged commit 05170c6 into PaddlePaddle:develop Nov 13, 2024
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor External developers HappyOpenSource 快乐开源活动issue与PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants