diff --git a/aesara/compile/compilelock.py b/aesara/compile/compilelock.py index 1c2e7ba7d4..5a441e3163 100644 --- a/aesara/compile/compilelock.py +++ b/aesara/compile/compilelock.py @@ -4,8 +4,8 @@ """ import os import threading -import typing from contextlib import contextmanager +from typing import Optional import filelock @@ -45,7 +45,7 @@ def force_unlock(lock_dir: os.PathLike): @contextmanager -def lock_ctx(lock_dir: os.PathLike = None, *, timeout: typing.Optional[float] = -1): +def lock_ctx(lock_dir: os.PathLike = None, *, timeout: Optional[float] = None): """Context manager that wraps around FileLock and SoftFileLock from filelock package. Parameters @@ -59,10 +59,9 @@ def lock_ctx(lock_dir: os.PathLike = None, *, timeout: typing.Optional[float] = """ if lock_dir is None: lock_dir = config.compiledir - if timeout == -1: + + if timeout is None: timeout = config.compile__timeout - elif not (timeout is None or timeout > 0): - raise ValueError(f"Timeout parameter must be None or positive. Got {timeout}.") # locks are kept in a dictionary to account for changing compiledirs dir_key = f"{lock_dir}-{os.getpid()}" diff --git a/aesara/link/c/basic.py b/aesara/link/c/basic.py index 87fe0354a8..cbe726cf01 100644 --- a/aesara/link/c/basic.py +++ b/aesara/link/c/basic.py @@ -1771,6 +1771,9 @@ def __call__(self): raise raise exc_value.with_traceback(exc_trace) + def __str__(self): + return f"{type(self).__name__}({self.module})" + class OpWiseCLinker(LocalLinker): """ diff --git a/aesara/link/c/cmodule.py b/aesara/link/c/cmodule.py index deb1670a13..2001766c03 100644 --- a/aesara/link/c/cmodule.py +++ b/aesara/link/c/cmodule.py @@ -28,7 +28,7 @@ # we will abuse the lockfile mechanism when reading and writing the registry from aesara.compile.compilelock import lock_ctx from aesara.configdefaults import config, gcc_version_str -from aesara.link.c.exceptions import MissingGXX +from aesara.link.c.exceptions import CompileError, MissingGXX from aesara.utils import ( LOCAL_BITWIDTH, flatten, @@ -2543,9 +2543,9 @@ def print_command_line_error(): # We replace '\n' by '. ' in the error message because when Python # prints the exception, having '\n' in the text makes it more # difficult to read. - compile_stderr = compile_stderr.replace("\n", ". ") - raise Exception( - f"Compilation failed (return status={status}): {compile_stderr}" + # compile_stderr = compile_stderr.replace("\n", ". ") + raise CompileError( + f"Compilation failed (return status={status}):\n{' '.join(cmd)}\n{compile_stderr}" ) elif config.cmodule__compilation_warning and compile_stderr: # Print errors just below the command line. diff --git a/aesara/link/c/exceptions.py b/aesara/link/c/exceptions.py index 3776ed8859..c5dcf3294c 100644 --- a/aesara/link/c/exceptions.py +++ b/aesara/link/c/exceptions.py @@ -1,6 +1,18 @@ +from distutils.errors import CompileError as BaseCompileError + + class MissingGXX(Exception): """ This error is raised when we try to generate c code, but g++ is not available. """ + + +class CompileError(BaseCompileError): + """This custom `Exception` prints compilation errors with their original + formatting. + """ + + def __str__(self): + return self.args[0] diff --git a/aesara/link/utils.py b/aesara/link/utils.py index afe76f83a5..4d13247548 100644 --- a/aesara/link/utils.py +++ b/aesara/link/utils.py @@ -330,9 +330,9 @@ def raise_with_op( else: scalar_values.append("not shown") else: - shapes = "The thunk don't have an inputs attributes." - strides = "So we can't access the strides of inputs values" - scalar_values = "And can't print its inputs scalar value" + shapes = "The thunk doesn't have an `inputs` attributes." + strides = "So we can't access the strides of the input values" + scalar_values = "and we can't print its scalar input values" clients = [[c[0] for c in fgraph.clients[var]] for var in node.outputs] detailed_err_msg += ( f"Inputs shapes: {shapes}" @@ -349,14 +349,15 @@ def raise_with_op( detailed_err_msg += f"\nOutputs clients: {clients}\n" else: hints.append( - "HINT: Use another linker then the c linker to" - " have the inputs shapes and strides printed." + "HINT: Use a linker other than the C linker to" + " print the inputs' shapes and strides." ) # Print node backtraces tr = getattr(node.outputs[0].tag, "trace", []) if isinstance(tr, list) and len(tr) > 0: - detailed_err_msg += "\nBacktrace when the node is created(use Aesara flag traceback__limit=N to make it longer):\n" + detailed_err_msg += "\nBacktrace when the node is created " + detailed_err_msg += "(use Aesara flag traceback__limit=N to make it longer):\n" # Print separate message for each element in the list of batcktraces sio = io.StringIO() @@ -365,9 +366,9 @@ def raise_with_op( detailed_err_msg += str(sio.getvalue()) else: hints.append( - "HINT: Re-running with most Aesara optimization disabled could" - " give you a back-trace of when this node was created. This can" - " be done with by setting the Aesara flag" + "HINT: Re-running with most Aesara optimizations disabled could" + " provide a back-trace showing when this node was created. This can" + " be done by setting the Aesara flag" " 'optimizer=fast_compile'. If that does not work," " Aesara optimizations can be disabled with 'optimizer=None'." ) @@ -378,7 +379,7 @@ def raise_with_op( f = io.StringIO() aesara.printing.debugprint(node, file=f, stop_on_name=True, print_type=True) - detailed_err_msg += "\nDebugprint of the apply node: \n" + detailed_err_msg += "\nDebug print of the apply node: \n" detailed_err_msg += f.getvalue() # Prints output_map @@ -497,8 +498,8 @@ def raise_with_op( else: hints.append( - "HINT: Use the Aesara flag 'exception_verbosity=high'" - " for a debugprint and storage map footprint of this apply node." + "HINT: Use the Aesara flag `exception_verbosity=high`" + " for a debug print-out and storage map footprint of this Apply node." ) try: @@ -506,7 +507,9 @@ def raise_with_op( str(exc_value) + detailed_err_msg + "\n" + "\n".join(hints) ) except TypeError: - warnings.warn(f"{exc_type} error does not allow us to add extra error message") + warnings.warn( + f"{exc_type} error does not allow us to add an extra error message" + ) # Some exception need extra parameter in inputs. So forget the # extra long error message in that case. raise exc_value.with_traceback(exc_trace) @@ -541,7 +544,7 @@ def write(msg): write(line) write( "For the full definition stack trace set" - " the Aesara flags traceback__limit to -1" + " the Aesara flags `traceback__limit` to -1" ) diff --git a/aesara/tensor/elemwise.py b/aesara/tensor/elemwise.py index b0607238bf..b48462d2e4 100644 --- a/aesara/tensor/elemwise.py +++ b/aesara/tensor/elemwise.py @@ -409,7 +409,7 @@ def __setstate__(self, d): def get_output_info(self, dim_shuffle, *inputs): """Return the outputs dtype and broadcastable pattern and the - dimshuffled niputs. + dimshuffled inputs. """ shadow = self.scalar_op.make_node( @@ -736,30 +736,9 @@ def perform(self, node, inputs, output_storage): # should be disabled. super().perform(node, inputs, output_storage) - for dims in zip( - *[ - list(zip(input.shape, sinput.type.broadcastable)) - for input, sinput in zip(inputs, node.inputs) - ] - ): - if max(d for d, b in dims) != 1 and (1, False) in dims: - # yes there may be more compact ways to write this code, - # but please maintain python 2.4 compatibility - # (no "x if c else y") - msg = [] - assert len(inputs) == len(node.inputs) - for input, sinput in zip(inputs, node.inputs): - assert len(input.shape) == len(sinput.type.broadcastable) - msg2 = [] - for d, b in zip(input.shape, sinput.type.broadcastable): - if b: - msg2 += ["*"] - else: - msg2 += [str(d)] - msg.append(f"({', '.join(msg2)})") - - base_exc_str = f"Dimension mismatch; shapes are {', '.join(msg)}" - raise ValueError(base_exc_str) + for d, dim_shapes in enumerate(zip(*(i.shape for i in inputs))): + if len(set(dim_shapes) - {1}) > 1: + raise ValueError(f"Shapes on dimension {d} do not match: {dim_shapes}") # Determine the shape of outputs out_shape = [] @@ -878,9 +857,9 @@ def infer_shape(self, fgraph, node, i_shapes): return rval def _c_all(self, node, nodename, inames, onames, sub): - # Some ops call directly the Elemwise._c_all or Elemwise.c_code + # Some `Op`s directly call `Elemwise._c_all` or `Elemwise.c_code` # To not request all of them to call prepare_node(), do it here. - # There is no harm if it get called multile time. + # There is no harm if it get called multiple times. if not hasattr(node.tag, "fake_node"): self.prepare_node(node, None, None, "c") _inames = inames diff --git a/aesara/tensor/random/basic.py b/aesara/tensor/random/basic.py index 2e25876394..b0ae170e79 100644 --- a/aesara/tensor/random/basic.py +++ b/aesara/tensor/random/basic.py @@ -378,7 +378,7 @@ def _shape_from_params(self, dist_params, rep_param_idx=1, param_shapes=None): multinomial = MultinomialRV() -vsearchsorted = np.vectorize(np.searchsorted, otypes=[np.int], signature="(n),()->()") +vsearchsorted = np.vectorize(np.searchsorted, otypes=[int], signature="(n),()->()") class CategoricalRV(RandomVariable): diff --git a/tests/compile/test_compilelock.py b/tests/compile/test_compilelock.py index bc0a684b02..cb559a137f 100644 --- a/tests/compile/test_compilelock.py +++ b/tests/compile/test_compilelock.py @@ -11,16 +11,6 @@ from aesara.compile.compilelock import force_unlock, local_mem, lock_ctx -def test_compilelock_errors(): - with tempfile.TemporaryDirectory() as dir: - with pytest.raises(ValueError): - with lock_ctx(dir, timeout=0): - pass - with pytest.raises(ValueError): - with lock_ctx(dir, timeout=-2): - pass - - def test_compilelock_force_unlock(): with tempfile.TemporaryDirectory() as dir_name: with lock_ctx(dir_name): @@ -81,13 +71,22 @@ def run_locking_test(ctx): def test_locking_thread(): + import traceback with tempfile.TemporaryDirectory() as dir_name: - def test_fn_1(): - with lock_ctx(dir_name): - # Sleep "indefinitely" - time.sleep(100) + def test_fn_1(arg): + try: + with lock_ctx(dir_name): + # Notify the outside that we've obtained the lock + arg.append(False) + while True not in arg: + time.sleep(0.5) + except Exception: + # Notify the outside that we done + arg.append(False) + # If something unexpected happened, we want to know what it was + traceback.print_exc() def test_fn_2(arg): try: @@ -98,18 +97,30 @@ def test_fn_2(arg): # It timed out, which means that the lock was still held by the # first thread arg.append(True) + except Exception: + # If something unexpected happened, we want to know what it was + traceback.print_exc() - thread_1 = threading.Thread(target=test_fn_1) res = [] + thread_1 = threading.Thread(target=test_fn_1, args=(res,)) thread_2 = threading.Thread(target=test_fn_2, args=(res,)) thread_1.start() + + # Make sure the first thread has obtained the lock + while False not in res: + time.sleep(0.5) + thread_2.start() # The second thread should raise `filelock.Timeout` thread_2.join() assert True in res + thread_1.join() + assert not thread_1.is_alive() + assert not thread_2.is_alive() + @pytest.mark.skipif(sys.platform != "linux", reason="Fork is only available on linux") def test_locking_multiprocess_fork(): diff --git a/tests/link/c/test_basic.py b/tests/link/c/test_basic.py index dc0d42c500..e7b6bc8b71 100644 --- a/tests/link/c/test_basic.py +++ b/tests/link/c/test_basic.py @@ -3,8 +3,8 @@ import aesara from aesara.compile.mode import Mode -from aesara.graph import fg from aesara.graph.basic import Apply, Constant, Variable +from aesara.graph.fg import FunctionGraph from aesara.graph.op import COp from aesara.graph.type import CType from aesara.link.basic import PerformLinker @@ -180,27 +180,30 @@ def inputs(): return x, y, z -def Env(inputs, outputs): - e = fg.FunctionGraph(inputs, outputs) - return e - - -################ -# Test CLinker # -################ - - @pytest.mark.skipif( not aesara.config.cxx, reason="G++ not available, so we need to skip this test." ) def test_clinker_straightforward(): x, y, z = inputs() e = add(mul(add(x, y), div(x, y)), bad_sub(bad_sub(x, y), z)) - lnk = CLinker().accept(Env([x, y, z], [e])) + lnk = CLinker().accept(FunctionGraph([x, y, z], [e])) fn = lnk.make_function() assert fn(2.0, 2.0, 2.0) == 2.0 +@pytest.mark.skipif( + not aesara.config.cxx, reason="G++ not available, so we need to skip this test." +) +def test_cthunk_str(): + x = double("x") + y = double("y") + e = add(x, y) + lnk = CLinker().accept(FunctionGraph([x, y], [e])) + cthunk, input_storage, output_storage = lnk.make_thunk() + assert str(cthunk).startswith("_CThunk") + assert "module" in str(cthunk) + + @pytest.mark.skipif( not aesara.config.cxx, reason="G++ not available, so we need to skip this test." ) @@ -208,7 +211,7 @@ def test_clinker_literal_inlining(): x, y, z = inputs() z = Constant(tdouble, 4.12345678) e = add(mul(add(x, y), div(x, y)), bad_sub(bad_sub(x, y), z)) - lnk = CLinker().accept(Env([x, y], [e])) + lnk = CLinker().accept(FunctionGraph([x, y], [e])) fn = lnk.make_function() assert abs(fn(2.0, 2.0) + 0.12345678) < 1e-9 code = lnk.code_gen() @@ -253,7 +256,7 @@ def test_clinker_literal_cache(): def test_clinker_single_node(): x, y, z = inputs() node = add.make_node(x, y) - lnk = CLinker().accept(Env(node.inputs, node.outputs)) + lnk = CLinker().accept(FunctionGraph(node.inputs, node.outputs)) fn = lnk.make_function() assert fn(2.0, 7.0) == 9 @@ -265,7 +268,7 @@ def test_clinker_dups(): # Testing that duplicate inputs are allowed. x, y, z = inputs() e = add(x, x) - lnk = CLinker().accept(Env([x, x], [e])) + lnk = CLinker().accept(FunctionGraph([x, x], [e])) fn = lnk.make_function() assert fn(2.0, 2.0) == 4 # note: for now the behavior of fn(2.0, 7.0) is undefined @@ -278,7 +281,7 @@ def test_clinker_not_used_inputs(): # Testing that unused inputs are allowed. x, y, z = inputs() e = add(x, y) - lnk = CLinker().accept(Env([x, y, z], [e])) + lnk = CLinker().accept(FunctionGraph([x, y, z], [e])) fn = lnk.make_function() assert fn(2.0, 1.5, 1.0) == 3.5 @@ -290,20 +293,16 @@ def test_clinker_dups_inner(): # Testing that duplicates are allowed inside the graph x, y, z = inputs() e = add(mul(y, y), add(x, z)) - lnk = CLinker().accept(Env([x, y, z], [e])) + lnk = CLinker().accept(FunctionGraph([x, y, z], [e])) fn = lnk.make_function() assert fn(1.0, 2.0, 3.0) == 8.0 -###################### -# Test OpWiseCLinker # -###################### - # slow on linux, but near sole test and very central def test_opwiseclinker_straightforward(): x, y, z = inputs() e = add(mul(add(x, y), div(x, y)), bad_sub(bad_sub(x, y), z)) - lnk = OpWiseCLinker().accept(Env([x, y, z], [e])) + lnk = OpWiseCLinker().accept(FunctionGraph([x, y, z], [e])) fn = lnk.make_function() if aesara.config.cxx: assert fn(2.0, 2.0, 2.0) == 2.0 @@ -316,7 +315,7 @@ def test_opwiseclinker_constant(): x, y, z = inputs() x = Constant(tdouble, 7.2, name="x") e = add(mul(x, y), mul(y, z)) - lnk = OpWiseCLinker().accept(Env([y, z], [e])) + lnk = OpWiseCLinker().accept(FunctionGraph([y, z], [e])) fn = lnk.make_function() res = fn(1.5, 3.0) assert res == 15.3 @@ -331,15 +330,10 @@ def _my_checker(x, y): raise MyExc("Output mismatch.", {"performlinker": x[0], "clinker": y[0]}) -################### -# Test DualLinker # -################### - - def test_duallinker_straightforward(): x, y, z = inputs() e = add(mul(x, y), mul(y, z)) # add and mul are correct in C and in Python - lnk = DualLinker(checker=_my_checker).accept(Env([x, y, z], [e])) + lnk = DualLinker(checker=_my_checker).accept(FunctionGraph([x, y, z], [e])) fn = lnk.make_function() res = fn(7.2, 1.5, 3.0) assert res == 15.3 @@ -352,7 +346,7 @@ def test_duallinker_mismatch(): x, y, z = inputs() # bad_sub is correct in C but erroneous in Python e = bad_sub(mul(x, y), mul(y, z)) - g = Env([x, y, z], [e]) + g = FunctionGraph([x, y, z], [e]) lnk = DualLinker(checker=_my_checker).accept(g) fn = lnk.make_function() @@ -371,11 +365,6 @@ def test_duallinker_mismatch(): fn(1.0, 2.0, 3.0) -################################ -# Test that failure code works # -################################ - - class AddFail(Binary): def c_code(self, node, name, inp, out, sub): x, y = inp @@ -399,7 +388,7 @@ def test_c_fail_error(): x, y, z = inputs() x = Constant(tdouble, 7.2, name="x") e = add_fail(mul(x, y), mul(y, z)) - lnk = OpWiseCLinker().accept(Env([y, z], [e])) + lnk = OpWiseCLinker().accept(FunctionGraph([y, z], [e])) fn = lnk.make_function() with pytest.raises(RuntimeError): fn(1.5, 3.0) diff --git a/tests/link/c/test_cmodule.py b/tests/link/c/test_cmodule.py index 1fa9c5f786..62ef302c9f 100644 --- a/tests/link/c/test_cmodule.py +++ b/tests/link/c/test_cmodule.py @@ -4,18 +4,21 @@ But this one tests a current behavior that isn't good: the c_code isn't deterministic based on the input type and the op. """ - import logging +import tempfile from unittest.mock import patch import numpy as np +import pytest import aesara +from aesara.compile.ops import DeepCopyOp from aesara.link.c.cmodule import GCC_compiler, default_blas_ldflags +from aesara.link.c.exceptions import CompileError from aesara.tensor.type import dvectors -class MyOp(aesara.compile.ops.DeepCopyOp): +class MyOp(DeepCopyOp): nb_called = 0 def c_code_cache_version(self): @@ -37,6 +40,11 @@ def c_code(self, node, name, inames, onames, sub): ) +def test_compiler_error(): + with pytest.raises(CompileError), tempfile.TemporaryDirectory() as dir_name: + GCC_compiler.compile_str("module_name", "blah", location=dir_name) + + def test_inter_process_cache(): # When an op with c_code, but no version. If we have 2 apply node # in the graph with different inputs variable(so they don't get diff --git a/tests/link/test_vm.py b/tests/link/test_vm.py index 71e6742be7..e29e8f8ea7 100644 --- a/tests/link/test_vm.py +++ b/tests/link/test_vm.py @@ -57,30 +57,35 @@ def test_callback_with_ifelse(self): assert self.n_callbacks["IfElse"] == 2 -def test_c_thunks(): - a = scalars("a") - b, c = vectors("bc") +def test_use_c_thunks(): + a_at = scalars("a") + b_at = vectors("b") + + a = np.array(0.0).astype(config.floatX) + b = np.array([2.0]).astype(config.floatX) + cases = [False] if config.cxx: cases.append(True) - for c_thunks in cases: + + for use_c_thunks in cases: f = function( - [a, b, c], - ifelse(a, a * b, b * c), + [a_at, b_at], + a_at * b_at, mode=Mode( - optimizer=None, linker=VMLinker(c_thunks=c_thunks, use_cloop=False) + optimizer=None, linker=VMLinker(c_thunks=use_c_thunks, use_cloop=False) ), ) - f(1, [2], [3, 2]) - with pytest.raises(ValueError): - f(0, [2], [3, 4]) - assert any([hasattr(t, "cthunk") for t in f.fn.thunks]) == c_thunks + assert np.array_equal(a * b, f(a, b)) + assert any([hasattr(t, "cthunk") for t in f.fn.thunks]) == use_c_thunks @pytest.mark.skipif( not config.cxx, reason="G++ not available, so we need to skip this test." ) def test_speed(): + # TODO FIXME: This isn't a real test. + def build_graph(x, depth=5): z = x for d in range(depth): @@ -145,6 +150,8 @@ def time_linker(name, linker): def test_speed_lazy(): + # TODO FIXME: This isn't a real test. + def build_graph(x, depth=5): z = x for d in range(depth): diff --git a/tests/tensor/test_elemwise.py b/tests/tensor/test_elemwise.py index b8cee55a13..ef897197a6 100644 --- a/tests/tensor/test_elemwise.py +++ b/tests/tensor/test_elemwise.py @@ -25,6 +25,7 @@ discrete_dtypes, matrix, scalar, + vector, vectors, ) from tests import unittest_tools @@ -644,6 +645,36 @@ def test_input_dimensions_overflow(self): g = aesara.function([a, b, c, d, e, f], s, mode=Mode(linker="py")) g(*[np.zeros(2 ** 11, config.floatX) for i in range(6)]) + def check_input_dimensions_match(self, mode): + """Make sure that our input validation works correctly and doesn't + throw erroneous broadcast-based errors. + """ + x_v = matrix("x") + m_v = vector("m") + + x = np.array([[-1.32720483], [0.23442016]]).astype(config.floatX) + m = np.array([0.0, 0.0]).astype(config.floatX) + + z_v = x_v - m_v + f = aesara.function([x_v, m_v], z_v, mode=mode) + + res = f(x, m) + + assert np.array_equal(res, x - m) + + def test_input_dimensions_match_python(self): + self.check_input_dimensions_match(Mode(linker="py")) + + @pytest.mark.xfail( + reason="Elemwise C implementation does not broadcast parameters", + exception=ValueError, + ) + @pytest.mark.skipif( + not aesara.config.cxx, reason="G++ not available, so we need to skip this test." + ) + def test_input_dimensions_match_c(self): + self.check_input_dimensions_match(Mode(linker="c")) + def test_not_implemented_elemwise_grad(): # Regression test for unimplemented gradient in an Elemwise Op. diff --git a/tests/tensor/test_type.py b/tests/tensor/test_type.py index 2f39b08459..369782cd33 100644 --- a/tests/tensor/test_type.py +++ b/tests/tensor/test_type.py @@ -46,7 +46,7 @@ def test_filter_float_subclass(): test_type = TensorType("float64", broadcastable=[]) nan = np.array([np.nan], dtype="float64")[0] - assert isinstance(nan, np.float) and not isinstance(nan, np.ndarray) + assert isinstance(nan, float) and not isinstance(nan, np.ndarray) filtered_nan = test_type.filter(nan) assert isinstance(filtered_nan, np.ndarray) diff --git a/tests/test_ifelse.py b/tests/test_ifelse.py index 8bbdb0e3cf..2f5ca243ee 100644 --- a/tests/test_ifelse.py +++ b/tests/test_ifelse.py @@ -621,26 +621,22 @@ def perform(self, *args, **kwargs): raise NotImplementedError() -def test_ifelse(): +@aesara.config.change_flags(vm__lazy=True) +def test_ifelse_lazy_c(): a = scalar() b = generic() c = generic() notimpl = NotImplementedOp() - lazys = [True] - # We need lazy to end up being True for this test. - if aesara.config.vm__lazy in [True, None]: - lazys = [True, None] - cloops = [True, False] if aesara.config.cxx == "": cloops = [False] - for cloop in cloops: - for lazy in lazys: - linker = aesara.link.vm.VMLinker(use_cloop=cloop, lazy=lazy) + for use_cloop in cloops: + for lazy in [True, None]: + linker = aesara.link.vm.VMLinker(use_cloop=use_cloop, lazy=lazy) f = function( [a, b, c], ifelse(a, notimpl(b), c),