Fix solve so that additional args are properly handled

[kcrisman]

In this ask.sagemath question, there is some inconsistency in our use of Maxima's solve, or possibly its solving.

s=list(var('s_%d' % i) for i in range(3));
var('x y z');
print solve([],s_0,s_1,s_2)
print solve([z==z],s_0,s_1,s_2)
print solve([z==z,y==y],s_0,s_1,s_2)
print solve([z==z,y==y,x==x],s_0,s_1,s_2)
print solve([s_0==0],s_0,s_1,s_2);
print solve([s_0==0,s_1==s_1],s_0,s_1,s_2)

[[s_0 == c1, s_1 == c2, s_2 == c3]]
({s_0: r1}, [])
[[s_0 == r6, s_1 == r5, s_2 == r4]]
[[s_0 == r9, s_1 == r8, s_2 == r7]]
([s_0 == 0], [1])
[[s_0 == 0, s_1 == r11, s_2 == r10]]

Most of these do make sense, but you'll notice the change from complex to real variables, one place that seems to have a multiplicity, and the one with only a dictionary as output!

See also this sage-devel thread for another example of where this cropped up. We need to change the behavior of solve and the documentation to fix this.

CC: @jhpalmieri

Component: symbolics

Keywords: solve, maxima, Maxima, sd35.5

Work Issues: error in Maxima

Author: John Palmieri

Reviewer: Paul Zimmermann

Issue created by migration from https://trac.sagemath.org/ticket/10750

[burcin]

comment:1

AFAIK, the issue with only a dictionary output was fixed in #8553. Multiplicity seems to be (one of) the problem(s) in #5679. Perhaps we should make this ticket only about the real & complex variables.

There are so many tickets about solve(). It would be good to make a trac wiki page to list all of them. This should help avoid duplicates.

[kcrisman]

comment:2

There are so many tickets about solve(). It would be good to make a trac wiki page to list all of them. This should help avoid duplicates.

This is here.

[kcrisman]

Description changed:

--- 
+++ 
@@ -17,5 +17,8 @@
 ([s_0 == 0], [1])
 [[s_0 == 0, s_1 == r11, s_2 == r10]]
 }}}
+
 Most of these do make sense, but you'll notice the change from complex to real variables, one place that seems to have a multiplicity, and the one with only a dictionary as output!
+
+See also [http://groups.google.com/group/sage-devel/browse_thread/thread/daf0a5da1da38410 this sage-support thread] for another example of where this cropped up.  We need to change the behavior of solve and the documentation to fix this.
 ```

[kcrisman]

comment:3

Okay, it turns out that there are a few problems here.

• When one solves a single Expression, all args are passed to Expression.solve() which means s_1 goes to multiplicities, and so we are getting multiplicities with the trailing [] and [1].
• If one does this with three variables, then solution_dict is also True and we get the dict.
• Then there is the complex/real issue, which I haven't delved into.

We should fix this all so that the solutions make sense and are consistent.

[kcrisman]

Description changed:

--- 
+++ 
@@ -8,17 +8,18 @@
 print solve([z==z,y==y],s_0,s_1,s_2)
 print solve([z==z,y==y,x==x],s_0,s_1,s_2)
 print solve([s_0==0],s_0,s_1,s_2);
-print solve([s_0==0,s_1==s_1],s_0,s_1,s_2)}}}
-{{{
+print solve([s_0==0,s_1==s_1],s_0,s_1,s_2)
+```
+
+```
 [[s_0 == c1, s_1 == c2, s_2 == c3]]
 ({s_0: r1}, [])
 [[s_0 == r6, s_1 == r5, s_2 == r4]]
 [[s_0 == r9, s_1 == r8, s_2 == r7]]
 ([s_0 == 0], [1])
 [[s_0 == 0, s_1 == r11, s_2 == r10]]
-}}}
+```
 
 Most of these do make sense, but you'll notice the change from complex to real variables, one place that seems to have a multiplicity, and the one with only a dictionary as output!
 
-See also [http://groups.google.com/group/sage-devel/browse_thread/thread/daf0a5da1da38410 this sage-support thread] for another example of where this cropped up.  We need to change the behavior of solve and the documentation to fix this.
-```
+See also [this sage-support thread](http://groups.google.com/group/sage-devel/browse_thread/thread/daf0a5da1da38410) for another example of where this cropped up.  We need to change the behavior of solve and the documentation to fix this.

[kcrisman]

Description changed:

--- 
+++ 
@@ -22,4 +22,4 @@
 
 Most of these do make sense, but you'll notice the change from complex to real variables, one place that seems to have a multiplicity, and the one with only a dictionary as output!
 
-See also [this sage-support thread](http://groups.google.com/group/sage-devel/browse_thread/thread/daf0a5da1da38410) for another example of where this cropped up.  We need to change the behavior of solve and the documentation to fix this.
+See also [this sage-devel thread](http://groups.google.com/group/sage-devel/browse_thread/thread/daf0a5da1da38410) for another example of where this cropped up.  We need to change the behavior of solve and the documentation to fix this.

[jhpalmieri]

comment:7

The documentation for "solve" in symbolic/relation.py says that the arguments are

    -  ``f`` - equation or system of equations (given by a
       list or tuple)
    
    -  ``*args`` - variables to solve for.
    
    -  ``solution_dict``

The function is defined as def solve(f, *args, **kwds):. So I think we can assume that args is the list of variables, whereas any keywords are in kwds. So

• we should document some more possible keywords, like multiplicities, or at least give pointers to documentation to any other solve routines which get called by this one, and

• if args has length greater than one, then we obviously shouldn't just call Expression.solve(), but deal with it some other way. Can we just change the beginning?

diff --git a/sage/symbolic/relation.py b/sage/symbolic/relation.py
--- a/sage/symbolic/relation.py
+++ b/sage/symbolic/relation.py
@@ -646,7 +646,8 @@ def solve(f, *args, **kwds):
         []
     """
     from sage.symbolic.expression import is_Expression
-    if is_Expression(f): # f is a single expression
+    # f is a single expression and there is a single variable
+    if is_Expression(f) and len(args) == 1: 
         ans = f.solve(*args,**kwds)
         return ans

(This is completely untested. Maybe we also need to check is_SymbolicVariable(args[0])...)

[kcrisman]

comment:8

Some of the documentation things you mention are dealt with (to some extent) at #10444, which bitrotted a bit.

Your potential solution seems reasonable - really (as Jason implies in his post about the Python keywords issue) the args shouldn't go on without keywords in this case. I wouldn't worry as much about the is_Symbolic... though I guess it couldn't hurt.

[jhpalmieri]

comment:9

Oh, I didn't see your comment, so I wrote a new patch which adds the same documentation as in #10444, although not quite in the same way. The attached patch passes doctests for me, at least for the patched file. It just adds one new doctest, to illustrate that you can solve for a single equation with respect to more than one variable.

I'm marking as this as "needs review", but it may just be a first draft. I certainly haven't tested it very thoroughly.

[jhpalmieri]

Author: John Palmieri