[Dy2static] FunctionScopeVisitor Enhance and substitute the original NameVisitor in If #43967

2742195759 · 2022-06-30T07:12:27Z

PR types

Others

PR changes

Others

Describe

FunctionScopeVisitor Enhance and substitute the original NameVisitor in If.
这个PR主要目标是增强了FunctionScopeVisitor的功能，支持控制流的 name 解析。并且替换了IF的NameVisitor和名字解析逻辑。其余的修改是针对上述修改的单测修复。
新的FunctionScopeVisitor主要有如下几个优点：

支持闭包分析：目前的FunctionScopeVIsitor支持了闭包的模拟机制。因此不会出现闭包中的变量被误解析的问题。之前的if就有这种问题。
统一了名字解析逻辑并简化。为了保证正确性，我们将FunctionScope中解析的所有需要修改的name作为了控制流的输入。这样可以保证正确性、简单性和鲁棒性
新增的了UndefinedVar来解决true、false分支定义临时变量的问题
Disable 了变长的 return ，消除了之前变长return引入的bug。也因此消除了 run_python_if 中的不一致性。不会引用两个分支中的名字，不会出现name error。

TODO：

添加新解决的单测。
删除未删除干净的代码。具体文件有： return_transformer、ifelse_transformer 等
逻辑思考：是否 while 这种具有额外的参数的 body 函数会影响 FunctionScope 解析。

Aurelius84 · 2022-07-04T12:19:00Z

python/paddle/fluid/dygraph/dygraph_to_static/convert_operators.py

+        cond_outs = control_flow.cond(pred, new_true_fn, new_false_fn,
+                                      return_name_ids)
+    except Exception as e:
+        if re.search("Unsupported return type of true_fn and false_fn in cond",


这里最好加个单测确保这行代码按照预期触发，因为报错信息有可能被别人迭代优化后，这个分支就失效了

Aurelius84 · 2022-07-04T12:20:46Z

python/paddle/fluid/dygraph/dygraph_to_static/convert_operators.py

+    while True:
+        pred = cond()
+        if isinstance(pred, Variable):
+            raise Dygraph2StaticException(


raise这个error时会触发用户源码行的标记么？

这个会触发源码行的标记：

Aurelius84 · 2022-07-04T12:21:20Z

python/paddle/fluid/dygraph/dygraph_to_static/convert_operators.py

+        if re.search("Unsupported return type of true_fn and false_fn in cond",
+                     str(e)):
+            raise Dygraph2StaticException(
+                "Your if/else have different return type. TODO: add link to modifty."


Suggested change

"Your if/else have different return type. TODO: add link to modifty."

# TODO: add link to modifty.

"Your if/else has different return type. %s" % e

这里如果还没有加TODO的话，最好先把之前的err msg re-throw下

Aurelius84 · 2022-07-04T12:22:55Z

python/paddle/fluid/dygraph/dygraph_to_static/convert_operators.py

+                "Incompatible return values of true_fn and false_fn in cond",
+                str(e)):
+            raise Dygraph2StaticException(
+                "Your if/else have different number of return value. TODO: add link to modifty."


Aurelius84 · 2022-07-04T12:25:04Z

python/paddle/fluid/dygraph/dygraph_to_static/ifelse_transformer.py

@@ -52,6 +52,8 @@ def __init__(self, wrapper_root):
        ), "Type of input node should be AstNodeWrapper, but received %s ." % type(
            wrapper_root)
        self.root = wrapper_root.node
+        FunctionNameLivenessAnalysis(
+            self.root)  # name analysis of current ast tree.
        self.static_analysis_visitor = StaticAnalysisVisitor(self.root)


self.static_analysis_visitor 没有用到了，这个我们是不是可以删除掉了？

Aurelius84 · 2022-07-04T12:31:59Z

python/paddle/fluid/dygraph/dygraph_to_static/utils.py

+def create_undefined_var_like(variable):
+    """ create a undefined var with the same shape and dtype like varaible.
+    """
+    from paddle.fluid.dygraph.dygraph_to_static.return_transformer import RETURN_NO_VALUE_MAGIC_NUM


后续可以把RETURN_NO_VALUE_MAGIC_NUM 也放到这个文件，这里import虽然时动态import，单最好让utils成为一个叶子结点的文件，可以被其他文件import。可以后续PR优化

Aurelius84 · 2022-07-04T12:32:46Z

python/paddle/fluid/dygraph/dygraph_to_static/utils.py

+    from paddle.fluid.dygraph.dygraph_to_static.return_transformer import RETURN_NO_VALUE_MAGIC_NUM
+    var = data_layer_not_check(unique_name.generate("undefined_var"),
+                               variable.shape, variable.dtype)
+    assign(RETURN_NO_VALUE_MAGIC_NUM, var)


这里为什么还需要一个assign？不可以通过name来过滤么

Aurelius84 · 2022-07-04T12:36:11Z

python/paddle/fluid/dygraph/dygraph_to_static/utils.py

    template = """
    def {func_name}():
-        nonlocal {nonlocal_vars}
+        {nonlocal_vars}


如果 nonlocal_vars 是空，还需要return names么？

Aurelius84 · 2022-07-04T12:39:06Z

python/paddle/fluid/layers/control_flow.py

@@ -1159,7 +1181,10 @@ def assign_skip_lod_tensor_array(input, output):
    Assign input to output, but skip the process of copying LoDTensorArray unless it's created in while_block.
    """
    if not isinstance(input, Variable) and not isinstance(input, core.VarBase):


Suggested change

if not isinstance(input, Variable) and not isinstance(input, core.VarBase):

if not isinstance(input, (Variable ，core.VarBase)):

Aurelius84 · 2022-07-04T12:39:48Z

python/paddle/fluid/layers/control_flow.py

@@ -2377,7 +2402,7 @@ def copy_var_to_parent_block(var, layer_helper):
    return parent_block_var


-def cond(pred, true_fn=None, false_fn=None, name=None):
+def cond(pred, true_fn=None, false_fn=None, return_name_ids=None, name=None):


Suggested change

def cond(pred, true_fn=None, false_fn=None, return_name_ids=None, name=None):

def cond(pred, true_fn=None, false_fn=None, name=None, return_names=None):

为了兼容性，新增参数一般放到最后

XiaoguangHu01

LGTM

Aurelius84

LGTM

TCChenlong · 2022-07-06T08:37:13Z

python/paddle/fluid/layers/control_flow.py

@@ -2423,6 +2454,7 @@ def cond(pred, true_fn=None, false_fn=None, name=None):
            true. The default value is ``None`` .
        false_fn(callable, optional): A callable to be performed if ``pred`` is
            false. The default value is ``None`` .
+        return_names: A list of strings to represents the name of returned vars. useful to debug.


return_names: -> return_names(支持的数据类型, optional): XXX. Default is None, means XXX.
useful to debug，太 Chinese English 了，要改一下。
顺序在 name 后面。

2742195759 added 10 commits June 29, 2022 09:32

add support for control flow block analysis

1a76531

move FunctionNameLivenessAnalysis into utils

ae2cbf2

pass test_ifelse.py

55dd278

merge

945de74

remove duplicate data_layer_not_check

0444670

pass the test_ifelse.py

0b191eb

fix unittest error .

8dd1a4b

fix all ci error in first version

d512394

merge

056f7b8

temporay disable CreateVariableTransformer

3d22432

Aurelius84 reviewed Jul 4, 2022

View reviewed changes

2742195759 added 6 commits July 4, 2022 16:29

fix ci errors

624f35f

fix function name liveness analysis bugs

17fe717

merge

0be1e75

modifty def cond

18b3125

fix

e2138f7

fix ci error - v2

15fb43a

2742195759 changed the title ~~[Dy2static] FunctionScopeVisitor Enhance and substitute the original NameVisitor in If.~~ [Dy2static] FunctionScopeVisitor Enhance and substitute the original NameVisitor in If Jul 5, 2022

fix by code review

bb6f175

2742195759 marked this pull request as draft July 5, 2022 17:31

2742195759 marked this pull request as ready for review July 5, 2022 17:32

change return_name_ids -> return_name

14f45b2

XiaoguangHu01 approved these changes Jul 6, 2022

View reviewed changes

Aurelius84 approved these changes Jul 6, 2022

View reviewed changes

TCChenlong reviewed Jul 6, 2022

View reviewed changes

TCChenlong approved these changes Jul 6, 2022

View reviewed changes

2742195759 merged commit b603dd5 into PaddlePaddle:develop Jul 6, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Dy2static] FunctionScopeVisitor Enhance and substitute the original NameVisitor in If #43967

[Dy2static] FunctionScopeVisitor Enhance and substitute the original NameVisitor in If #43967

2742195759 commented Jun 30, 2022 •

edited

Loading

Aurelius84 Jul 4, 2022

Aurelius84 Jul 4, 2022

2742195759 Jul 5, 2022

Aurelius84 Jul 4, 2022

Aurelius84 Jul 4, 2022

Aurelius84 Jul 4, 2022

Aurelius84 Jul 4, 2022

Aurelius84 Jul 4, 2022

Aurelius84 Jul 4, 2022

Aurelius84 Jul 4, 2022

Aurelius84 Jul 4, 2022

XiaoguangHu01 left a comment

Aurelius84 left a comment

TCChenlong Jul 6, 2022

	"Your if/else have different return type. TODO: add link to modifty."
	# TODO: add link to modifty.
	"Your if/else has different return type. %s" % e

	if not isinstance(input, Variable) and not isinstance(input, core.VarBase):
	if not isinstance(input, (Variable ，core.VarBase)):

	def cond(pred, true_fn=None, false_fn=None, return_name_ids=None, name=None):
	def cond(pred, true_fn=None, false_fn=None, name=None, return_names=None):

[Dy2static] FunctionScopeVisitor Enhance and substitute the original NameVisitor in If #43967

[Dy2static] FunctionScopeVisitor Enhance and substitute the original NameVisitor in If #43967

Conversation

2742195759 commented Jun 30, 2022 • edited Loading

PR types

PR changes

Describe

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

XiaoguangHu01 left a comment

Choose a reason for hiding this comment

Aurelius84 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

2742195759 commented Jun 30, 2022 •

edited

Loading