Opened 14 months ago

Closed 5 months ago

# better input handling for solve()

Reported by: Owned by: llpamies burcin trivial sage-5.12 symbolics kcrisman sage-5.12.beta0 Burcin Erocal Punarbasu Purkayastha N/A

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)
```

### comment:1 Changed 14 months ago by burcin

• Milestone changed from sage-5.4 to sage-duplicate/invalid/wontfix
• Reviewers set to Burcin Erocal
• Status changed from new to needs_review

Here is what I get:

```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)
676     for v in variables:
677         if not is_SymbolicVariable(v):
--> 678             raise TypeError, "%s is not a valid variable."%v
679
680     try:

TypeError: not all arguments converted during string formatting
```

Is this what you mean by a "crash"?

Note that when you work with variables in a polynomial ring, the comparison is evaluated immediately.

```sage: poly.<a,b> = PolynomialRing(RR)
sage: a+b+a*b == 1
False
```

In this case, the call to solve becomes solve([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.

### comment:2 follow-up: ↓ 3 Changed 14 months ago by llpamies

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,

### comment:3 in reply to: ↑ 2 Changed 14 months ago by burcin

• Authors set to Burcin Erocal
• 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'>
```

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 13 months ago by burcin

• Summary changed from PolynomialRing variables are not generic symbolic variables to better input handling for solve()

### comment:5 Changed 13 months ago by ppurka

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 months ago by chapoton

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 months ago by ppurka

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 months ago by kcrisman

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 months ago by ppurka

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 6 months ago by kcrisman

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 5 months ago by ppurka

• Status changed from needs_review to positive_review

Then Burcin's patch seems the right thing to do.

So... do you want to review this, then?

### Changed 5 months ago by ppurka

Rebased to 5.11-beta3

### comment:12 Changed 5 months ago by ppurka

• Description modified (diff)
• Reviewers set to Punarbasu Purkayastha

Patchbot apply trac_13645-solve_input_handling-rebased.patch

### comment:13 Changed 5 months ago by jdemeyer

• Milestone changed from sage-5.11 to sage-5.12

### comment:14 Changed 5 months ago by jdemeyer

• Merged in set to sage-5.12.beta0
• Resolution set to fixed
• Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.