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

make_2d_sym: support flat list + shape input #1236

Open cbm755 opened 1 year ago

cbm755 commented 1 year ago

@alexvong243f I tried gating dbg_no_array to True on old (that is, currently released) SymPy. We're going to want all CI tests to pass in that case.

But they do not.

Consider:

 a = zeros (sym(3), 0);
 assert (size (a == a), [3, 0])

Here a == a gives 0x0. Perhaps there is a fundamental limitation of list-of-lists representation: you can make [[], [], []] for a 3x0 but there is no representation for a 0x3.

One fix here would be to introduce an even lower-level "flat-list + shape" helper function... What do you think?

Originally posted by @cbm755 in https://github.com/cbm755/octsympy/issues/1233#issuecomment-1238818601

cbm755 commented 1 year ago

@alexvong243f after #1237 there are only 3 callers of the "list-of-lists" form: I think I will port them all to "flat+shape" (currently the underscore version).

@sym/private/elementwise_op.m:          'return _make_2d_sym(g, shape=q.shape)' ];
@sym/private/mat_rclist_access.m:         'M = make_2d_sym(MM)'
@sym/private/mat_rclist_asgn.m:         'M = make_2d_sym(MM)'
@sym/subs.m:    'return _make_2d_sym(g, shape=(m, n))'
@sym/vertcat.m:         'return make_2d_sym(CC)'};

Checking now if vertcat can give wrong size empties...

cbm755 commented 1 year ago

Yep, vertcat is wrong:

>> A = sym(zeros(0,3))
A = (sym) []  (empty 0×3 matrix)
>> [A; A]
ans = (sym) []  (empty 0×0 matrix)
alexvong243f commented 1 year ago

I think this was introduced in 5ea9b3039eb16701e3cc63bc0eb9be41138ecd70. The problem goes away after I revert 5ea9b3039eb16701e3cc63bc0eb9be41138ecd70.

alexvong243f commented 1 year ago

Indeed, this was mentioned in #1233. I'll comment there.