Refine VarBase init function (#21587)

* refine init function, test=develop

* add tests, test=develop

* remove extern, which may cause symbol error in gcc-4.8, test=develop

paddle/fluid/imperative/tests/test_tracer.cc

@@ -280,8 +280,8 @@ TEST(test_tracer, test_unique_name_generator) {
   imperative::Tracer tracer;
   auto fc_1 = tracer.GenerateUniqueName("fc");
   auto fc_2 = tracer.GenerateUniqueName("fc");
-  ASSERT_STREQ("fc_1", fc_1.c_str());
-  ASSERT_STREQ("fc_2", fc_2.c_str());
+  ASSERT_STREQ("fc_0", fc_1.c_str());
+  ASSERT_STREQ("fc_1", fc_2.c_str());
 }
 
 TEST(test_tracer, test_current_tracer) {

paddle/fluid/imperative/tracer.h

@@ -33,7 +33,7 @@ class UniqueNameGenerator {
  public:
   explicit UniqueNameGenerator(std::string prefix = "") : prefix_(prefix) {}
   std::string Generate(std::string key = "tmp") {
-    return prefix_ + key + "_" + std::to_string(++id_);
+    return prefix_ + key + "_" + std::to_string(id_++);
   }
 
  private:

paddle/fluid/pybind/imperative.cc

@@ -31,18 +31,13 @@ limitations under the License. */
 #include "paddle/fluid/imperative/tracer.h"
 #include "paddle/fluid/imperative/type_defs.h"
 #include "paddle/fluid/pybind/pybind_boost_headers.h"
+#include "paddle/fluid/pybind/tensor_py.h"
 
 namespace paddle {
 namespace pybind {
 
 namespace py = ::pybind11;
 
-template <typename P>
-extern void SetTensorFromPyArray(framework::Tensor *self, const py::object &obj,
-                                 const P &place, bool zero_copy);
-extern py::array TensorToPyArray(const framework::Tensor &tensor,
-                                 bool need_deep_copy = false);
-
 class Layer : public imperative::Layer {
  public:
   using imperative::Layer::Layer;  // Inherit constructors
@@ -55,16 +50,30 @@ class Layer : public imperative::Layer {
   }
 };
 
-static void InitTensorForVarBase(imperative::VarBase *self, bool persistable,
-                                 bool is_default, const py::array &array,
-                                 const py::object &obj = py::object(),
-                                 bool zero_copy = false) {
-  new (self) imperative::VarBase(
-      imperative::GetCurrentTracer()->GenerateUniqueName("generated_var_"));
-  self->SetPersistable(persistable);
+static const platform::Place PyObjectToPlace(const py::object &place_obj) {
+  if (py::isinstance<platform::CPUPlace>(place_obj)) {
+    return place_obj.cast<platform::CPUPlace>();
+  } else if (py::isinstance<platform::CUDAPlace>(place_obj)) {
+    return place_obj.cast<platform::CUDAPlace>();
+  } else if (py::isinstance<platform::CUDAPinnedPlace>(place_obj)) {
+    return place_obj.cast<platform::CUDAPinnedPlace>();
+  } else {
+    PADDLE_THROW(platform::errors::InvalidArgument(
+        "Place should be one of CPUPlace/CUDAPlace/CUDAPinnedPlace"));
+  }
+}
+
+static void InitTensorForVarBase(imperative::VarBase *self,
+                                 const py::array &array,
+                                 const platform::Place place,
+                                 bool persistable = false,
+                                 bool zero_copy = false,
+                                 std::string name = "") {
+  if (name == "") {
+    name = imperative::GetCurrentTracer()->GenerateUniqueName("generated_var");
+  }
+  new (self) imperative::VarBase(name);
   auto *tensor = self->MutableVar()->GetMutable<framework::LoDTensor>();
-  if (is_default) {
-    auto place = imperative::GetCurrentTracer()->ExpectedPlace();
   if (platform::is_cpu_place(place)) {
     SetTensorFromPyArray<platform::CPUPlace>(
         tensor, array, boost::get<platform::CPUPlace>(place), zero_copy);
@@ -73,27 +82,12 @@ static void InitTensorForVarBase(imperative::VarBase *self, bool persistable,
         tensor, array, boost::get<platform::CUDAPlace>(place), zero_copy);
   } else if (platform::is_cuda_pinned_place(place)) {
     SetTensorFromPyArray<platform::CUDAPinnedPlace>(
-          tensor, array, boost::get<platform::CUDAPinnedPlace>(place),
-          zero_copy);
-    } else {
-      PADDLE_THROW(platform::errors::InvalidArgument(
-          "Place should be one of CPUPlace/CUDAPlace/CUDAPinnedPlace"));
-    }
-  } else {
-    if (py::isinstance<platform::CPUPlace>(obj)) {
-      SetTensorFromPyArray<platform::CPUPlace>(
-          tensor, array, obj.cast<platform::CPUPlace>(), zero_copy);
-    } else if (py::isinstance<platform::CUDAPlace>(obj)) {
-      SetTensorFromPyArray<platform::CUDAPlace>(
-          tensor, array, obj.cast<platform::CUDAPlace>(), zero_copy);
-    } else if (py::isinstance<platform::CUDAPinnedPlace>(obj)) {
-      SetTensorFromPyArray<platform::CUDAPinnedPlace>(
-          tensor, array, obj.cast<platform::CUDAPinnedPlace>(), zero_copy);
+        tensor, array, boost::get<platform::CUDAPinnedPlace>(place), zero_copy);
   } else {
     PADDLE_THROW(platform::errors::InvalidArgument(
         "Place should be one of CPUPlace/CUDAPlace/CUDAPinnedPlace"));
   }
-  }
+  self->SetPersistable(persistable);
   self->SetType(framework::proto::VarType::LOD_TENSOR);
   self->SetDataType(tensor->type());
 }
@@ -103,28 +97,32 @@ static void InitVarBaseFromNumpyWithKwargs(imperative::VarBase *self,
   PADDLE_ENFORCE_EQ(
       kwargs.contains("value"), true,
       platform::errors::InvalidArgument("Missing argument: value"));
-  if (kwargs.contains("place")) {
-    InitTensorForVarBase(self, kwargs.contains("persistable")
-                                   ? kwargs["persistable"].cast<bool>()
-                                   : false,
-                         false, kwargs["value"].cast<py::array>(),
-                         kwargs["place"], kwargs["zero_copy"].cast<bool>());
-  } else {
-    InitTensorForVarBase(self, kwargs.contains("persistable")
+
+  auto persistable = kwargs.contains("persistable")
                          ? kwargs["persistable"].cast<bool>()
-                                   : false,
-                         true, kwargs["value"].cast<py::array>(), py::object(),
-                         kwargs["zero_copy"].cast<bool>());
-  }
+                         : false;
+  auto array = kwargs.contains("value") ? kwargs["value"].cast<py::array>()
+                                        : py::array();
+  auto zero_copy =
+      kwargs.contains("zero_copy") ? kwargs["zero_copy"].cast<bool>() : false;
+  auto name = kwargs.contains("name") ? kwargs["name"].cast<std::string>() : "";
+  auto default_place = imperative::GetCurrentTracer()->ExpectedPlace();
+  auto place = kwargs.contains("place") ? PyObjectToPlace(kwargs["place"])
+                                        : default_place;
+  InitTensorForVarBase(self, array, place, persistable, zero_copy, name);
 }
 
 template <typename P>
 static void InitVarBaseFromNumpyWithArg(imperative::VarBase *self,
                                         const py::array &array, const P &place,
-                                        bool persistable, bool zero_copy) {
-  // 0: value, 1: place, 2: name 3: persistable, 4: zero_copy
-  new (self) imperative::VarBase(
-      imperative::GetCurrentTracer()->GenerateUniqueName("generated_var_"));
+                                        bool persistable = false,
+                                        bool zero_copy = false,
+                                        std::string name = "") {
+  // 0: self, 1: value, 2: place, 3: persistable, 4: zero_copy, 5: name
+  if (name == "") {
+    name = imperative::GetCurrentTracer()->GenerateUniqueName("generated_var");
+  }
+  new (self) imperative::VarBase(name);
   self->SetPersistable(persistable);
   auto *tensor = self->MutableVar()->GetMutable<framework::LoDTensor>();
   SetTensorFromPyArray<P>(tensor, array, place, zero_copy);
@@ -133,9 +131,9 @@ static void InitVarBaseFromNumpyWithArg(imperative::VarBase *self,
 }
 
 static void InitVarBaseFromNumpyWithArgDefault(imperative::VarBase *self,
-                                               const py::array &array,
-                                               bool persistable) {
-  InitTensorForVarBase(self, persistable, true, array);
+                                               const py::array &array) {
+  auto place = imperative::GetCurrentTracer()->ExpectedPlace();
+  InitTensorForVarBase(self, array, place);
 }
 
 static std::string GetTypeName(const imperative::VarBase &var) {
@@ -147,6 +145,7 @@ static std::string GetTypeName(const imperative::VarBase &var) {
     return framework::ToTypeName(var.Var().Type());
   }
 }
+
 using PyNameVarBaseMap = std::unordered_map<std::string, py::handle>;
 
 template <typename T>
@@ -301,15 +300,14 @@ void BindImperative(py::module *m_ptr) {
            })
       .def("__init__", &InitVarBaseFromNumpyWithArg<platform::CPUPlace>,
            py::arg("value"), py::arg("place"), py::arg("persistable") = false,
-           py::arg("zero_copy") = false)
+           py::arg("zero_copy") = false, py::arg("name") = "")
       .def("__init__", &InitVarBaseFromNumpyWithArg<platform::CUDAPlace>,
            py::arg("value"), py::arg("place"), py::arg("persistable") = false,
-           py::arg("zero_copy") = false)
+           py::arg("zero_copy") = false, py::arg("name") = "")
       .def("__init__", &InitVarBaseFromNumpyWithArg<platform::CUDAPinnedPlace>,
            py::arg("value"), py::arg("place"), py::arg("persistable") = false,
-           py::arg("zero_copy") = false)
-      .def("__init__", &InitVarBaseFromNumpyWithArgDefault, py::arg("value"),
-           py::arg("persistable") = false)
+           py::arg("zero_copy") = false, py::arg("name") = "")
+      .def("__init__", &InitVarBaseFromNumpyWithArgDefault, py::arg("value"))
       .def("__init__", &InitVarBaseFromNumpyWithKwargs)
       .def("numpy",
            [](imperative::VarBase &self) -> py::array {
@@ -384,26 +382,6 @@ void BindImperative(py::module *m_ptr) {
                     y = x.detach()
 
        )DOC")
-      .def("_run_backward",
-           [](imperative::VarBase &self,
-              const imperative::detail::BackwardStrategy &bckst,
-              const imperative::Tracer &tracer) {
-             // TODO(jiabin): when we impl more backward execution we can select
-             // them
-
-             imperative::Engine *engine = tracer.GetDefaultEngine();
-             VLOG(3) << "Start backward";
-             engine->Init(&self, bckst);
-             engine->Execute();
-             VLOG(3) << "Finish backward";
-           },
-           py::call_guard<py::gil_scoped_release>())
-      .def("_grad_name", &imperative::VarBase::GradVarName)
-      .def("_grad_value",
-           [](imperative::VarBase &self) {
-             return self.MutableGradVar()->Get<framework::LoDTensor>();
-           },
-           py::return_value_policy::reference)
       .def("clear_gradient", &imperative::VarBase::ClearGradient, R"DOC(
 
         **Notes**:
@@ -437,6 +415,26 @@ void BindImperative(py::module *m_ptr) {
                     loss2.clear_gradient()
                     print("After clear {}".format(loss2.gradient()))
       )DOC")
+      .def("_run_backward",
+           [](imperative::VarBase &self,
+              const imperative::detail::BackwardStrategy &bckst,
+              const imperative::Tracer &tracer) {
+             // TODO(jiabin): when we impl more backward execution we can select
+             // them
+
+             imperative::Engine *engine = tracer.GetDefaultEngine();
+             VLOG(3) << "Start backward";
+             engine->Init(&self, bckst);
+             engine->Execute();
+             VLOG(3) << "Finish backward";
+           },
+           py::call_guard<py::gil_scoped_release>())
+      .def("_grad_name", &imperative::VarBase::GradVarName)
+      .def("_grad_value",
+           [](imperative::VarBase &self) {
+             return self.MutableGradVar()->Get<framework::LoDTensor>();
+           },
+           py::return_value_policy::reference)
       .def("_grad_ivar",
            [](const imperative::VarBase &self) {
              auto &grad_var = self.GradVarBase();
@@ -467,6 +465,11 @@ void BindImperative(py::module *m_ptr) {
            py::return_value_policy::reference)
       .def_property("name", &imperative::VarBase::Name,
                     &imperative::VarBase::SetName)
+      .def_property("stop_gradient",
+                    &imperative::VarBase::OverridedStopGradient,
+                    &imperative::VarBase::SetOverridedStopGradient)
+      .def_property("persistable", &imperative::VarBase::Persistable,
+                    &imperative::VarBase::SetPersistable)
       .def_property_readonly(
           "shape",
           [](imperative::VarBase &self) {
@@ -483,12 +486,7 @@ void BindImperative(py::module *m_ptr) {
             }
           })
       .def_property_readonly("type", &imperative::VarBase::Type)
-      .def_property_readonly("dtype", &imperative::VarBase::DataType)
-      .def_property("persistable", &imperative::VarBase::Persistable,
-                    &imperative::VarBase::SetPersistable)
-      .def_property("stop_gradient",
-                    &imperative::VarBase::OverridedStopGradient,
-                    &imperative::VarBase::SetOverridedStopGradient);
+      .def_property_readonly("dtype", &imperative::VarBase::DataType);
 
   py::class_<imperative::Layer, Layer /* <--- trampoline*/> layer(m, "Layer");
   layer.def(py::init<>())

paddle/fluid/pybind/tensor_py.h

@@ -294,7 +294,7 @@ void _concatCompute(const std::vector<paddle::framework::Tensor> &ins,
   }
 }
 
-void _getSliceinfo(const framework::Tensor &self, py::object obj,
+inline void _getSliceinfo(const framework::Tensor &self, py::object obj,
                           const int64_t dim, int64_t *pstart, int64_t *pstop,
                           int64_t *pstep, int64_t *pslicelength) {
   auto &start = *pstart;

python/paddle/fluid/dygraph/base.py

@@ -172,6 +172,7 @@ def _print_debug_msg(limit=5, is_test=False):
         return unique_name_size, tracer_var_size, alive_cpp_var_size
 
 
+# TODO(zhiqiu): Param 'block' should be deprecated, since block is meaningless in dygraph 
 @framework.dygraph_only
 def to_variable(value, block=None, name=None, zero_copy=None):
     """
@@ -215,10 +216,10 @@ def to_variable(value, block=None, name=None, zero_copy=None):
             zero_copy = False
         py_var = core.VarBase(
             value=value,
-            name=name,
-            persistable=False,
             place=framework._current_expected_place(),
-            zero_copy=zero_copy)
+            persistable=False,
+            zero_copy=zero_copy,
+            name=name if name else '')
         return py_var
     elif isinstance(value, (core.VarBase, framework.Variable)):
         return value

python/paddle/fluid/framework.py

@@ -221,6 +221,23 @@ def _current_expected_place():
     return _dygraph_current_expected_place_
 
 
+# TODO(zhiqiu): remove this function.
+def _var_base_to_np(var_base):
+    """	
+    convert VarBase tp numpy	
+    	
+    Args:	
+        var_base(VarBase) : the VarBase to convert	
+    Returns (np.ndarray): the np.ndarray contain the value of VarBase	
+    """
+
+    warnings.warn(
+        "paddle.fluid.framework._var_base_to_np is deprecated, please use var_base.numpy() instead of _var_base_to_np(var_base)."
+    )
+
+    return var_base.numpy()
+
+
 def _cpu_num():
     if "CPU_NUM" not in os.environ.keys():
         if multiprocessing.cpu_count() > 1:

python/paddle/fluid/layer_helper_base.py

@@ -73,7 +73,7 @@ class LayerHelperBase(object):
             ), "to_variable could only be called in dygraph mode"
             py_var = core.VarBase(
                 value=value,
-                name=name,
+                name=name if name else '',
                 persistable=False,
                 place=_current_expected_place(),
                 zero_copy=False)

python/paddle/fluid/tests/unittests/test_var_base.py

+#   Copyright (c) 2018 PaddlePaddle Authors. All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# 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.
+
+from __future__ import print_function
+
+import unittest
+from paddle.fluid.framework import default_main_program, Program, convert_np_dtype_to_dtype_, in_dygraph_mode
+import paddle.fluid as fluid
+import paddle.fluid.layers as layers
+import paddle.fluid.core as core
+import numpy as np
+
+
+class TestVarBase(unittest.TestCase):
+    def setUp(self):
+        self.shape = [512, 1234]
+        self.dtype = np.float32
+        self.array = np.random.uniform(0.1, 1, self.shape).astype(self.dtype)
+
+    def test_to_variable(self):
+        with fluid.dygraph.guard():
+            var = fluid.dygraph.to_variable(self.array, name="abc")
+            self.assertTrue(np.array_equal(var.numpy(), self.array))
+            self.assertEqual(var.name, 'abc')
+            # default value
+            self.assertEqual(var.persistable, False)
+            self.assertEqual(var.stop_gradient, True)
+            self.assertEqual(var.shape, self.shape)
+            self.assertEqual(var.dtype, core.VarDesc.VarType.FP32)
+            self.assertEqual(var.type, core.VarDesc.VarType.LOD_TENSOR)
+
+    def test_write_property(self):
+        with fluid.dygraph.guard():
+            var = fluid.dygraph.to_variable(self.array)
+
+            self.assertEqual(var.name, 'generated_var_0')
+            var.name = 'test'
+            self.assertEqual(var.name, 'test')
+
+            self.assertEqual(var.persistable, False)
+            var.persistable = True
+            self.assertEqual(var.persistable, True)
+
+            self.assertEqual(var.stop_gradient, True)
+            var.stop_gradient = False
+            self.assertEqual(var.stop_gradient, False)
+
+    # test some patched methods
+    def test_set_value(self):
+        with fluid.dygraph.guard():
+            var = fluid.dygraph.to_variable(self.array)
+            tmp1 = np.random.uniform(0.1, 1, [2, 2, 3]).astype(self.dtype)
+            self.assertRaises(AssertionError, var.set_value, tmp1)
+
+            tmp2 = np.random.uniform(0.1, 1, self.shape).astype(self.dtype)
+            var.set_value(tmp2)
+            self.assertTrue(np.array_equal(var.numpy(), tmp2))
+
+    def test_to_string(self):
+        with fluid.dygraph.guard():
+            var = fluid.dygraph.to_variable(self.array)
+            self.assertTrue(isinstance(str(var.to_string(True)), str))
+
+    def test_backward(self):
+        with fluid.dygraph.guard():
+            var = fluid.dygraph.to_variable(self.array)
+            var.stop_gradient = False
+            loss = fluid.layers.relu(var)
+            loss.backward()
+            grad_var = var._grad_ivar()
+            self.assertEqual(grad_var.shape, self.shape)
+
+    def test_gradient(self):
+        with fluid.dygraph.guard():
+            var = fluid.dygraph.to_variable(self.array)
+            var.stop_gradient = False
+            loss = fluid.layers.relu(var)
+            loss.backward()
+            grad_var = var.gradient()
+            self.assertEqual(grad_var.shape, self.array.shape)
+
+    def test_block(self):
+        with fluid.dygraph.guard():
+            var = fluid.dygraph.to_variable(self.array)
+            self.assertEqual(var.block,
+                             fluid.default_main_program().global_block())
+
+    def test_slice(self):
+        with fluid.dygraph.guard():
+            var = fluid.dygraph.to_variable(self.array)
+            self.assertTrue(np.array_equal(var[1, :].numpy(), self.array[1, :]))
+
+    def test_var_base_to_np(self):
+        with fluid.dygraph.guard():
+            var = fluid.dygraph.to_variable(self.array)
+            self.assertTrue(
+                np.array_equal(var.numpy(),
+                               fluid.framework._var_base_to_np(var)))
+
+
+if __name__ == '__main__':
+    unittest.main()