Opened 10 years ago
Closed 9 years ago
#13645 closed defect (fixed)
better input handling for solve()
Reported by: | llpamies | Owned by: | burcin |
---|---|---|---|
Priority: | trivial | Milestone: | sage-5.12 |
Component: | symbolics | Keywords: | |
Cc: | kcrisman | Merged in: | sage-5.12.beta0 |
Authors: | Burcin Erocal | Reviewers: | Punarbasu Purkayastha |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
Consider the following Sage code:
sage: a,b=var('a,b') sage: solve([a+b+a*b == 1], a) [a == -(b - 1)/(b + 1)]
If I want to do something similar with elements of a PolynomialRing? the code crashes:
sage: poly.<a,b> = PolynomialRing(RR) sage: solve([a+b+a*b == 1], a)
Attachments (2)
Change History (16)
comment:1 Changed 10 years ago by
- Milestone changed from sage-5.4 to sage-duplicate/invalid/wontfix
- Reviewers set to Burcin Erocal
- Status changed from new to needs_review
comment:2 follow-up: ↓ 3 Changed 10 years ago by
Tanks burcin, do not close the ticket, I think that something needs to be fixed.
If you say that,
The comparison operator for polynomial rings is important (think monomial orders and Groebner bases), we cannot change it to keep the relations symbolic.
Shouldn't Sage raise an exception when evaluating "a+b+a*b == 1", saying something like "Symbolic comparisons are not allowed for polynomial rings.", instead of returning False. And besides that, why is "solve([False], a)" raising such a incomprehensible "TypeError?" exception ?
Thanks,
Changed 10 years ago by
comment:3 in reply to: ↑ 2 Changed 10 years ago by
- Description modified (diff)
- Milestone changed from sage-duplicate/invalid/wontfix to sage-5.4
- Priority changed from major to trivial
- Reviewers Burcin Erocal deleted
OK. I attached a patch to fix the error message. Now we have:
sage: poly.<a,b> = PolynomialRing(RR) sage: solve([a+b+a*b == 1], a) --------------------------------------------------------------------------- TypeError Traceback (most recent call last) /home/burcin/sage/sage-5.2/<ipython console> in <module>() /home/burcin/sage/sage-5.2/local/lib/python2.7/site-packages/sage/symbolic/relation.pyc in solve(f, *args, **kwds) 681 return f[0].solve(*args,**kwds) 682 # otherwise complain --> 683 raise TypeError("The first argument to solve() should be a symbolic expression or a list of symbolic expressions, cannot handle %s"%repr(type(f[0]))) 684 685 # f is a list of such expressions or equations TypeError: The first argument to solve() should be a symbolic expression or a list of symbolic expressions, cannot handle <type 'bool'>
Please review.
Replying to llpamies:
If you say that,
The comparison operator for polynomial rings is important (think monomial orders and Groebner bases), we cannot change it to keep the relations symbolic.
Shouldn't Sage raise an exception when evaluating "a+b+a*b == 1", saying something like "Symbolic comparisons are not allowed for polynomial rings.", instead of returning False.
No, that operations checks for equality of polynomials. That polynomial is not equal to 1
. As I wrote before, we cannot modify the comparison of polynomials.
And besides that, why is "solve([False], a)" raising such a incomprehensible "TypeError?" exception ?
This was caused by a typo. Python was complaining about a different issue when it said TypeError: not all arguments converted during string formatting
. I hope the new error message is more useful.
comment:4 Changed 10 years ago by
- Summary changed from PolynomialRing variables are not generic symbolic variables to better input handling for solve()
comment:5 Changed 10 years ago by
Should this be explicitly provided in the documentation of solve
as a warning? It seems natural that someone could try to use variables from a polynomial ring and work with them. After all, they are "variables."
I have something like this in mind:
.. warning: Provide only symbolic variables to the ``solve`` function, that is, variables obtained by using the :func:`var`. Other variables, for instance variables obtained from polynomial rings, should not be provided to the ``solve`` function.
what other variables can be defined in Sage, by the way?
comment:6 Changed 9 years ago by
There is another problem here, not related to equality:
sage: a,b=var('a,b') sage: solve([a+b+a*b- 1], a) [a == -(b - 1)/(b + 1)] sage: poly.<a,b> = PolynomialRing(RR) sage: solve([a+b+a*b- 1], a)
The last line does not work, but it could and should ! It can be done by converting polynomials to elements of the symbolic ring.
comment:7 Changed 9 years ago by
Here is an example code which can convert the polynomial to a symbolic expression.
R.<y,z> = RR[] a = y^2 + 2 * z^2 is_Polynomial(a) # test for polynomial a False is_MPolynomial(a) # test for multinomial a True va = a.variables() vSR = map(SR, va) aSR = a.subs({_x: _xs for _x,_xs in zip(va, vSR)}) type(aSR) <type 'sage.symbolic.expression.Expression'> solve(aSR, *vSR) ([y == -I*sqrt(2)*z, y == I*sqrt(2)*z], [1, 1])
This can be done internally in the solve function.
There is only one problem to this approach. Polynomial rings can be over other fields, for instance finite fields, too. The output of solve in this case doesn't really make much sense because it loses the information that the elements are in finite fields.
sage: F.<b> = GF(4) sage: R.<y,z> = F[] sage: a = y^2 + b* z^2 sage: type(a) <type 'sage.rings.polynomial.multi_polynomial_libsingular.MPolynomial_libsingular'> sage: is_MPolynomial(a) True sage: va = a.variables() sage: vSR = map(SR, va) sage: aSR = a.subs({_x: _xs for _x,_xs in zip(va, vSR)}) sage: type(aSR) <type 'sage.symbolic.expression.Expression'> sage: aSR y^2 + b*z^2 sage: solve(aSR, *vSR) ([y == -sqrt(-b)*z, y == sqrt(-b)*z], [1, 1]) sage: aSR.coeffs(z) [[y^2, 0], [b, 2]] sage: type(aSR.coeffs(z)[1][0]) <type 'sage.symbolic.expression.Expression'>
So, we need some test to check whether the polynomial is defined only over reals or complexes.
comment:8 Changed 9 years ago by
- Cc kcrisman added
I somehow feel like maybe it makes more sense that if we have type bool in the list of things as in Burcin's error message
TypeError: The first argument to solve() should be a symbolic expression or a list of symbolic expressions, cannot handle <type 'bool'>
That perhaps we can suggest to the user that instead of a polynomial ring, they should use symbolic variables (rather than the message about bool, which will no doubt confuse people). Of course, there is the other question of whether we want
solve([True,x==1],x)
to return x==1
as a solution, which would muddle this whole approach up.
I don't mind (uncharacteristically) if people can't get an answer to this, not only for the base field issue, but also because we don't want people using polynomial rings any more unless they know what they are doing. The symbolic ring should take care of all "calculus mode" users, which was not the case in the (long ago) past.
comment:9 Changed 9 years ago by
Yikes. Indeed, I forgot that polynomial ring elements actually evaluate to a boolean when input with comparison operators. My example will working only when the relation is (implicitly) equality to zero.
Then Burcin's patch seems the right thing to do.
comment:10 follow-up: ↓ 11 Changed 9 years ago by
Then Burcin's patch seems the right thing to do.
So... do you want to review this, then?
comment:11 in reply to: ↑ 10 Changed 9 years ago by
- Status changed from needs_review to positive_review
Replying to kcrisman:
Then Burcin's patch seems the right thing to do.
So... do you want to review this, then?
Yes. Positive review. I am uploading a rebased patch.
comment:12 Changed 9 years ago by
- Description modified (diff)
- Reviewers set to Punarbasu Purkayastha
Patchbot apply trac_13645-solve_input_handling-rebased.patch
comment:13 Changed 9 years ago by
- Milestone changed from sage-5.11 to sage-5.12
comment:14 Changed 9 years ago by
- Merged in set to sage-5.12.beta0
- Resolution set to fixed
- Status changed from positive_review to closed
Here is what I get:
Is this what you mean by a "crash"?
Note that when you work with variables in a polynomial ring, the comparison is evaluated immediately.
In this case, the call to
solve
becomessolve([False], a)
. The comparison operator for polynomial rings is important (think monomial orders and Groebner bases), we cannot change it to keep the relations symbolic.I suggest we close this as invalid.