From 8674a82c034f26e401dd79a8787a806176746885 Mon Sep 17 00:00:00 2001
From: Aurelius84 <liujiezhangbupt@gmail.com>
Date: Wed, 8 Apr 2020 13:50:01 +0800
Subject: [PATCH] Op (Scope) error message enhancement (#23458)

* Op (Scope) error message enhancement test=develop
---
 paddle/fluid/framework/scope.cc                | 18 ++++++++++++------
 paddle/fluid/framework/scope_pool.cc           |  7 ++++++-
 .../paddle/fluid/tests/unittests/test_scope.py | 11 +++++++++++
 3 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/paddle/fluid/framework/scope.cc b/paddle/fluid/framework/scope.cc
index 6b83c047323..45e4c3edb05 100644
--- a/paddle/fluid/framework/scope.cc
+++ b/paddle/fluid/framework/scope.cc
@@ -125,8 +125,9 @@ std::vector<std::string> Scope::LocalVarNames() const {
 void Scope::DeleteScope(Scope* scope) const {
   SCOPE_KIDS_WRITER_LOCK
   auto it = std::find(this->kids_.begin(), this->kids_.end(), scope);
-  PADDLE_ENFORCE(it != this->kids_.end(), "%p Cannot find %p as kid scope",
-                 this, scope);
+  PADDLE_ENFORCE_NE(it, this->kids_.end(),
+                    platform::errors::NotFound(
+                        "%p is not found in %p as kid scope", scope, this));
   this->kids_.erase(it);
   // When making memory benchmark on Fluid, we have to delete scope sync.
   if (FLAGS_benchmark || FLAGS_eager_delete_scope) {
@@ -189,11 +190,16 @@ const Scope* Scope::FindScopeInternal(const std::string& name) const {
 void Scope::RenameInternal(const std::string& origin_name,
                            const std::string& new_name) const {
   auto origin_it = vars_.find(origin_name);
-  PADDLE_ENFORCE(origin_it != vars_.end(),
-                 "Cannot find original variable with name %s", origin_name);
+  PADDLE_ENFORCE_NE(
+      origin_it, vars_.end(),
+      platform::errors::NotFound(
+          "Original variable with name %s is not found in the scope.",
+          origin_name));
   auto new_it = vars_.find(new_name);
-  PADDLE_ENFORCE(new_it == vars_.end(),
-                 "The variable with name %s is already in the scope", new_name);
+  PADDLE_ENFORCE_EQ(
+      new_it, vars_.end(),
+      platform::errors::AlreadyExists(
+          "The variable with name %s already exists in the scope.", new_name));
   vars_[new_name].reset(origin_it->second.release());
   vars_.erase(origin_it);
 }
diff --git a/paddle/fluid/framework/scope_pool.cc b/paddle/fluid/framework/scope_pool.cc
index 5cb241a7a34..4bb077a2c52 100644
--- a/paddle/fluid/framework/scope_pool.cc
+++ b/paddle/fluid/framework/scope_pool.cc
@@ -11,6 +11,7 @@
 // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 // See the License for the specific language governing permissions and
 // limitations under the License.
+#include <memory>
 
 #include "paddle/fluid/framework/scope_pool.h"
 #include "paddle/fluid/framework/threadpool.h"
@@ -36,7 +37,11 @@ void ScopePool::Remove(Scope *s) {
     std::lock_guard<std::mutex> guard(mtx_);
     has_scope = scopes_.erase(s);
   }
-  PADDLE_ENFORCE(has_scope > 0, "Delete non-existing global scope");
+  PADDLE_ENFORCE_GT(
+      has_scope, 0,
+      platform::errors::NotFound("Global scope %p is not found in ScopePool. "
+                                 "Deleting a nonexistent scope is not allowed.",
+                                 s));
   DeleteScope(s);
 }
 
diff --git a/python/paddle/fluid/tests/unittests/test_scope.py b/python/paddle/fluid/tests/unittests/test_scope.py
index 45fcbfba6eb..aa093069c49 100644
--- a/python/paddle/fluid/tests/unittests/test_scope.py
+++ b/python/paddle/fluid/tests/unittests/test_scope.py
@@ -16,6 +16,7 @@ from __future__ import print_function
 
 import paddle.fluid.core
 import unittest
+import six
 
 
 class TestScope(unittest.TestCase):
@@ -48,6 +49,16 @@ class TestScope(unittest.TestCase):
         self.assertTrue(var.is_int())
         self.assertEqual(10, var.get_int())
 
+    def test_scope_pool(self):
+        paddle_c = paddle.fluid.core
+        scope = paddle_c.Scope()
+        # Delete the scope.
+        scope._remove_from_pool()
+        with self.assertRaisesRegexp(
+                Exception, "Deleting a nonexistent scope is not allowed*"):
+            # It is not allowed to delete a nonexistent scope.
+            scope._remove_from_pool()
+
 
 if __name__ == '__main__':
     unittest.main()
-- 
GitLab