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

[Dy2St] Make bool(Value) always throw error #60902

Merged

Conversation

SigureMo
Copy link
Member

@SigureMo SigureMo commented Jan 17, 2024

PR types

Bug fixes

PR changes

Others

Description

#60836 的拆分

bool(Value) 是一种错误的用法,对于一个自定义 Object 来说,如果没有重写 __bool__,那么 bool(obj) 永远为 True,相关判断逻辑是错误的

而对于编译期的 Value 来说是不可能知道值的,__bool__ 只能报错,因此本 PR 加了本限制,用户代码永远不应该触发 Value.__bool__

但现存很多代码的 API 里触发了 Value.__bool__,需要修复

注意,现有代码大多并不是显式调用 bool(Value),大多是通过 if 隐式调用的,比如

if x:
    ...

if x > 0: # 这里甚至错误地触发了组网,虽然后面没被用到不会导致结果错误
    ...

这些 case 大多是只考虑 x 是 int 的情况,没有考虑是 Value 的情况,因此传入 Value 其实走了错误的逻辑

对于这些 case,可以考虑加逻辑分支或调整写法来避免这种错误用法,如:

 x: Value | None
-if x and x.get_defining_op(): ...
+if x is not None and x.get_defining_op(): ...

 x: Value | float
-if x > 0: raise ValueError(...)
+if isinstance(x, float) and x > 0: raise ValueError(...) # 对于 Value,这个检查在组网阶段做不了,跳过即可

 x: Value | Tensor
-if x > 0: # 比如这里真的就应该根据条件走不同分支
+if in_dynamic_mode(): # 或者判断 isinstance(x, Value)
+    if x > 0: ...
+else:
+    paddle.static.nn.cond(x > 0, ...) # 分离动静态图逻辑,使用 cond OP

这里只是一些示例,但具体情况仍应具体分析

PCard-66972

问题推进 tracker

Copy link

paddle-bot bot commented Jan 17, 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.

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

@2742195759 2742195759 left a comment

Choose a reason for hiding this comment

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

LGTM

@SigureMo SigureMo merged commit 269de63 into PaddlePaddle:develop Jan 24, 2024
29 checks passed
@SigureMo SigureMo deleted the dy2st/make-value-bool-always-throw-error branch January 24, 2024 03:43
eee4017 pushed a commit to eee4017/Paddle that referenced this pull request Jan 30, 2024
---------

Co-authored-by: zoooo0820 <zoooo0820@qq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants