From d3b180d6da9c07f3672ff5046d1863deb2865081 Mon Sep 17 00:00:00 2001 From: 0x45f Date: Mon, 6 Dec 2021 10:55:10 +0000 Subject: [PATCH 1/4] allow ifelse return `int` in paddle cond --- .../dygraph_to_static/convert_operators.py | 16 ++++++------ python/paddle/fluid/layers/control_flow.py | 26 ++++++++++++++++++- .../dygraph_to_static/ifelse_simple_func.py | 13 ++++++++++ .../dygraph_to_static/test_ifelse.py | 12 +++++++++ 4 files changed, 58 insertions(+), 9 deletions(-) diff --git a/python/paddle/fluid/dygraph/dygraph_to_static/convert_operators.py b/python/paddle/fluid/dygraph/dygraph_to_static/convert_operators.py index 0ac4da947a46b..0c48a84337af7 100644 --- a/python/paddle/fluid/dygraph/dygraph_to_static/convert_operators.py +++ b/python/paddle/fluid/dygraph/dygraph_to_static/convert_operators.py @@ -253,14 +253,14 @@ def _run_paddle_cond(pred, true_fn, false_fn, true_args, false_args, # NOTE 1: Returned vars of Paddle op `control_flow.cond` must be Paddle Tensors # NOTE 2: Here uses id(var) not var, because `if var in return_var` use operator `==`, # which will call `fluid.layers.equal` and causes error when var in return_vars is not initialized. - true_args = [ - to_static_variable(var) if id(var) in return_var_ids else var - for var in true_args - ] - false_args = [ - to_static_variable(var) if id(var) in return_var_ids else var - for var in false_args - ] + # true_args = [ + # to_static_variable(var) if id(var) in return_var_ids else var + # for var in true_args + # ] + # false_args = [ + # to_static_variable(var) if id(var) in return_var_ids else var + # for var in false_args + # ] pred = cast_bool_if_necessary(pred) return control_flow.cond(pred, lambda: true_fn(*true_args), diff --git a/python/paddle/fluid/layers/control_flow.py b/python/paddle/fluid/layers/control_flow.py index af2316a9a443e..4ebfee47da200 100755 --- a/python/paddle/fluid/layers/control_flow.py +++ b/python/paddle/fluid/layers/control_flow.py @@ -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 + false_var, true_var = inputs + + if isinstance(false_var, type(true_var)) and isinstance(false_var, ( + bool, float, six.integer_types)): + if false_var == true_var: + return false_var + else: + inputs = [ + to_static_variable(false_var), to_static_variable(true_var) + ] + # When false_var is int, true_var is Variable(no value placehold) will cause Error + elif (isinstance(false_var, (bool, float, six.integer_types)) and + 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)] + + return select_input(inputs, mask) + + def split_lod_tensor(input, mask, level=0): """ This function takes in an input that contains the complete lod information, @@ -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)): + return var prog = layer_helper.main_program parent_idx = prog.current_block().parent_idx assert parent_idx >= 0, "Got wrong parent block index when assigning var to parent scope in control_flow" @@ -2466,7 +2490,7 @@ def false_func(): format(e)) mask = cast(pred, dtype='int32') - merge_func = lambda false_var, true_var : select_input([false_var, true_var], mask) + merge_func = lambda false_var, true_var : select_input_with_buildin_type([false_var, true_var], mask) merged_output = map_structure(merge_func, false_output, true_output) return merged_output diff --git a/python/paddle/fluid/tests/unittests/dygraph_to_static/ifelse_simple_func.py b/python/paddle/fluid/tests/unittests/dygraph_to_static/ifelse_simple_func.py index b343c54d6b1ee..3d1fc7a172d7e 100644 --- a/python/paddle/fluid/tests/unittests/dygraph_to_static/ifelse_simple_func.py +++ b/python/paddle/fluid/tests/unittests/dygraph_to_static/ifelse_simple_func.py @@ -340,3 +340,16 @@ def if_tensor_case(x): x += 1 return x + + +def dyfunc_ifelse_ret_int1(x): + index = 0 + pred = paddle.to_tensor([1]) + if pred: + y = x[index] + 1 + index = index + 1 + return y, index + else: + y = x[index] + 2 + index = index + 1 + return y, index diff --git a/python/paddle/fluid/tests/unittests/dygraph_to_static/test_ifelse.py b/python/paddle/fluid/tests/unittests/dygraph_to_static/test_ifelse.py index 5db1bb2a384f5..1975e451acc4c 100644 --- a/python/paddle/fluid/tests/unittests/dygraph_to_static/test_ifelse.py +++ b/python/paddle/fluid/tests/unittests/dygraph_to_static/test_ifelse.py @@ -365,5 +365,17 @@ def case_func(training): self.assertEqual(paddle.jit.to_static(case_func)(True), -2) +class TestDygraphIfElseRetInt(unittest.TestCase): + def setUp(self): + self.x = np.random.random([5]).astype('float32') + self.dyfunc = dyfunc_ifelse_ret_int1 + + def test_ast_to_func(self): + static_func = paddle.jit.to_static(self.dyfunc) + out = static_func(self.x) + self.assertIsInstance(out[0], paddle.Tensor) + self.assertEqual(out[1], 1) + + if __name__ == '__main__': unittest.main() From 34486fcfb67011215063c6857d68cea60d330b64 Mon Sep 17 00:00:00 2001 From: 0x45f Date: Tue, 7 Dec 2021 03:44:08 +0000 Subject: [PATCH 2/4] add test and refine code --- .../dygraph_to_static/convert_operators.py | 2 +- python/paddle/fluid/layers/control_flow.py | 2 +- .../dygraph_to_static/ifelse_simple_func.py | 13 ++++++++++++ .../dygraph_to_static/test_ifelse.py | 20 +++++++++++++++++-- 4 files changed, 33 insertions(+), 4 deletions(-) diff --git a/python/paddle/fluid/dygraph/dygraph_to_static/convert_operators.py b/python/paddle/fluid/dygraph/dygraph_to_static/convert_operators.py index 0c48a84337af7..76e3527fd3336 100644 --- a/python/paddle/fluid/dygraph/dygraph_to_static/convert_operators.py +++ b/python/paddle/fluid/dygraph/dygraph_to_static/convert_operators.py @@ -254,7 +254,7 @@ def _run_paddle_cond(pred, true_fn, false_fn, true_args, false_args, # NOTE 2: Here uses id(var) not var, because `if var in return_var` use operator `==`, # which will call `fluid.layers.equal` and causes error when var in return_vars is not initialized. # true_args = [ - # to_static_variable(var) if id(var) in return_var_ids else var + # to_static_variable(var) if id(var) in return_var_ids else var # for var in true_args # ] # false_args = [ diff --git a/python/paddle/fluid/layers/control_flow.py b/python/paddle/fluid/layers/control_flow.py index 4ebfee47da200..d835bfb433370 100755 --- a/python/paddle/fluid/layers/control_flow.py +++ b/python/paddle/fluid/layers/control_flow.py @@ -114,7 +114,7 @@ def select_input_with_buildin_type(inputs, mask): inputs = [ to_static_variable(false_var), to_static_variable(true_var) ] - # When false_var is int, true_var is Variable(no value placehold) will cause Error + # Deal with this situation: false_var is int and true_var is Variable elif (isinstance(false_var, (bool, float, six.integer_types)) and isinstance(true_var, Variable)) or (isinstance(true_var, ( bool, float, six.integer_types)) and isinstance(false_var, diff --git a/python/paddle/fluid/tests/unittests/dygraph_to_static/ifelse_simple_func.py b/python/paddle/fluid/tests/unittests/dygraph_to_static/ifelse_simple_func.py index 3d1fc7a172d7e..56f4e47a244dc 100644 --- a/python/paddle/fluid/tests/unittests/dygraph_to_static/ifelse_simple_func.py +++ b/python/paddle/fluid/tests/unittests/dygraph_to_static/ifelse_simple_func.py @@ -353,3 +353,16 @@ def dyfunc_ifelse_ret_int1(x): y = x[index] + 2 index = index + 1 return y, index + + +def dyfunc_ifelse_ret_int2(x): + index = 0 + pred = paddle.to_tensor([1]) + if pred: + y = x[index] + 1 + index = index + 1 + return y, index + else: + y = x[index] + 2 + index = index + 1 + return y diff --git a/python/paddle/fluid/tests/unittests/dygraph_to_static/test_ifelse.py b/python/paddle/fluid/tests/unittests/dygraph_to_static/test_ifelse.py index 1975e451acc4c..da358fc8e9d50 100644 --- a/python/paddle/fluid/tests/unittests/dygraph_to_static/test_ifelse.py +++ b/python/paddle/fluid/tests/unittests/dygraph_to_static/test_ifelse.py @@ -365,16 +365,32 @@ def case_func(training): self.assertEqual(paddle.jit.to_static(case_func)(True), -2) -class TestDygraphIfElseRetInt(unittest.TestCase): +class TestDy2StIfElseRetInt1(unittest.TestCase): def setUp(self): self.x = np.random.random([5]).astype('float32') self.dyfunc = dyfunc_ifelse_ret_int1 def test_ast_to_func(self): + ProgramTranslator().enable(True) static_func = paddle.jit.to_static(self.dyfunc) out = static_func(self.x) self.assertIsInstance(out[0], paddle.Tensor) - self.assertEqual(out[1], 1) + self.assertIsInstance(out[1], int) + ProgramTranslator().enable(False) + + +class TestDy2StIfElseRetInt2(TestDy2StIfElseRetInt1): + def setUp(self): + self.x = np.random.random([5]).astype('float32') + self.dyfunc = dyfunc_ifelse_ret_int2 + + def test_ast_to_func(self): + ProgramTranslator().enable(True) + static_func = paddle.jit.to_static(self.dyfunc) + out = static_func(self.x) + self.assertIsInstance(out[0], paddle.Tensor) + self.assertIsInstance(out[1], paddle.Tensor) + ProgramTranslator().enable(False) if __name__ == '__main__': From ed60681cc10d25856d41b1a2f0d91684b154790f Mon Sep 17 00:00:00 2001 From: 0x45f Date: Wed, 8 Dec 2021 08:55:53 +0000 Subject: [PATCH 3/4] polish code, add test --- .../dygraph_to_static/convert_operators.py | 14 ------ python/paddle/fluid/layers/control_flow.py | 31 ++++++++----- .../dygraph_to_static/ifelse_simple_func.py | 24 +++++++++++ .../dygraph_to_static/test_ifelse.py | 43 +++++++++++++++---- 4 files changed, 80 insertions(+), 32 deletions(-) diff --git a/python/paddle/fluid/dygraph/dygraph_to_static/convert_operators.py b/python/paddle/fluid/dygraph/dygraph_to_static/convert_operators.py index 76e3527fd3336..606e3757442c8 100644 --- a/python/paddle/fluid/dygraph/dygraph_to_static/convert_operators.py +++ b/python/paddle/fluid/dygraph/dygraph_to_static/convert_operators.py @@ -248,20 +248,6 @@ def _remove_no_value_return_var(out): def _run_paddle_cond(pred, true_fn, false_fn, true_args, false_args, return_vars): - - return_var_ids = [id(var) for var in return_vars] - # NOTE 1: Returned vars of Paddle op `control_flow.cond` must be Paddle Tensors - # NOTE 2: Here uses id(var) not var, because `if var in return_var` use operator `==`, - # which will call `fluid.layers.equal` and causes error when var in return_vars is not initialized. - # true_args = [ - # to_static_variable(var) if id(var) in return_var_ids else var - # for var in true_args - # ] - # false_args = [ - # to_static_variable(var) if id(var) in return_var_ids else var - # for var in false_args - # ] - pred = cast_bool_if_necessary(pred) return control_flow.cond(pred, lambda: true_fn(*true_args), lambda: false_fn(*false_args)) diff --git a/python/paddle/fluid/layers/control_flow.py b/python/paddle/fluid/layers/control_flow.py index d835bfb433370..8bc191651f4db 100755 --- a/python/paddle/fluid/layers/control_flow.py +++ b/python/paddle/fluid/layers/control_flow.py @@ -104,22 +104,35 @@ def select_input(inputs, mask): def select_input_with_buildin_type(inputs, mask): from paddle.fluid.dygraph.dygraph_to_static.variable_trans_func import to_static_variable + support_ret_buildin_type = (bool, float, six.integer_types) false_var, true_var = inputs - if isinstance(false_var, type(true_var)) and isinstance(false_var, ( - bool, float, six.integer_types)): + if isinstance(false_var, Variable) and isinstance(true_var, Variable): + return select_input(inputs, mask) + + elif (isinstance(false_var, (support_ret_buildin_type)) and isinstance( + false_var, type(true_var))): if false_var == true_var: return false_var else: inputs = [ to_static_variable(false_var), to_static_variable(true_var) ] - # Deal with this situation: false_var is int and true_var is Variable - elif (isinstance(false_var, (bool, float, six.integer_types)) and - isinstance(true_var, Variable)) or (isinstance(true_var, ( - bool, float, six.integer_types)) and isinstance(false_var, - Variable)): + # Deal with the situations like this: false_var is int and true_var is Variable + elif ((isinstance(false_var, support_ret_buildin_type) and + isinstance(true_var, Variable)) or + (isinstance(true_var, support_ret_buildin_type) and + isinstance(false_var, Variable))): inputs = [to_static_variable(false_var), to_static_variable(true_var)] + warnings.warn( + "Return results from different branches in cond are not same type: " + "false_var returned by fasle_fn is '{}' and true_var of true_fn is " + "'{}'".format(type(false_var), type(true_var))) + else: + raise TypeError( + "Unsupported return type of true_fn and false_fn in cond: false_var " + "returned by fasle_fn is '{}' and true_var of true_fn is '{}'". + format(type(false_var), type(true_var))) return select_input(inputs, mask) @@ -2304,9 +2317,7 @@ 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)): + if not isinstance(var, Variable): return var prog = layer_helper.main_program parent_idx = prog.current_block().parent_idx diff --git a/python/paddle/fluid/tests/unittests/dygraph_to_static/ifelse_simple_func.py b/python/paddle/fluid/tests/unittests/dygraph_to_static/ifelse_simple_func.py index 56f4e47a244dc..04f44f68b4234 100644 --- a/python/paddle/fluid/tests/unittests/dygraph_to_static/ifelse_simple_func.py +++ b/python/paddle/fluid/tests/unittests/dygraph_to_static/ifelse_simple_func.py @@ -366,3 +366,27 @@ def dyfunc_ifelse_ret_int2(x): y = x[index] + 2 index = index + 1 return y + + +def dyfunc_ifelse_ret_int3(x): + index = 0 + pred = paddle.to_tensor([1]) + if pred: + y = x[index] + 1 + index = index + 1 + return index + else: + y = x[index] + 2 + return y + + +def dyfunc_ifelse_ret_int4(x): + index = 0 + pred = paddle.to_tensor([1]) + if pred: + y = x[index] + 1 + index = index + 1 + return 'unsupport ret' + else: + y = x[index] + 2 + return y diff --git a/python/paddle/fluid/tests/unittests/dygraph_to_static/test_ifelse.py b/python/paddle/fluid/tests/unittests/dygraph_to_static/test_ifelse.py index da358fc8e9d50..7e999e3b21a88 100644 --- a/python/paddle/fluid/tests/unittests/dygraph_to_static/test_ifelse.py +++ b/python/paddle/fluid/tests/unittests/dygraph_to_static/test_ifelse.py @@ -369,28 +369,55 @@ class TestDy2StIfElseRetInt1(unittest.TestCase): def setUp(self): self.x = np.random.random([5]).astype('float32') self.dyfunc = dyfunc_ifelse_ret_int1 + self.out = self.get_dy2stat_out() - def test_ast_to_func(self): + def get_dy2stat_out(self): ProgramTranslator().enable(True) static_func = paddle.jit.to_static(self.dyfunc) out = static_func(self.x) - self.assertIsInstance(out[0], paddle.Tensor) - self.assertIsInstance(out[1], int) ProgramTranslator().enable(False) + return out + + def test_ast_to_func(self): + self.assertIsInstance(self.out[0], paddle.Tensor) + self.assertIsInstance(self.out[1], int) class TestDy2StIfElseRetInt2(TestDy2StIfElseRetInt1): def setUp(self): self.x = np.random.random([5]).astype('float32') self.dyfunc = dyfunc_ifelse_ret_int2 + self.out = self.get_dy2stat_out() def test_ast_to_func(self): - ProgramTranslator().enable(True) - static_func = paddle.jit.to_static(self.dyfunc) - out = static_func(self.x) - self.assertIsInstance(out[0], paddle.Tensor) - self.assertIsInstance(out[1], paddle.Tensor) + self.assertIsInstance(self.out[0], paddle.Tensor) + self.assertIsInstance(self.out[1], paddle.Tensor) + + +class TestDy2StIfElseRetInt3(TestDy2StIfElseRetInt1): + def setUp(self): + self.x = np.random.random([5]).astype('float32') + self.dyfunc = dyfunc_ifelse_ret_int3 + self.out = self.get_dy2stat_out() + + def test_ast_to_func(self): + self.assertIsInstance(self.out, paddle.Tensor) + + +class TestDy2StIfElseRetInt4(TestDy2StIfElseRetInt1): + def setUp(self): + self.x = np.random.random([5]).astype('float32') + self.dyfunc = dyfunc_ifelse_ret_int4 + + def test_ast_to_func(self): + with self.assertRaises(TypeError): + ProgramTranslator().enable(True) + static_func = paddle.jit.to_static(self.dyfunc) + out = static_func(self.x) + + def __del__(self): ProgramTranslator().enable(False) + super(TestDy2StIfElseRetInt4, self).__del__() if __name__ == '__main__': From 2b6632e052321a7dadc2687ae690f3a4581e035c Mon Sep 17 00:00:00 2001 From: 0x45f Date: Wed, 8 Dec 2021 10:58:50 +0000 Subject: [PATCH 4/4] code format --- python/paddle/fluid/layers/control_flow.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/paddle/fluid/layers/control_flow.py b/python/paddle/fluid/layers/control_flow.py index 8bc191651f4db..668cb01549f6c 100755 --- a/python/paddle/fluid/layers/control_flow.py +++ b/python/paddle/fluid/layers/control_flow.py @@ -110,8 +110,8 @@ def select_input_with_buildin_type(inputs, mask): if isinstance(false_var, Variable) and isinstance(true_var, Variable): return select_input(inputs, mask) - elif (isinstance(false_var, (support_ret_buildin_type)) and isinstance( - false_var, type(true_var))): + elif (isinstance(false_var, (support_ret_buildin_type)) and + isinstance(false_var, type(true_var))): if false_var == true_var: return false_var else: