gnu-octave / symbolic

A Symbolic Package for Octave using SymPy
https://octave.sourceforge.io/symbolic/
GNU General Public License v3.0
151 stars 36 forks source link

Array_not_Matrix: boolean ops on Arrays are broken #1222

Closed cbm755 closed 1 year ago

cbm755 commented 2 years ago
>> t = sym(true)
>> w = [t t]
pydebug: make_matrix_or_array: making 2D Array...
pydebug: I am here with an array with shape (2, 1)
pydebug: make_matrix_or_array: making 2D Array...
pydebug: I am here with an array with shape (1, 2)
w = (sym) [[True  True]]  (1×2 matrix)
>> sympy(w)
ans = ImmutableDenseNDimArray(Tuple(true, true), Tuple(Integer(1), Integer(2)))

So far so good! But try boolean op:

>> w & w
error: Python exception: TypeError: expecting bool or Boolean, not `[[True, True]]`.
    occurred at line 14 of the Python code block:
    elements.append(_op(*[k[i] if isinstance(k, MatrixBase) else k for k in _ins]))
error: called from
    pycall_sympy__ at line 179 column 7
    elementwise_op at line 105 column 5
    and at line 47 column 5
cbm755 commented 2 years ago

I traced this to linear indexing in SymPy. With Matrix M one can do M[0] and get the 1st element (i.e., same as M[0,0]). But Array does not behave this way:

ipython:


In [30]: A = ImmutableDenseNDimArray((true, true, false, true, false, false), (2,3))

In [31]: B = Matrix(2, 3, range(10, 16))

In [32]: pprint(A)
⎡True  True   False⎤
⎢                  ⎥
⎣True  False  False⎦

In [27]: pprint(B)
⎡10  11  12⎤
⎢          ⎥
⎣13  14  15⎦

In [28]: A[0]
Out[28]: [True, True, False]

In [29]: B[0]
Out[29]: 10

So we can either redo our uniop_... and binop_ helpers yet again or check which behaviour is intended upstream.

cbm755 commented 2 years ago

Filed upstream https://github.com/sympy/sympy/issues/24007

I suspect we should re-write this code not to use linear indexing

cbm755 commented 2 years ago

Yeah, upstream is pretty clear that 2-d indexing is the way to go. 1-d indexing of Matrix is just b/c its always been that way. 1-d indexing of Array is not going to happen: Array is supposed to stay close to numpy behaviour.

alexvong243f commented 1 year ago

Fixed by https://github.com/cbm755/octsympy/pull/1223