Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make_2d_sym: support flat list + shape input #1236

Open
cbm755 opened this issue Sep 13, 2022 · 4 comments
Open

make_2d_sym: support flat list + shape input #1236

cbm755 opened this issue Sep 13, 2022 · 4 comments

Comments

@cbm755
Copy link
Collaborator

cbm755 commented Sep 13, 2022

@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 #1233 (comment)

cbm755 added a commit that referenced this issue Sep 13, 2022
Fixes #1236.  For now, just add a new underscore version that takes a
shape kwarg.
cbm755 added a commit that referenced this issue Sep 13, 2022
Fixes #1236.  For now, just add a new underscore version that takes a
shape kwarg.
cbm755 added a commit that referenced this issue Sep 13, 2022
Fixes #1236.  Or at least the part of #1236 that pertains to `subs`.
Added new tests.
@cbm755
Copy link
Collaborator Author

cbm755 commented Sep 13, 2022

@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
Copy link
Collaborator Author

cbm755 commented Sep 13, 2022

Yep, vertcat is wrong:

>> A = sym(zeros(0,3))
A = (sym) []  (empty 0×3 matrix)
>> [A; A]
ans = (sym) []  (empty 0×0 matrix)

cbm755 added a commit that referenced this issue Sep 13, 2022
Related to #1236.  I don't see a problem with the list-of-lists
approach, but these are the only routine using it so we can delete some
code if we port them to flat + shape.
cbm755 added a commit that referenced this issue Sep 13, 2022
Related to #1236.  I don't see a problem with the list-of-lists
approach, but these are the only routine using it so we can delete some
code if we port them to flat + shape.
@alexvong243f
Copy link
Collaborator

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

@alexvong243f
Copy link
Collaborator

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

cbm755 added a commit that referenced this issue Nov 12, 2024
Fixes #1236.  For now, just add a new underscore version that takes a
shape kwarg.
cbm755 added a commit that referenced this issue Nov 12, 2024
Fixes #1236.  Or at least the part of #1236 that pertains to `subs`.
Added new tests.
cbm755 added a commit that referenced this issue Nov 12, 2024
Related to #1236.  I don't see a problem with the list-of-lists
approach, but these are the only routine using it so we can delete some
code if we port them to flat + shape.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants