Skip to content

Commit

Permalink
Merge pull request #2992 from nouiz/subtensor_list
Browse files Browse the repository at this point in the history
[CRASH] Subtensor list
  • Loading branch information
lamblin committed Jun 4, 2015
2 parents da3c4cd + b172a19 commit 944e36d
Show file tree
Hide file tree
Showing 7 changed files with 33 additions and 10 deletions.
2 changes: 1 addition & 1 deletion theano/compile/function_module.py
Original file line number Diff line number Diff line change
Expand Up @@ -640,7 +640,7 @@ def __call__(self, *args, **kwargs):
if c.required:
c.storage[0] = None

# if we are allowing garbage collection, remove the input and
# if we are allowing garbage collection, remove the
# output reference from the internal storage cells
if getattr(self.fn, 'allow_gc', False):
assert len(self.output_storage) == len(self.maker.fgraph.outputs)
Expand Down
7 changes: 2 additions & 5 deletions theano/gof/destroyhandler.py
Original file line number Diff line number Diff line change
Expand Up @@ -291,8 +291,6 @@ def on_attach(self, fgraph):

####### Do the checking ###########
already_there = False
if self.fgraph is fgraph:
already_there = True
if self.fgraph not in [None, fgraph]:
raise Exception("A DestroyHandler instance can only serve"
" one FunctionGraph. (Matthew 6:24)")
Expand Down Expand Up @@ -796,12 +794,11 @@ def on_import(self, fgraph, app, reason):
# print 'DH IMPORT', app, id(app), id(self), len(self.debug_all_apps)

# If it's a destructive op, add it to our watch list
if getattr(app.op, 'destroy_map', OrderedDict()):
if getattr(app.op, 'destroy_map', {}):
self.destroyers.add(app)

# add this symbol to the forward and backward maps
for o_idx, i_idx_list in getattr(app.op, 'view_map',
OrderedDict()).items():
for o_idx, i_idx_list in getattr(app.op, 'view_map', {}).items():
if len(i_idx_list) > 1:
raise NotImplementedError(
'destroying this output invalidates multiple inputs',
Expand Down
6 changes: 6 additions & 0 deletions theano/gof/link.py
Original file line number Diff line number Diff line change
Expand Up @@ -624,6 +624,12 @@ def gc_helper(node_list):
dictionary that maps each Variable instance to a the last node to use Variable as an input.
This is used to allow garbage collection within graphs.
It ignore view_map and destroy_map. This isn't needed as python
have referecence count. In Theano gc, we should not take into
account view_map and destroy_map as if the thunk decided to create
a new output, we would delay uselessly its gc by Python.
"""
# for freeing memory
last_user = {}
Expand Down
13 changes: 11 additions & 2 deletions theano/gof/vm.py
Original file line number Diff line number Diff line change
Expand Up @@ -378,8 +378,8 @@ def __init__(self, nodes, thunks, pre_call_clear,
# destroy_dependencies
# --------------------
# The destroy_dependencies is a list of variables that are implicit
# dependencies induced by a destroy_map (compare node.inputs which
# are *explicit* dependencies). The variables in
# dependencies induced by destroy_map and view_map (compared to
# node.inputs which are *explicit* dependencies). The variables in
# destroy_dependencies would be impossible to compute after the
# current `node` runs, because node.thunk() is going to destroy a
# common input variable needed by whatever node owns each variable
Expand Down Expand Up @@ -787,6 +787,12 @@ def compute_gc_dependencies(self, variables):
N.B. gc means garbage collection
Note
----
It don't take care of the view_map/destroy_map. So
it mean it rely on Python gc to don't free the object real
storage.
"""
dependencies = {}
for k in variables:
Expand All @@ -796,6 +802,9 @@ def compute_gc_dependencies(self, variables):
# way of getting it back.
#
# XXX if k has no clients... what is it doing in the computation?
# Fred guess: it could happen for node with multiple outputs when
# we don't use all outputs.

if k.owner and k.clients:
ls = []
for cl in k.clients:
Expand Down
4 changes: 3 additions & 1 deletion theano/tensor/blas_c.py
Original file line number Diff line number Diff line change
Expand Up @@ -730,7 +730,9 @@ def check_force_gemv_init():
f = theano.function(
[aa, yy, xx],
gemv_no_inplace(aa, 1., xx, yy, 0.),
theano.compile.Mode(optimizer='fast_compile')
theano.compile.Mode(optimizer='fast_compile').excluding('gpu',
'gpuarray'),
profile=False
)
finally:
theano.config.compute_test_value = tv
Expand Down
6 changes: 6 additions & 0 deletions theano/tensor/tests/test_subtensor.py
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,12 @@ def test_long_too_big(self):
n = self.shared(numpy.arange(12, dtype=self.dtype).reshape((4, 3)))
self.assertRaises(Exception, lambda: n[:(2L ** 63)])

def test_list_slice(self):
x = theano.tensor.arange(100).reshape((5, 5, 4))
res = x[[slice(1, -1)] * x.ndim].eval()
x = numpy.arange(100).reshape((5, 5, 4))
numpy.allclose(res, x[[slice(1, -1)] * x.ndim])

def test_newaxis(self):
"""
newaxis support comes from logic in the __getitem__ of TensorType
Expand Down
5 changes: 4 additions & 1 deletion theano/tensor/var.py
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,10 @@ def astype(self, dtype):

# SLICING/INDEXING
def __getitem__(self, args):
if not isinstance(args, tuple):
if (isinstance(args, list) and
any([isinstance(a, slice) for a in args])):
pass
elif not isinstance(args, tuple):
args = args,
# Convert python literals to theano constants
args = theano.tensor.subtensor.make_constant(args)
Expand Down

0 comments on commit 944e36d

Please sign in to comment.