From 131de920cda98f222eb80ae18b0b7b6b3c428a53 Mon Sep 17 00:00:00 2001 From: pangyoki Date: Fri, 19 Nov 2021 04:09:57 +0000 Subject: [PATCH 1/4] fix inplace bug --- paddle/fluid/imperative/basic_engine.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/paddle/fluid/imperative/basic_engine.cc b/paddle/fluid/imperative/basic_engine.cc index 62d449ccd2ea8c..e4a78b69e2079c 100644 --- a/paddle/fluid/imperative/basic_engine.cc +++ b/paddle/fluid/imperative/basic_engine.cc @@ -73,7 +73,6 @@ void BasicEngine::Init( VLOG(5) << "Clear the auto-grad graph from grad var " << var->Name() << " because of retain_graph=False when calling backward"; var->GradVarBase()->SetGraphIsFreed(true); - var->GradVarBase()->ClearGradNode(); } if (init_node == nullptr || var->OverridedStopGradient()) { From e4ed837ccff192d77aa97a671bca21ccb7e1ceb6 Mon Sep 17 00:00:00 2001 From: pangyoki Date: Mon, 22 Nov 2021 07:17:57 +0000 Subject: [PATCH 2/4] fix custom grad input error --- paddle/fluid/imperative/basic_engine.cc | 37 ++++++++++++++++++------- 1 file changed, 27 insertions(+), 10 deletions(-) diff --git a/paddle/fluid/imperative/basic_engine.cc b/paddle/fluid/imperative/basic_engine.cc index e4a78b69e2079c..45f37f32d70019 100644 --- a/paddle/fluid/imperative/basic_engine.cc +++ b/paddle/fluid/imperative/basic_engine.cc @@ -53,6 +53,10 @@ void BasicEngine::Init( platform::errors::AlreadyExists( "Accumulators are not empty before preparing it for " "backward network execution.")); + PADDLE_ENFORCE_EQ(accumulators_with_grad_node_.empty(), true, + platform::errors::AlreadyExists( + "Accumulators with grad_node as the key are not empty " + "before preparing it for backward network execution.")); for (size_t i = 0; i < tensors.size(); ++i) { auto var = tensors[i]; @@ -107,13 +111,30 @@ void BasicEngine::Init( } VariableWrapper* init_grad_var = var->GradVarBase()->SharedVar().get(); - auto& accumulator = accumulators_[init_grad_var]; - if (!accumulator) { - if (FLAGS_sort_sum_gradient) { - accumulator.reset(new SortedGradientAccumulator(init_grad_var)); - } else { - accumulator.reset(new EagerGradientAccumulator(init_grad_var)); + if (!init_grad_var->HasGradNode()) { + auto& accumulator = accumulators_[init_grad_var]; + if (!accumulator) { + if (FLAGS_sort_sum_gradient) { + accumulator.reset(new SortedGradientAccumulator(init_grad_var)); + } else { + accumulator.reset(new EagerGradientAccumulator(init_grad_var)); + } } + accumulator->IncreaseRefCnt(); + accumulator->IncreaseCurCnt(); + } else { + auto& accumulator = + accumulators_with_grad_node_[init_grad_var->GetGradNode()] + [init_grad_var]; + if (!accumulator) { + if (FLAGS_sort_sum_gradient) { + accumulator.reset(new SortedGradientAccumulator(init_grad_var)); + } else { + accumulator.reset(new EagerGradientAccumulator(init_grad_var)); + } + } + accumulator->IncreaseRefCnt(); + accumulator->IncreaseCurCnt(); } init_nodes_.push_back(init_node); @@ -252,10 +273,6 @@ void BasicEngine::PrepareDeps() { node_deps_.empty(), true, platform::errors::AlreadyExists("Op deps are not empty before preparing " "it for backward network execution.")); - PADDLE_ENFORCE_EQ(accumulators_with_grad_node_.empty(), true, - platform::errors::AlreadyExists( - "Accumulators with grad_node as the key are not empty " - "before preparing it for backward network execution.")); std::queue q; std::unordered_set visited; From e64f09abf971dd9f98b25f729552db3691a65d62 Mon Sep 17 00:00:00 2001 From: pangyoki Date: Mon, 22 Nov 2021 07:42:24 +0000 Subject: [PATCH 3/4] add unittest --- .../fluid/tests/unittests/test_inplace.py | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/python/paddle/fluid/tests/unittests/test_inplace.py b/python/paddle/fluid/tests/unittests/test_inplace.py index 3d158763527e71..98e2d2367fd5ed 100644 --- a/python/paddle/fluid/tests/unittests/test_inplace.py +++ b/python/paddle/fluid/tests/unittests/test_inplace.py @@ -409,5 +409,30 @@ def inplace_api_processing(self, var): return var.subtract_(self.input_var_2) +class TestLossIsInplaceVar(unittest.TestCase): + def test_loss_is_inplace_var(self): + with paddle.fluid.dygraph.guard(): + var_a = paddle.ones((2, 2)) + var_a.stop_gradient = False + + var_b = var_a * 2 + loss = var_b.tanh_() + + loss.backward() + inplace_grad_var_a = var_a.grad.numpy() + + with paddle.fluid.dygraph.guard(): + var_a = paddle.ones((2, 2)) + var_a.stop_gradient = False + + var_b = var_a * 2 + loss = var_b.tanh() + + loss.backward() + grad_var_a = var_a.grad.numpy() + + self.assertTrue(np.array_equal(inplace_grad_var_a, grad_var_a)) + + if __name__ == '__main__': unittest.main() From 45a423b61b51ed6916439883cc05ee8bcffa105d Mon Sep 17 00:00:00 2001 From: pangyoki Date: Mon, 22 Nov 2021 08:11:10 +0000 Subject: [PATCH 4/4] fix inplace bug --- paddle/fluid/imperative/basic_engine.cc | 33 ++++++++----------------- 1 file changed, 10 insertions(+), 23 deletions(-) diff --git a/paddle/fluid/imperative/basic_engine.cc b/paddle/fluid/imperative/basic_engine.cc index 45f37f32d70019..d5402699553c7f 100644 --- a/paddle/fluid/imperative/basic_engine.cc +++ b/paddle/fluid/imperative/basic_engine.cc @@ -111,31 +111,18 @@ void BasicEngine::Init( } VariableWrapper* init_grad_var = var->GradVarBase()->SharedVar().get(); - if (!init_grad_var->HasGradNode()) { - auto& accumulator = accumulators_[init_grad_var]; - if (!accumulator) { - if (FLAGS_sort_sum_gradient) { - accumulator.reset(new SortedGradientAccumulator(init_grad_var)); - } else { - accumulator.reset(new EagerGradientAccumulator(init_grad_var)); - } - } - accumulator->IncreaseRefCnt(); - accumulator->IncreaseCurCnt(); - } else { - auto& accumulator = - accumulators_with_grad_node_[init_grad_var->GetGradNode()] - [init_grad_var]; - if (!accumulator) { - if (FLAGS_sort_sum_gradient) { - accumulator.reset(new SortedGradientAccumulator(init_grad_var)); - } else { - accumulator.reset(new EagerGradientAccumulator(init_grad_var)); - } + auto& accumulator = + accumulators_with_grad_node_[init_grad_var->GetGradNode()] + [init_grad_var]; + if (!accumulator) { + if (FLAGS_sort_sum_gradient) { + accumulator.reset(new SortedGradientAccumulator(init_grad_var)); + } else { + accumulator.reset(new EagerGradientAccumulator(init_grad_var)); } - accumulator->IncreaseRefCnt(); - accumulator->IncreaseCurCnt(); } + accumulator->IncreaseRefCnt(); + accumulator->IncreaseCurCnt(); init_nodes_.push_back(init_node); }