Remove the updating notice as it gets annoying with multiple pulls.

doc/tutorial/aliasing.txt

 
 .. _basictutaliasing:
 
-===============
-Memory Aliasing
-===============
+=======================================================
+Understanding Memory Aliasing for Speed and Correctness
+=======================================================
 
 The aggressive reuse of memory is one of the ways Theano makes code fast, and
 it's important for the correctness and speed of your program that you understand
@@ -174,6 +174,26 @@ This pattern works regardless of the compute device, and when the compute device
 makes it possible to expose Theano's internal variables without a copy, then it
 goes as fast as an in-place update.
 
+Retrieving and assigning via the .value property
+------------------------------------------------
+
+Shared variables have a ``.value`` property that is connected to ``get_value``
+and ``set_value``.  The borrowing behaviour of the property is controlled by a
+boolean configuration variable ``config.shared.value_borrows``, which currently
+defaults to ``True``.  If that variable is ``True`` then an assignment like ``s.value=v``
+is equivalent to ``s.set_value(v, borrow=True)``, and a retrieval like ``print
+s.value`` is equivalent to ``print s.get_value(borrow=True)``.  Likewise, 
+if ``config.shared.value_borrows`` is ``False``, then the borrow parameter that the ``.value`` property 
+passes to ``set_value`` and ``get_value`` is ``False``.
+
+The ``True`` default value of ``config.shared.value_borrows`` means that
+aliasing can sometimes happen and sometimes not, which can be confusing. 
+Be aware that the default value may be changed to ``False`` sometime in the
+not-to-distant future. This change will create more copies, and potentially slow
+down code that accesses ``.value`` attributes inside tight loops.  To avoid this
+potential impact on your code, use the ``.get_value`` and ``.set_value`` methods
+directly with appropriate flags.
+
 
 Borrowing when constructing Function objects
 ============================================
@@ -207,7 +227,11 @@ The default is of course to *not borrow* internal results.
 
 It is also possible to pass an ``return_internal_type=True`` flag to the ``Out``
 variable which has the same interpretation as the ``return_internal_type`` flag
-to the shared variable's ``get_value`` function.
+to the shared variable's ``get_value`` function.  Unlike ``get_value()``, the
+combination of ``return_internal_type=True`` and ``borrow=True`` arguments to
+``Out()`` are not guaranteed to avoid copying an output value.  They are just
+hints that give more flexibility to the compilation and optimization of the
+graph.
 
 *Take home message:*
 When an input ``x`` to a function is not needed after the function returns and you
@@ -218,7 +242,3 @@ When a return value ``y`` is large (in terms of memory footprint), and you only
 away when it's returned, then consider marking it with an ``Out(y,
 borrow=True)``.
 
-Shared variable .value attribute
-================================
-
-TODO: talk about sharedvar.value and the associated config variable.

doc/tutorial/index.txt

@@ -29,6 +29,7 @@ you out.
     loading_and_saving
     symbolic_graphs
     modes
+    aliasing
     using_gpu
     remarks
     debug_faq

theano/compile/function_module.py

@@ -5,6 +5,7 @@ __docformat__ = "restructuredtext en"
 
 import copy_reg
 import cPickle
+import itertools
 
 import sys, time, copy
 
@@ -537,19 +538,24 @@ class Function(object):
             ## Collect aliased inputs among the storage space
             args_share_memory = []
             for i in xrange(len(self.input_storage)):
-                if isinstance(self.input_storage[i].storage[0],
-                              numpy.ndarray):
+                i_var = self.maker.inputs[i].variable
+                i_val = self.input_storage[i].storage[0]
+                if hasattr( i_var.type, 'may_share_memory'):
                     is_aliased = False
                     for j in xrange(len(args_share_memory)):
-                        for k in args_share_memory[j]:
-                            if numpy.may_share_memory(
-                                    self.input_storage[i].storage[0] ,
-                                    self.input_storage[k].storage[0]):
+
+                        group_j = itertools.izip(
+                            [self.maker.inputs[k].variable for k
+                             in args_share_memory[j]],
+                            [self.input_storage[k].storage[0] for k
+                             in args_share_memory[j]])
+                        if numpy.any([ (var.type is i_var.type and
+                                        var.type.may_share_memory(val,i_val)
+                                       ) for (var,val) in group_j]):
+
                             is_aliased = True
                             args_share_memory[j].append(i)
                             break
-                        if is_aliased:
-                                      break
 
                     if not is_aliased:
                         args_share_memory.append([i])

theano/compile/tests/test_pfunc.py

@@ -517,7 +517,57 @@ class Test_aliasing_rules(unittest.TestCase):
         assert not numpy.may_share_memory(A.get_value(borrow=False), data_of(A))
 
 
-    def test_potential_input_aliasing_affecting_inplace_operations(self):
+    def test_sparse_input_aliasing_affecting_inplace_operations(self):
+        ##
+        ## Note this test will never fail because I am not aware of any
+        ## inplace op on sparse variables
+        try:
+            import scipy.sparse as sp
+        except ImportError:
+            pass#the variable enable_sparse will be used to disable the test file.
+
+        from theano.sparse import enable_sparse
+        if enable_sparse == False:
+            raise SkipTest('Optional package sparse disabled')
+
+        from theano import sparse
+
+        ## Note: to trigger this bug with theano rev 4586:2bc6fc7f218b,
+        #        you need to make in inputs mutable ( so that inplace
+        #        operations are used) and to break the elemwise composition
+        #        with some non-elemwise op ( here dot )
+
+        x  = sparse.SparseType('csc', dtype = 'float64')()
+        y  = sparse.SparseType('csc', dtype = 'float64')()
+        f = theano.function( [theano.In(x,  mutable = True),
+                              theano.In(y, mutable = True)],
+                                (x+y)+(x+y))
+        ## Test 1. If the same variable is given twice
+
+        # Compute bogus values
+        m = sp.csc_matrix(numpy.asarray([[1,0,0,0,0],
+                           [0,1,0,0,0],
+                           [0,0,1,0,0],
+                           [0,0,0,1,0],
+                           [0,0,0,0,1]], dtype = 'float64'))
+        bogus_vals =  f(m,m)
+        # Since we used inplace operation v and m may be corrupted
+        # so we need to recreate them
+
+        m = sp.csc_matrix(numpy.asarray([[1,0,0,0,0],
+                           [0,1,0,0,0],
+                           [0,0,1,0,0],
+                           [0,0,0,1,0],
+                           [0,0,0,0,1]], dtype = 'float64'))
+        m_copy = m.copy()
+        vals =  f(m,m_copy)
+
+        assert numpy.allclose(vals.todense(), bogus_vals.todense())
+
+
+
+
+    def test_input_aliasing_affecting_inplace_operations(self):
 
         ## Note: to trigger this bug with theano rev 4586:2bc6fc7f218b,
         #        you need to make in inputs mutable ( so that inplace
@@ -532,20 +582,79 @@ class Test_aliasing_rules(unittest.TestCase):
                               theano.In(m1, mutable = True),
                               theano.In(m2, mutable = True)],
                             theano.dot(x*2,m1)+theano.dot(y*3,m2))
+        ## Test 1. If the same variable is given twice
+
         # Compute bogus values
-        v = numpy.asarray([1,2], dtype = 'float64')
-        m = numpy.asarray([[1,0],[0,1]], dtype = 'float64')
+        v = numpy.asarray( [1,2,3,4,5], dtype = 'float64')
+        m = numpy.asarray([[1,0,0,0,0],
+                           [0,1,0,0,0],
+                           [0,0,1,0,0],
+                           [0,0,0,1,0],
+                           [0,0,0,0,1]], dtype = 'float64')
         bogus_vals =  f(v,v,m,m)
         # Since we used inplace operation v and m may be corrupted
         # so we need to recreate them
-        m = numpy.asarray([[1,0],[0,1]], dtype = 'float64')
-        v = numpy.asarray([1,2], dtype = 'float64')
+
+        v = numpy.asarray( [1,2,3,4,5], dtype = 'float64')
+        m = numpy.asarray([[1,0,0,0,0],
+                           [0,1,0,0,0],
+                           [0,0,1,0,0],
+                           [0,0,0,1,0],
+                           [0,0,0,0,1]], dtype = 'float64')
         m_copy = m.copy()
         v_copy = v.copy()
         vals =  f(v,v_copy,m,m_copy)
 
         assert numpy.allclose(vals, bogus_vals)
 
+    def test_partial_input_aliasing_affecting_inplace_operations(self):
+
+        ## Note: to trigger this bug with theano rev 4586:2bc6fc7f218b,
+        #        you need to make in inputs mutable ( so that inplace
+        #        operations are used) and to break the elemwise composition
+        #        with some non-elemwise op ( here dot )
+        x  = theano.tensor.dvector()
+        y  = theano.tensor.dvector()
+        z  = theano.tensor.dvector()
+        m1 = theano.tensor.dmatrix()
+        m2 = theano.tensor.dmatrix()
+        m3 = theano.tensor.dmatrix()
+
+        ## Test 2. If variables only partial overlap
+        #   more exactly we care about the case when we have a,b,c
+        #   and a shares memory with b, b shares memory with c, but
+        #   c does not share memory with a
+
+
+
+        f = theano.function( [theano.In(x,  mutable = True),
+                              theano.In(y,  mutable = True),
+                              theano.In(z,  mutable = True),
+                              theano.In(m1, mutable = True),
+                              theano.In(m2, mutable = True),
+                              theano.In(m3, mutable = True)],
+                            theano.dot(x*2,m1)+theano.dot(y*3,m2)+theano.dot(z*4,m3))
+        # Compute bogus values
+        v = numpy.asarray( [1,2,3,4,5], dtype = 'float64')
+        m = numpy.asarray([[1,0],
+                           [0,1]], dtype = 'float64')
+        bogus_vals =  f(v[:2],v[1:3],v[2:4],m,m,m)
+        # Since we used inplace operation v and m may be corrupted
+        # so we need to recreate them
+
+        v = numpy.asarray( [1,2,3,4,5], dtype = 'float64')
+        m = numpy.asarray([[1,0],
+                           [0,1]], dtype = 'float64')
+        m_copy1 = m.copy()
+        v_copy1 = v.copy()
+        m_copy2 = m.copy()
+        v_copy2 = v.copy()
+        vals =  f(v[:2],v_copy1[1:3],v_copy2[2:4],m,m_copy1, m_copy2)
+
+        assert numpy.allclose(vals, bogus_vals)
+
+
+
     def test_potential_output_aliasing_induced_by_updates(self):
 
         A = self.shared(numpy.zeros((2,2)))

theano/misc/hg_version_hook.sh

@@ -2,6 +2,5 @@
 # Script to update version.py in response to Mercurial hooks. This should
 # not appear in a release tarball.
 
-echo "Updating version.py..."
 sed -e "s/^hg_revision.*/hg_revision = '`expr substr $HG_NODE 1 12`'/" theano/version.py >theano/version.py.out && mv theano/version.py.out theano/version.py
 

theano/sandbox/cuda/tests/test_basic_ops.py

@@ -803,7 +803,7 @@ def test_duplicate_arg_elemwise():
 
 
 import theano.tensor.tests.test_basic
-test_shared_options = theano.tensor.tests.test_basic.build_test_shared_options(tcn.shared_constructor, 'float32', False, False)
+test_shared_options = theano.tensor.tests.test_basic.makeSharedTester(tcn.shared_constructor, 'float32', False, False, False, cuda_ndarray.CudaNdarray, theano.tensor.exp, numpy.exp)
 
 if __name__ == '__main__':
     test_many_arg_elemwise()

theano/sandbox/cuda/var.py

+import copy
+
 import numpy
 
 import theano
-from theano import Op, Type, Apply, Variable, Constant
+from theano import Variable, Constant
 from theano import tensor
-from theano.compile import shared, SharedVariable
+from theano.compile import SharedVariable
 
 from theano.sandbox.cuda.type import CudaNdarrayType
 from theano.sandbox.cuda import filter as type_support_filter

theano/sparse/basic.py

@@ -99,6 +99,7 @@ def as_sparse_variable(x, name=None):
     except TypeError:
         raise TypeError("Cannot convert %s to SparseType" % x, type(x))
 
+
 as_sparse = as_sparse_variable
 
 def constant(x, name=None):
@@ -147,7 +148,6 @@ class SparseType(gof.Type):
         @param format:  The sparse storage strategy.
         @return         An empty SparseVariable instance.
         """
-
         dtype = str(dtype)
         if dtype in self.dtype_set:
             self.dtype = dtype
@@ -174,6 +174,12 @@ class SparseType(gof.Type):
             raise NotImplementedError()
         return sp
 
+    @staticmethod
+    def may_share_memory(a,b):
+        # This is Fred suggestion for a quick and dirty way of checking
+        # aliasing .. this can potentially be further refined (ticket #374)
+        return a is b
+
     def make_variable(self, name = None):
         return SparseVariable(self, name = name)
 

theano/tensor/basic.py

@@ -472,6 +472,10 @@ class TensorType(Type):
         return type(self) == type(other) and other.dtype == self.dtype \
             and other.broadcastable == self.broadcastable
 
+    @staticmethod
+    def may_share_memory(a,b):
+        return numpy.may_share_memory(a,b)
+
     @staticmethod
     def values_eq(a, b):
         #TODO: check to see if the dtype and shapes must match

theano/tensor/tests/test_basic.py

@@ -3378,10 +3378,14 @@ def test_dimshuffle_duplicate():
 
     assert success
 
-def build_test_shared_options(shared_constructor_,
+def makeSharedTester(shared_constructor_,
                               dtype_,
                               get_value_borrow_true_alias_,
-                              shared_borrow_true_alias_):
+                              shared_borrow_true_alias_,
+                              set_value_borrow_true_alias_,
+                              internal_type_,
+                              theano_fct_,
+                              ref_fct_):
     """
     This is a generic fct to allow reusing the same test function
     for many shared variable of many types.
@@ -3391,6 +3395,10 @@ def build_test_shared_options(shared_constructor_,
         dtype = dtype_
         get_value_borrow_true_alias = get_value_borrow_true_alias_
         shared_borrow_true_alias = shared_borrow_true_alias_
+        internal_type = internal_type_
+        theano_fct = staticmethod(theano_fct_)
+        ref_fct = staticmethod(ref_fct_)
+        set_value_borrow_true_alias = set_value_borrow_true_alias_
 
         def test_shared_dont_alias(self):
             dtype = self.dtype
@@ -3399,22 +3407,22 @@ def build_test_shared_options(shared_constructor_,
 
             rng = numpy.random.RandomState([3,5,17])
             x = numpy.asarray(rng.uniform(0,1,[2,4]),dtype=dtype)
-            x_sum = x.sum()
+            x_ref = self.ref_fct(x)
             x_shared = self.shared_constructor(x, borrow = False)
-            total = theano.tensor.sum(x_shared)
+            total = self.theano_fct(x_shared)
 
             total_func = theano.function([],total)
 
             total_val = total_func()
 
-            assert numpy.allclose(x.sum(), total_val)
+            assert numpy.allclose(self.ref_fct(x), total_val)
 
             x += 1
 
             total_val_2 = total_func()
 
             #value used to construct should not alias with internal
-            assert total_val == total_val_2
+            assert numpy.allclose(total_val, total_val_2)
 
             x = x_shared.get_value(borrow = False)
 
@@ -3423,7 +3431,7 @@ def build_test_shared_options(shared_constructor_,
             total_val_3 = total_func()
 
             #value returned by access should not alias with internal
-            assert total_val == total_val_3
+            assert numpy.allclose(total_val, total_val_3)
 
             #in this case we can alias
             x = x_shared.get_value(borrow = True)
@@ -3432,10 +3440,89 @@ def build_test_shared_options(shared_constructor_,
             #this is not required by the contract but it is a feature we've 
             #implemented for some type of SharedVariable.
             if self.get_value_borrow_true_alias:
-                assert numpy.allclose(x.sum(), total_func())
+                assert numpy.allclose(self.ref_fct(x), total_func())
             else:
-                assert numpy.allclose(x_sum, total_func())
+                assert numpy.allclose(x_ref, total_func())
 
+
+        def test_return_internal_type(self):
+            dtype = self.dtype
+            if dtype is None:
+                dtype = theano.config.floatX
+
+            rng = numpy.random.RandomState([3,5,17])
+            x = numpy.asarray(rng.uniform(0,1,[2,4]),dtype=dtype)
+            x_ref = self.ref_fct(x)
+            x_shared = self.shared_constructor(x, borrow = False)
+            total = self.theano_fct(x_shared)
+
+            total_func = theano.function([],total)
+
+            #in this case we can alias with the internal value
+            x = x_shared.get_value(borrow = True, return_internal_type = True)
+            assert isinstance(x, self.internal_type)
+            
+            values_to_add = numpy.ones(x.shape,dtype=dtype)
+            if not isinstance(values_to_add, self.internal_type):
+                values_to_add = self.internal_type(values_to_add)#supported for cudandarray, but not ndarray.
+            x += values_to_add#supported by ndarray and CudaNdarray
+
+            #this is not required by the contract but it is a feature we can 
+            #implement for some type of SharedVariable.
+            assert numpy.allclose(self.ref_fct(x), total_func())
+            
+            x = x_shared.get_value(borrow = False, return_internal_type = True)
+            assert isinstance(x, self.internal_type)
+            
+            x += values_to_add#supported by ndarray and CudaNdarray
+
+            #this is required by the contract
+            assert not numpy.allclose(self.ref_fct(x), total_func())
+
+        def test_set_value(self):
+            dtype = self.dtype
+            if dtype is None:
+                dtype = theano.config.floatX
+
+            rng = numpy.random.RandomState([3,5,17])
+            x = numpy.asarray(rng.uniform(0,1,[2,4]),dtype=dtype)
+            x_orig = x
+            x_orig_copy = x.copy()
+            x_ref = self.ref_fct(x)
+            x_shared = self.shared_constructor(x, borrow = False)
+            total = self.theano_fct(x_shared)
+
+            total_func = theano.function([],total)
+
+            #test if that theano shared variable optimize set_value(borrow=True)
+            get_x = x_shared.get_value(borrow=True)
+            assert get_x is not x_orig#borrow=False to shared_constructor
+            get_x +=1
+            x_shared.set_value(get_x, borrow=True)
+            x = x_shared.get_value(borrow=True)
+            if self.set_value_borrow_true_alias:
+                assert x is get_x
+            else:
+                assert x is not get_x
+            assert numpy.allclose(self.ref_fct(x_orig+1),self.ref_fct(x))
+
+            #test optimized get set value on the gpu(don't pass data to the cpu)
+            get_x = x_shared.get_value(borrow=True, return_internal_type=True)
+            assert get_x is not x_orig#borrow=False to shared_constructor
+            assert isinstance(get_x, self.internal_type)
+            values_to_add = numpy.ones(x.shape,dtype=dtype)
+            if not isinstance(values_to_add, self.internal_type):
+                values_to_add = self.internal_type(values_to_add)#supported for cudandarray, but not ndarray.
+            assert isinstance(values_to_add, self.internal_type)
+
+            get_x += values_to_add#supported by ndarray and CudaNdarray
+            assert isinstance(get_x, self.internal_type)
+            x_shared.set_value(get_x, borrow=True)
+            x = x_shared.get_value(borrow=True, return_internal_type=True)
+            assert isinstance(x, self.internal_type)
+            assert x is get_x
+
+            ################ TODO test Out.
         def test_shared_do_alias(self):
             dtype = self.dtype
             if dtype is None:
@@ -3443,29 +3530,31 @@ def build_test_shared_options(shared_constructor_,
 
             rng = numpy.random.RandomState([2,4,16])
             x = numpy.asarray(rng.uniform(1,2,[4,2]),dtype=dtype)
-            x_sum = x.sum()
+            x_ref = self.ref_fct(x)
 
             x_shared = self.shared_constructor(x, borrow = True)
 
-            total = theano.tensor.sum(x_shared)
+            total = self.theano_fct(x_shared)
 
             total_func = theano.function([],total)
 
             total_val = total_func()
 
-            assert numpy.allclose(x.sum(), total_val)
+            assert numpy.allclose(self.ref_fct(x), total_val)
 
             x += 1
 
             #not required by the contract but it is a feature we've implemented
             if self.shared_borrow_true_alias:
-                assert numpy.allclose(x.sum(), total_func())
+                assert numpy.allclose(self.ref_fct(x), total_func())
             else:
-                assert numpy.allclose(x_sum, total_func())
+                assert numpy.allclose(x_ref, total_func())
 
     return SharedTester
 
-test_shared_options=build_test_shared_options(tensor.shared, 'float64', True, True)
+test_shared_options=makeSharedTester(tensor.shared, 'float64',
+                                              True, True, True, numpy.ndarray,
+                                              theano.tensor.sum, numpy.sum)
 
 
 if __name__ == '__main__':