aesara-devs / aesara

Aesara is a Python library for defining, optimizing, and efficiently evaluating mathematical expressions involving multi-dimensional arrays.
https://aesara.readthedocs.io
Other
1.17k stars 156 forks source link

Implement ConvolveOp #1485

Closed Smit-create closed 1 year ago

Smit-create commented 1 year ago

Fixes #1223 Here are a few important guidelines and requirements to check before your PR can be merged:

Don't worry, your PR doesn't need to be in perfect order to submit it. As development progresses and/or reviewers request changes, you can always rewrite the history of your feature/PR branches.

If your PR is an ongoing effort and you would like to involve us in the process, simply make it a draft PR.

codecov[bot] commented 1 year ago

Codecov Report

Merging #1485 (cc6f9f2) into main (fa2bbaf) will increase coverage by 0.01%. The diff coverage is 92.00%.

:exclamation: Current head cc6f9f2 differs from pull request most recent head f64018b. Consider uploading reports for the commit f64018b to get more accurate results

Additional details and impacted files [![Impacted file tree graph](https://codecov.io/gh/aesara-devs/aesara/pull/1485/graphs/tree.svg?width=650&height=150&src=pr&token=2HNzVWyxrA&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aesara-devs)](https://codecov.io/gh/aesara-devs/aesara/pull/1485?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aesara-devs) ```diff @@ Coverage Diff @@ ## main #1485 +/- ## ========================================== + Coverage 75.06% 75.08% +0.01% ========================================== Files 194 194 Lines 50101 50151 +50 Branches 12097 12107 +10 ========================================== + Hits 37610 37656 +46 - Misses 10170 10172 +2 - Partials 2321 2323 +2 ``` | [Impacted Files](https://codecov.io/gh/aesara-devs/aesara/pull/1485?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aesara-devs) | Coverage Δ | | |---|---|---| | [aesara/tensor/basic.py](https://codecov.io/gh/aesara-devs/aesara/pull/1485?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aesara-devs#diff-YWVzYXJhL3RlbnNvci9iYXNpYy5weQ==) | `90.05% <92.00%> (+0.06%)` | :arrow_up: |
Smit-create commented 1 year ago

I have tried to accommodate the fix that you suggested. I have the following issues:

  1. The CI says a failing code-style test but that passes for me locally:

    % pre-commit run --show-diff-on-failure --color=always --all-files
    debug statements (python)................................................Passed
    check for merge conflicts................................................Passed
    pyupgrade................................................................Passed
    black....................................................................Passed
    flake8...................................................................Passed
    isort....................................................................Passed
    autoflake................................................................Passed
    mypy.....................................................................Passed
  2. Running the tests, there is a failure on the C-compilation side on macOS:

    
    Traceback (most recent call last):
    File "/Users/thebigbool/repos/aesara/b.py", line 160, in <module>
    aesara_sol = convolve(a, v, mode=mode).eval()
    File "/Users/thebigbool/repos/aesara/aesara/graph/basic.py", line 605, in eval
    self._fn_cache[inputs] = function(inputs, self)
    File "/Users/thebigbool/repos/aesara/aesara/compile/function/__init__.py", line 317, in function
    fn = pfunc(
    File "/Users/thebigbool/repos/aesara/aesara/compile/function/pfunc.py", line 367, in pfunc
    return orig_function(
    File "/Users/thebigbool/repos/aesara/aesara/compile/function/types.py", line 1815, in orig_function
    fn = m.create(defaults)
    File "/Users/thebigbool/repos/aesara/aesara/compile/function/types.py", line 1708, in create
    _fn, _i, _o = self.linker.make_thunk(
    File "/Users/thebigbool/repos/aesara/aesara/link/basic.py", line 254, in make_thunk
    return self.make_all(
    File "/Users/thebigbool/repos/aesara/aesara/link/vm.py", line 1252, in make_all
    raise_with_op(fgraph, node)
    File "/Users/thebigbool/repos/aesara/aesara/link/utils.py", line 533, in raise_with_op
    raise exc_value.with_traceback(exc_trace)
    File "/Users/thebigbool/repos/aesara/aesara/link/vm.py", line 1243, in make_all
    node.op.make_thunk(node, storage_map, compute_map, [], impl=impl)
    File "/Users/thebigbool/repos/aesara/aesara/link/c/op.py", line 131, in make_thunk
    return self.make_c_thunk(node, storage_map, compute_map, no_recycling)
    File "/Users/thebigbool/repos/aesara/aesara/link/c/op.py", line 96, in make_c_thunk
    outputs = cl.make_thunk(
    File "/Users/thebigbool/repos/aesara/aesara/link/c/basic.py", line 1200, in make_thunk
    cthunk, module, in_storage, out_storage, error_storage = self.__compile__(
    File "/Users/thebigbool/repos/aesara/aesara/link/c/basic.py", line 1120, in __compile__
    thunk, module = self.cthunk_factory(
    File "/Users/thebigbool/repos/aesara/aesara/link/c/basic.py", line 1644, in cthunk_factory
    module = cache.module_from_key(key=key, lnk=self)
    File "/Users/thebigbool/repos/aesara/aesara/link/c/cmodule.py", line 1240, in module_from_key
    module = lnk.compile_cmodule(location)
    File "/Users/thebigbool/repos/aesara/aesara/link/c/basic.py", line 1543, in compile_cmodule
    module = c_compiler.compile_str(
    File "/Users/thebigbool/repos/aesara/aesara/link/c/cmodule.py", line 2654, in compile_str
    raise CompileError(
    aesara.link.c.exceptions.CompileError: Compilation failed (return status=1):
    /Users/thebigbool/opt/anaconda3/envs/aesara-dev-1/bin/clang++ -dynamiclib -g -O3 -fno-math-errno -Wno-unused-label -Wno-unused-variable -Wno-write-strings -DNPY_NO_DEPRECATED_API=NPY_1_7_API_VERSION -m64 -fPIC -undefined dynamic_lookup -Wno-c++11-narrowing -I/Users/thebigbool/opt/anaconda3/envs/aesara-dev-1/lib/python3.10/site-packages/numpy/core/include -I/Users/thebigbool/opt/anaconda3/envs/aesara-dev-1/include/python3.10 -I/Users/thebigbool/repos/aesara/aesara/link/c/c_code -L/Users/thebigbool/opt/anaconda3/envs/aesara-dev-1/lib -fvisibility=hidden -o /Users/thebigbool/.aesara/compiledir_macOS-13.0-x86_64-i386-64bit-i386-3.10.8-64/tmpbb3yfwbq/mf6fae8a01551c3ac2d9a0aaba0ff89f2ec61ceb4a936a76263a7c5d4ea33e204.so /Users/thebigbool/.aesara/compiledir_macOS-13.0-x86_64-i386-64bit-i386-3.10.8-64/tmpbb3yfwbq/mod.cpp
    ld: unsupported tapi file type '!tapi-tbd' in YAML file '/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/lib/libSystem.tbd' for architecture x86_64
    clang-14: error: linker command failed with exit code 1 (use -v to see invocation)

Apply node that caused the error: DeepCopyOp(TensorConstant{[ 0 0 1 .. 10 13 10]}) Toposort index: 0 Inputs types: [TensorType(int64, (8,))]

HINT: Use a linker other than the C linker to print the inputs' shapes and strides. 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'. HINT: Use the Aesara flag exception_verbosity=high for a debug print-out and storage map footprint of this Apply node.

3. Some of the tests do fail after removing the constant abstraction at the make_node level as seen in the following traceback, where it is unable to extract constant shape for mode="valid" and mode="same", where it should if the m, and n are already constant.
```console
TypeError: __trunc__ returned non-Integral (type TensorVariable)

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/Users/thebigbool/repos/aesara/b.py", line 160, in <module>
    aesara_sol = convolve(a, v, mode=mode).eval()
  File "/Users/thebigbool/repos/aesara/aesara/graph/basic.py", line 605, in eval
    self._fn_cache[inputs] = function(inputs, self)
  File "/Users/thebigbool/repos/aesara/aesara/compile/function/__init__.py", line 317, in function
    fn = pfunc(
  File "/Users/thebigbool/repos/aesara/aesara/compile/function/pfunc.py", line 367, in pfunc
    return orig_function(
  File "/Users/thebigbool/repos/aesara/aesara/compile/function/types.py", line 1803, in orig_function
    m = Maker(
  File "/Users/thebigbool/repos/aesara/aesara/compile/function/types.py", line 1574, in __init__
    self.prepare_fgraph(inputs, outputs, found_updates, fgraph, mode, profile)
  File "/Users/thebigbool/repos/aesara/aesara/compile/function/types.py", line 1434, in prepare_fgraph
    rewriter_profile = rewriter(fgraph)
  File "/Users/thebigbool/repos/aesara/aesara/graph/rewriting/basic.py", line 133, in __call__
    return self.rewrite(fgraph)
  File "/Users/thebigbool/repos/aesara/aesara/graph/rewriting/basic.py", line 128, in rewrite
    self.add_requirements(fgraph)
  File "/Users/thebigbool/repos/aesara/aesara/graph/rewriting/basic.py", line 349, in add_requirements
    rewrite.add_requirements(fgraph)
  File "/Users/thebigbool/repos/aesara/aesara/tensor/rewriting/shape.py", line 733, in add_requirements
    fgraph.attach_feature(ShapeFeature())
  File "/Users/thebigbool/repos/aesara/aesara/graph/fg.py", line 707, in attach_feature
    feature.on_attach(self)
  File "/Users/thebigbool/repos/aesara/aesara/tensor/rewriting/shape.py", line 525, in on_attach
    self.on_import(fgraph, node, reason="on_attach")
  File "/Users/thebigbool/repos/aesara/aesara/tensor/rewriting/shape.py", line 589, in on_import
    self.set_shape(r, s)
  File "/Users/thebigbool/repos/aesara/aesara/tensor/rewriting/shape.py", line 361, in set_shape
    shape_vars += (constant(r.type.shape[i], dtype="int64", ndim=0),)
  File "/Users/thebigbool/repos/aesara/aesara/tensor/basic.py", line 215, in constant
    x_ = aes.convert(x, dtype=dtype)
  File "/Users/thebigbool/repos/aesara/aesara/scalar/basic.py", line 245, in convert
    x_ = _asarray(x, dtype=dtype)
  File "/Users/thebigbool/repos/aesara/aesara/misc/safe_asarray.py", line 35, in _asarray
    rval = np.asarray(a, dtype=dtype, order=order)
ValueError: setting an array element with a sequence.
brandonwillard commented 1 year ago

I just pushed a change that should fix some of those issues. It looks like we need to update the type hints (unrelated to this work), so I'll put in a separate PR for that shortly.

Smit-create commented 1 year ago

Thanks @brandonwillard!

maresb commented 1 year ago

@Smit-create, let's open a new issue for

ld: unsupported tapi file type '!tapi-tbd' in YAML file '/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/lib/libSystem.tbd' for architecture x86_64

and including the output of mamba list. It sounds like it may be some sort of Conda-related issue.