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

elementwise_op: call make_array_or_matrix #1233

Closed cbm755 closed 1 year ago

cbm755 commented 1 year ago

Instead of doing it ourselves, prepare a list of lists and call the centralized helper function.

cbm755 commented 1 year ago

@alexvong1995 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?

alexvong243f commented 1 year ago

------- Original Message ------- On Wednesday, September 7th, 2022 at 1:52 AM, Colin B. Macdonald @.***> wrote:

@alexvong1995 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?

I can see this was merged. But I am a bit worried about this being too low-level. I haven't look into it in details but perhaps there is another way to do this (?)

In any case, I think we should keep the high-level function so that we use the low-level function only when we have to. I think it's quite common for types / data structures / classes to have multiple constructors. WDYT?

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.

cbm755 commented 1 year ago

@alexvong1995 I moved this discussion to #1239: very relevant there. And yes we can keep both.