Undo a breaking change to the public API; while retaining the bugfix of #8504.

As it turns out, the intent of constructing tf.layers._Layer with reuse=True
is to reuse the subscope containing the layer's name.  This was broken before the recent change fixing #8504, but that change didn't actually fix the problem (it introduced a different bug!).  Here we finish fixing #8504.

The following will now work correctly:

a = Dense(3, name="a")
a.apply(...)  # scope of a is "a"

a_reuse = Dense(3, name="a", _reuse=True)
a_reuse.apply(...)  # scope of a_reuse is "a"

the errant behavior was that a_reuse scope became just "".

The alternative behavior can still be recreated via:

a_reuse = Dense(..., _reuse=True)
a_reuse.apply(..., scope=tf.get_variable_scope())  # scope of a_reuse is "".
Change: 150947095

2 files changed, 34 insertions, 13 deletions

diff --git a/tensorflow/python/layers/base.py b/tensorflow/python/layers/base.py
index f5a7fbacdb..3c67284f7f 100644
--- a/tensorflow/python/layers/base.py
+++ b/tensorflow/python/layers/base.py
@@ -280,7 +280,7 @@ class _Layer(object):
     Returns:
       Output tensor(s).
     """
-    scope = kwargs.get('scope', None)
+    scope = kwargs.pop('scope', None)
 
     # Define a custom getter to override tf.get_variable when creating layer
     # variables. The current custom getter is nested by the variable scope.
@@ -292,14 +292,11 @@ class _Layer(object):
           variable_getter=functools.partial(getter, **getter_kwargs))
 
     if not self._built and self._scope is None:
-      # If constructed with _scope=None
+      # If constructed with _scope=None, lazy setting of scope.
       if self._reuse:
-        # If reuse=True, use the scope argument (if any), or the current
-        # variable scope.
-        self._scope = scope or vs.get_variable_scope()
+        self._scope = next(vs.variable_scope(
+            scope if scope is not None else self._base_name).gen)
       else:
-        # Otherwise, if reuse=False, create a new scope now.  We may
-        # end up using the scope passed in via the scope argument.
         self._scope = next(vs.variable_scope(
             scope, default_name=self._base_name).gen)
       self._name = self._scope.name
diff --git a/tensorflow/python/layers/base_test.py b/tensorflow/python/layers/base_test.py
index d1909b4147..c82ec47671 100644
--- a/tensorflow/python/layers/base_test.py
+++ b/tensorflow/python/layers/base_test.py
@@ -100,21 +100,45 @@ class BaseLayerTest(test.TestCase):
       self.assertListEqual([v.name for v in layer.variables],
                            ['my_layer/my_var:0', 'my_layer/my_call_var:0'])
 
-      # Creating a layer with _reuse=True and _scope=None delays
-      # selecting the variable scope until call.
-      lazy_layer = MyLayer(name='lazy_layer', _reuse=True)
+      # Creating a layer with no scope leads to lazy construction of
+      # the scope at apply() time.  It uses scope "<current scope>/base_name"
+      lazy_layer = MyLayer(_reuse=True)
       with variable_scope.variable_scope('new_scope'):
         # This should attempt to reuse 'my_var' and 'my_call_var' in 'new_scope'
         with self.assertRaisesRegexp(
-            ValueError, r'new_scope/my_var does not exist'):
+            ValueError, r'new_scope/my_layer/my_var does not exist'):
+          lazy_layer.apply(inputs)
+        with variable_scope.variable_scope('my_layer'):
+          variable_scope.get_variable('my_var', [2, 2])
+        with self.assertRaisesRegexp(
+            ValueError, r'new_scope/my_layer/my_call_var does not exist'):
           lazy_layer.apply(inputs)
+        with variable_scope.variable_scope('my_layer'):
+          variable_scope.get_variable('my_call_var', [2, 2])
+        # Smoke test: it runs.
+        lazy_layer.apply(inputs)
+        # The variables were created outside of the Layer, and
+        # reuse=True, so the Layer does not own them and they are not
+        # stored in its collection.
+        self.assertListEqual(lazy_layer.variables, [])
+        self.assertEqual(lazy_layer.name, 'new_scope/my_layer')
+
+      # Creating a layer with no scope leads to lazy construction of
+      # the scope at apply() time.  If 'scope' argument is passed to
+      # apply(), it uses that scope when accessing variables.
+      lazy_layer = MyLayer(_reuse=True)
+      with variable_scope.variable_scope('new_scope') as new_scope:
+        # This should attempt to reuse 'my_var' and 'my_call_var' in 'new_scope'
+        with self.assertRaisesRegexp(
+            ValueError, r'new_scope/my_var does not exist'):
+          lazy_layer.apply(inputs, scope=new_scope)
         variable_scope.get_variable('my_var', [2, 2])
         with self.assertRaisesRegexp(
             ValueError, r'new_scope/my_call_var does not exist'):
-          lazy_layer.apply(inputs)
+          lazy_layer.apply(inputs, scope=new_scope)
         variable_scope.get_variable('my_call_var', [2, 2])
         # Smoke test: it runs.
-        lazy_layer.apply(inputs)
+        lazy_layer.apply(inputs, scope=new_scope)
         # The variables were created outside of the Layer, and
         # reuse=True, so the Layer does not own them and they are not
         # stored in its collection.