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.18k stars 154 forks source link

Numpy-like helper functions fail with non-symbolic inputs #876

Open ricardoV94 opened 2 years ago

ricardoV94 commented 2 years ago

Unlike most Ops, that convert inputs with as_tensor_variable, several of the helper functions in tensor.basic do not convert the inputs, leading to some failures when trying to access attributes that are only present in TensorVariables

import numpy as np
import aesara.tensor as at

at.transpose(np.eye(3))
AttributeError: 'numpy.ndarray' object has no attribute 'broadcastable'
brandonwillard commented 2 years ago

Yeah, I think I noticed this deficiency for at.broadcast_to as well.

ltoniazzi commented 2 years ago

Hello, can I work on this issue?

ricardoV94 commented 2 years ago

Hello, can I work on this issue?

Yes

ltoniazzi commented 2 years ago

It seems that these issues (along with the same for at.flatten) are already fixed in the environment aesara-dev. Is there a duplicate issue? Should I just try spot other similar bugs?

brandonwillard commented 2 years ago

It seems that these issues (along with the same for at.flatten) are already fixed in the environment aesara-dev.

You mean they're fixed on upstream (i.e. aesara-devs) main, no?

Is there a duplicate issue? Should I just try spot other similar bugs?

Yeah, there are still a few helper functions in aesara.tensor.basic that aren't calling as_tensor_variable/as_tensor.

The basic idea is that any function that immediately uses a method/attribute from the TensorVariable interface on one of its arguments is implicitly assuming that its arguments are TensorVariables, but they might need to be converted first.

For example, aesara.tensor.basic.roll uses x.ndim, but, if x isn't a numpy.ndarray or TensorVariable, then that expression will fail. Converting x to a TensorVariable before that statement is evaluated will fix the problem. tile, inverse_permutation, and diag all appear to have the same problem.

Aside from those cases, it's possible that some Op.make_node methods don't perform TensorVariable conversions, and, since most of those helper functions call Op.make_node, if neither the helper function nor the Op performs the conversion, there will likely be an error somewhere down the road.

I can't think of any such helper function + Op combinations right now, but they could exist.

ltoniazzi commented 2 years ago

Yes that's what I meant!, and thanks a lot for the hints!

ltoniazzi commented 2 years ago

Two questions:

  1. Is it better to use _x = as_tensor_variable(x) or x = as_tensor_variable(x) to perform TensorVariable conversions?
  2. Can someone suggest a testing strategy or a suitable example to test this TensorVariable conversions? (Something simple but ugly could be to pass a list, for example at.roll([1,2,3], 2), as some functions like roll still run with a numpy array even though the TensorVariable conversion is not performed on the input x)
brandonwillard commented 2 years ago
  1. Is it better to use _x = as_tensor_variable(x) or x = as_tensor_variable(x) to perform TensorVariable conversions?

aesara.tensor.as_tensor_variable is the interface for TensorVariable creation from Python/NumPy values.

2. Can someone suggest a testing strategy or a suitable example to test this TensorVariable conversions? (Something simple but ugly could be to pass a list, for example at.roll([1,2,3], 2), as some functions like roll still run with a numpy array even though the TensorVariable conversion is not performed on the input x)

Most of the test for aesara.tensor.as_tensor_variable are here.

ltoniazzi commented 2 years ago

Thanks, but for (1) I meant whether I should rename the input x to _x like here, or leave it as x like here.

brandonwillard commented 2 years ago

Thanks, but for (1) I meant whether I should rename the input x to _x like here, or leave it as x like here.

Oh, that's usually done for typing (i.e. Mypy) purposes, so, yeah, you can use that idiom.