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

Insure left and right division is working properly with mldivide.m and mrdivide.m #1118

Closed
chrisjohgorman opened this issue Apr 23, 2022 · 2 comments · Fixed by #1147
Closed
Milestone

Comments

@chrisjohgorman
Copy link
Contributor

Due to some failing tests we noticed that A = C/B where C was generated by A*B=C was failing in mrdivide.m. The issue is issue #1079. In an attempt to fix this issue made two minor changes to mrdivide.m. One was the a call to regular division if B is a scalar, or more accurately isn't a matrix, the other was solve call, from upstream in the case where B is a matrix. No changes were needed for to fix mldivide.m.

Experimentation showed that this will fail if B is not invertable.

Should we add an expected failure or documentation to outline what won't work with these operators?

@alexvong243f
Copy link
Collaborator

According to SMT documentation, B / A is equivalent to (A' \ B')'. Right now, mldivide (\) uses solve_linear_system in sympy to find a solution, but mrdivide (/) uses solve in sympy to find a solution.

I think this will cause inconsistency in the future and we should define B / A as (A' \ B')' instead of running sympy code directly. What does everyone think?

@cbm755
Copy link
Collaborator

cbm755 commented Jun 18, 2022

Try it, see if tests pass! Certainly sounds like less code to maintain going forward.

alexvong243f pushed a commit to alexvong243f/octsympy that referenced this issue Jun 22, 2022
Use "mldivide" instead of running sympy code directly.

Closes gnu-octave#1118.

* inst/@sym/mrdivide.m: Simplify it.
* inst/@sym/mldivide.m: Adjust accordingly to avoid infinite loop.
alexvong243f pushed a commit to alexvong243f/octsympy that referenced this issue Jun 22, 2022
Use @sym/mldivide instead of running sympy code directly.

Closes gnu-octave#1118.

* inst/@sym/mrdivide.m: Simplify it.
* inst/@sym/mldivide.m: Adjust accordingly to avoid infinite loop.
alexvong243f pushed a commit to alexvong243f/octsympy that referenced this issue Jun 22, 2022
Use @sym/mldivide instead of running sympy code directly.

Closes gnu-octave#1118.

* inst/@sym/mrdivide.m: Simplify it.
* inst/@sym/mldivide.m: Adjust accordingly to avoid infinite loop.
@cbm755 cbm755 added this to the 3.0.0 milestone Jun 22, 2022
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

Successfully merging a pull request may close this issue.

3 participants