Opened 10 years ago
Last modified 7 years ago
#10750 needs_work defect
Fix solve so that additional args are properly handled
Reported by: | kcrisman | Owned by: | burcin |
---|---|---|---|
Priority: | major | Milestone: | |
Component: | symbolics | Keywords: | solve, maxima, Maxima, sd35.5 |
Cc: | jhpalmieri | Merged in: | |
Authors: | John Palmieri | Reviewers: | Paul Zimmermann |
Report Upstream: | N/A | Work issues: | error in Maxima |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
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.
Attachments (1)
Change History (15)
comment:1 follow-up: ↓ 2 Changed 10 years ago by
comment:2 in reply to: ↑ 1 Changed 9 years ago by
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.
comment:3 Changed 9 years ago by
- Cc jhpalmieri added
- Description modified (diff)
Okay, it turns out that there are a few problems here.
- When one solves a single
Expression
, all args are passed toExpression.solve()
which meanss_1
goes tomultiplicities
, and so we are getting multiplicities with the trailing[]
and[1]
. - If one does this with three variables, then
solution_dict
is alsoTrue
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.
comment:4 Changed 9 years ago by
- Description modified (diff)
comment:5 Changed 9 years ago by
- Description modified (diff)
comment:6 Changed 9 years ago by
- Summary changed from Unify certain solve outputs to Fix solve so that additional args are properly handled
comment:7 Changed 9 years ago by
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?(This is completely untested. Maybe we also need to check-
sage/symbolic/relation.py
diff --git a/sage/symbolic/relation.py b/sage/symbolic/relation.py
a b def solve(f, *args, **kwds): 646 646 [] 647 647 """ 648 648 from sage.symbolic.expression import is_Expression 649 if is_Expression(f): # f is a single expression 649 # f is a single expression and there is a single variable 650 if is_Expression(f) and len(args) == 1: 650 651 ans = f.solve(*args,**kwds) 651 652 return ans
is_SymbolicVariable(args[0])
...) -
comment:8 Changed 9 years ago by
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.
comment:9 follow-up: ↓ 10 Changed 9 years ago by
- Status changed from new to needs_review
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.
Changed 9 years ago by
comment:10 in reply to: ↑ 9 Changed 9 years ago by
I'm marking as this as "needs review", but it may just be a first draft. I certainly haven't tested it very thoroughly.
I think you changed the previous test, which is fine. But we should catch the solution_dict
issue as well (that is to say we should test that that error no longer occurs).
To test this, one should especially try to make things which previously would have gone into Expression.solve
break. For instance, we would want to make sure multiplicities still could work as well as possible. I think your code will keep that, but it's what would want to be checked. I do not think I will have time to do that right now, sadly.
Someone would also want to just check whether this is enough to close #10444 as a dup, which jhpalmieri is suggesting and I suspect is true.
comment:11 Changed 9 years ago by
- Keywords sd35.5 added
- Reviewers set to Paul Zimmermann
- Status changed from needs_review to needs_work
- Work issues set to error in Maxima
after importing the patch on top of sage-4.8.apha6, I get:
Loading Sage library. Current Mercurial branch is: 10750 sage: s=list(var('s_%d' % i) for i in range(3)); sage: var('x y z'); sage: print solve([],s_0,s_1,s_2) [ [s_0 == c1, s_1 == c2, s_2 == c3] ] sage: print solve([z==z],s_0,s_1,s_2) --------------------------------------------------------------------------- RuntimeError Traceback (most recent call last) /localdisk/tmp/sage-4.8.alpha6/<ipython console> in <module>() /localdisk/tmp/sage-4.8.alpha6/local/lib/python2.6/site-packages/sage/symbolic/relation.pyc in solve(f, *args, **kwds) 736 raise 737 --> 738 if len(s)==0: # if Maxima's solve gave no solutions, try its to_poly_solve 739 try: 740 s = m.to_poly_solve(variables) /localdisk/tmp/sage-4.8.alpha6/local/lib/python2.6/site-packages/sage/interfaces/maxima_abstract.pyc in __len__(self) 1618 """ 1619 P = self._check_valid() -> 1620 return int(P.eval('length(%s)'%self.name())) 1621 1622 def dot(self, other): /localdisk/tmp/sage-4.8.alpha6/local/lib/python2.6/site-packages/sage/interfaces/maxima_lib.pyc in _eval_line(self, line, locals, reformat, **kwds) 418 statement = line[:ind_semi] 419 line = line[ind_semi+1:] --> 420 if statement: result = ((result + '\n') if result else '') + max_to_string(maxima_eval("#$%s$"%statement)) 421 else: 422 statement = line[:ind_dollar] /localdisk/tmp/sage-4.8.alpha6/local/lib/python2.6/site-packages/sage/libs/ecl.so in sage.libs.ecl.EclObject.__call__ (sage/libs/ecl.c:4732)() /localdisk/tmp/sage-4.8.alpha6/local/lib/python2.6/site-packages/sage/libs/ecl.so in sage.libs.ecl.ecl_safe_apply (sage/libs/ecl.c:2838)() RuntimeError: ECL says: Error executing code in Maxima: length: argument cannot be a symbol; found all
Paul
comment:12 Changed 9 years ago by
Paul's problem is just that Maxima recognized that everything is a solution and returned all
.
(%i1) solve(q=q,(w,y,z)); (%o1) all (%i2) solve(w=w,(w,y,z)); (%o2) all (%i3) solve(w=w,y); (%o3) all (%i4) solve(w=w,w); (%o4) all
We would need to special-case this somehow. Good eye!
Here's another place where someone might have run into this problem. We should really fix this soon.
comment:13 Changed 8 years ago by
See also #13286, which is yet another inconsistency.
comment:14 Changed 7 years ago by
I think http://ask.sagemath.org/question/3526/ is probably related to this as well.
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.