gnu-octave / symbolic

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

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

Closed chrisjohgorman closed 2 years ago

chrisjohgorman commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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