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

[Dy2Stat]Allow ifelse return buildin type in paddle cond #37888

Merged
merged 5 commits into from
Dec 13, 2021

Conversation

0x45f
Copy link
Contributor

@0x45f 0x45f commented Dec 6, 2021

PR types

Others

PR changes

Others

Describe

PR的修改如下:

  1. 此前在paddle cond动转静转写生成的true_fn和false_fn中,如果true_fn和false_fn的某个参数x是bool、int、float类型且x在return语句中,那么在该参数会被转为Variable。本PR之后允许传入的参数是int、float等基本类型,不再将参数自动转换为Variable。
  2. 允许true_fn和false_fn返回bool、int、float基本类型。
    • 只有true_fn和false_fn分支返回的结果类型和值都相等时才可以返回。比如true_fn返回了true_var,false_fn对应的返回结果为false_var,只有true_var和false_var均是基本类型,且type(true_var) == type(false_fn) ,且true_var == false_fn才会将结果作为基本类型返回
    • 当true_var和false_fn值不想等;或者一个为基本类型、另一个为Variable,则都会转换为Variable进行返回。
    • 其余情况,比如返回string、list等类型则会报错。

@paddle-bot-old
Copy link

paddle-bot-old bot commented Dec 6, 2021

Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

@0x45f 0x45f changed the title [Dy2Stat]Allow ifelse return int in paddle cond [Dy2Stat]Allow ifelse return buildin type in paddle cond Dec 8, 2021
# false_args = [
# to_static_variable(var) if id(var) in return_var_ids else var
# for var in false_args
# ]
Copy link
Contributor

Choose a reason for hiding this comment

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

delete unused codes.

@@ -102,6 +102,28 @@ def select_input(inputs, mask):
return out


def select_input_with_buildin_type(inputs, mask):
from paddle.fluid.dygraph.dygraph_to_static.variable_trans_func import to_static_variable
Copy link
Contributor

Choose a reason for hiding this comment

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

import 放在这里是因为有循环引用么?

@@ -2284,6 +2306,8 @@ def append_conditional_block_grad(self, parent_block, inside_block,
def copy_var_to_parent_block(var, layer_helper):
if var is None:
return None
if isinstance(var, (bool, float, six.integer_types)):
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if isinstance(var, (bool, float, six.integer_types)):
if not isinstance(var, Variable):

只要不是variable类型都直接返回。

isinstance(true_var, Variable)) or (isinstance(true_var, (
bool, float, six.integer_types)) and isinstance(false_var,
Variable)):
inputs = [to_static_variable(false_var), to_static_variable(true_var)]
Copy link
Contributor

Choose a reason for hiding this comment

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

这里的处理逻辑是:

  1. true_var、false_var 都不是variable,则直接判断是否相等(包括list、dict等其他类型)
  2. true_var、false_var 其中有一个是Variable,PR的逻辑是会将其中一个转为Variable返回,这个合理么?

Copy link
Contributor

@Aurelius84 Aurelius84 left a comment

Choose a reason for hiding this comment

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

LGTM

@Aurelius84 Aurelius84 merged commit 9598b19 into PaddlePaddle:develop Dec 13, 2021
@0x45f 0x45f deleted the dy2stat_ifelse_ret_int branch December 24, 2021 04:50
0x45f added a commit to 0x45f/Paddle that referenced this pull request Jan 4, 2022
…e#37888)

* allow ifelse return `int` in paddle cond

* add test and refine code

* polish code, add test

* code format
lanxianghit pushed a commit that referenced this pull request Jan 5, 2022
… in dy2stat (#38418)

Fix error when calling sublayer's non-forward func in dy2stat
cherrypick: #37713#37759#37296#38540#37888
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.

2 participants