Opened 14 years ago

# solve() can return undefined points as "solutions"

Reported by: Owned by: cwitty was major sage-9.5 calculus Matt Torrence Vincent Delecroix N/A u/gh-Torrencem/2617_solve_check_domain 2cc6fd5ac64ec0a86dea43286261b8135f303d4c todo

```sage: solve(sin(x^2)/x == 0, x)
[x == 0]
sage: solve(sin(x^2)/x^2 == 0, x)
[x == 0]
sage: solve(sin(x^2)/x^3 == 0, x)
[x == 0]
```

None of these functions are even defined at x=0, so that should not be returned as a solution. (The first two functions can be extended to x=0 by taking limits, in which case x=0 is a solution to the first one but not the second; the third function has a vertical asymptote at x=0.)

### comment:1 Changed 14 years ago by mabshoff

• Priority changed from major to critical

Is this a bug in Maxima? In that case we should report those to them? This also seams like a fairly serious issue, so I am elevating this to critical.

Cheers,

Michael

### comment:2 Changed 12 years ago by kcrisman

This is a Maxima bug as of 5.16.3, and has been reported there as 2845005 (see http://sourceforge.net/tracker/?func=detail&aid=2845005&group_id=4933&atid=104933).

### comment:3 follow-up: ↓ 4 Changed 12 years ago by robert.marik

Perhaps related issue is also that the solving acot(x) == 0 ends with error message "The number 0 isn't in the domain of cot"

The online tool Mathatmatical Assistant on Web ( http://user.mendelu.cz/marik/maw/index.php?lang=en&form=main ) has a wrapper for maxima's solve ( http://mathassistant.cvs.sourceforge.net/viewvc/mathassistant/maw/common/maw_solve.mac?revision=1.14&view=markup )

I hope, it could be used also in Sage. I'll try it, hope within a week.

### comment:4 in reply to: ↑ 3 ; follow-up: ↓ 5 Changed 12 years ago by kcrisman

Perhaps related issue is also that the solving acot(x) == 0 ends with error message "The number 0 isn't in the domain of cot"

No, this is an appropriate error message (it's from Maxima, not Sage). There are no solutions to acot(x)==0, at least over the reals (and presumably over the complex field as well?). Now that we know about that error, it would be easy to put a catch in for something like that error message and return sage: solve(acot(x),x) [] instead. Feel free to open a ticket for that and put me in the cc: field.

But this is unrelated to the issue in the ticket, which is a genuine Maxima bug, as far as I can tell.

### comment:5 in reply to: ↑ 4 Changed 12 years ago by kcrisman

• Report Upstream set to N/A

Perhaps related issue is also that the solving acot(x) == 0 ends with error message "The number 0 isn't in the domain of cot"

No, this is an appropriate error message (it's from Maxima, not Sage). There are no solutions to acot(x)==0, at least over the reals (and presumably over the complex field as well?). Now that we know about that error, it would be easy to put a catch in for something like that error message and return sage: solve(acot(x),x) []

This will be addressed (not the main point of this ticket) in the patch for #7745. The main point is still a bug in Maxima 5.20.1.

### comment:6 Changed 12 years ago by robert.marik

I had an idea to introduce new option to solve, which

1. Takes only explicit solutions

1. Substitutes into equation and if an error appears, removes this "solution" from the list.

The problem in this approach is, that for example ln(0)=-Infinity in Sage and so x=0 will be still reported as a solution of x/ln(x)=0. The problem could be solved by substituting values in Maxima and not in Sage, but I am still thinking on some cleaner solution. And still have no idea what should be returned as solution of x*ln(x-3) == 0. Distinguish in this new option, if the user works in real domain or in complex doman? Something like check_domain = False, True, or 'real'?

Any idea?

### comment:7 Changed 12 years ago by kcrisman

As it turns out, to_poly_solve can handle this sort of thing (see in Maxima the share/contrib/rtest_to_poly_solver.mac line 1092). But we would have to figure out a way to interpret the if statements properly (for instance, to note that twice an integer plus one is not zero).

```/* Sage Ticket 2617; see also Sage mailing list 18 March 2008 */

nicedummies(to_poly_solve(sin(x^2)/x,x));
%union(%if(2*%z0+1 # 0,[x = -sqrt(2*%pi*%z0+%pi)],%union()),
%if(2*%z0+1 # 0,[x = sqrt(2*%pi*%z0+%pi)],%union()),
%if(%z1 # 0,[x = -sqrt(2)*sqrt(%pi)*sqrt(%z1)],%union()),
%if(%z1 # 0,[x = sqrt(2)*sqrt(%pi)*sqrt(%z1)],%union()))\$

nicedummies(to_poly_solve(sin(x^2)/x^2,x));
%union(%if(2*%z0+1 # 0,[x = -sqrt(2*%pi*%z0+%pi)],%union()),
%if(2*%z0+1 # 0,[x = sqrt(2*%pi*%z0+%pi)],%union()),
%if(%z1 # 0,[x = -sqrt(2)*sqrt(%pi)*sqrt(%z1)],%union()),
%if(%z1 # 0,[x = sqrt(2)*sqrt(%pi)*sqrt(%z1)],%union()))\$

nicedummies(to_poly_solve(sin(x^2)/x^3,x));
%union(%if(2*%z0+1 # 0,[x = -sqrt(2*%pi*%z0+%pi)],%union()),
%if(2*%z0+1 # 0,[x = sqrt(2*%pi*%z0+%pi)],%union()),
%if(%z1 # 0,[x = -sqrt(2)*sqrt(%pi)*sqrt(%z1)],%union()),
%if(%z1 # 0,[x = sqrt(2)*sqrt(%pi)*sqrt(%z1)],%union()))\$
```

### comment:8 Changed 10 years ago by kcrisman

• Priority changed from critical to major

### comment:9 Changed 8 years ago by jdemeyer

• Milestone changed from sage-5.11 to sage-5.12

### comment:10 Changed 8 years ago by vbraun_spam

• Milestone changed from sage-6.1 to sage-6.2

### comment:11 Changed 8 years ago by vbraun_spam

• Milestone changed from sage-6.2 to sage-6.3

### comment:12 Changed 7 years ago by vbraun_spam

• Milestone changed from sage-6.3 to sage-6.4

### comment:13 Changed 6 years ago by jakobkroeker

• Stopgaps set to todo

### comment:14 Changed 6 years ago by zonova

SO, has this issue been fixed yet? What of kcrisman and robert.marik's suggestions? Also is there a reason that there is no branch to edit?

### comment:15 Changed 6 years ago by kcrisman

Presumably not fixed. No branch because no one has posted one yet - if you have a fix you can be the first to post a branch!

### comment:16 Changed 5 years ago by zonova

kcrisman, in your post (from 7 years ago) you had mentioned to_poly_solve in maxima's share/contrib. It's been a while since then, so it is not located there anymore. I couldn't find it anywhere in Maxima's source on github though, so i wasn't sure if it was still used at all. Does sage/maxima use it at all?

I've been looking at several old tickets, all involving solving equations. One was using find_root, which uses scipy, and the other had to do with solve just like this one. I think it would be best to just have one, no? As far as I can tell, they do about the same thing, and they both have issues. On a similar note, if to_poly_solve resolves this issue, then maybe we should use that for all equation solving?

### comment:17 Changed 5 years ago by kcrisman

We definitely have that method and it is still in Maxima. Looks like it moved to https://sourceforge.net/p/maxima/code/ci/master/tree/share/to_poly_solve/.

However, `find_root` is explicitly supposed to be a numerical solver, while `solve` is supposed to be an exact solver. Because `to_poly_solve` sometimes returns numerical answers in rare situations, there could be some overlap. Also, `to_poly_solve` is not what we want for all solving, because it changes some other things and of course might take longer for simple ones. It has a specific purpose, but that is not a general purpose.

On the other hand, if sympy can now solve everything Maxima does, one could try to switch the default algorithm to use that instead. I don't know if we're at that point, though.

### comment:18 Changed 2 years ago by gh-Torrencem

• Authors set to Matt Torrence
• Branch set to u/gh-Torrencem/2617_solve_check_domain
• Commit set to 2cc6fd5ac64ec0a86dea43286261b8135f303d4c
• Milestone changed from sage-6.4 to sage-8.9
• Status changed from new to needs_review

This has still been an issue, and so I've implemented an optional solution

You can pass the optional argument (`check_domain`) to `solve`, which will tell it to check each solution it finds with SymPy? to see if it's NaN. I added some documentation and an example.

I ran (on my machine) all the current doc-tests with the new argument to make sure it accepts all their solutions, and it does, but I believe it significantly slows down the function, so probably should not end up defaulting to `True` without some more considerations / optimizations (especially since `solve` is widely used)

```sage: solve((x^2 - 1)/(sin(x - 1)) == 0, x, check_domain=True)
[x == -1]
```

New commits:

 ​6d5aa9c `2617: Add check_domain argument to solve to remove results outside of the domain` ​2cc6fd5 `2617: Add small documentation note`

### comment:19 Changed 2 years ago by embray

• Milestone changed from sage-8.9 to sage-9.1

Ticket retargeted after milestone closed

### comment:20 Changed 22 months ago by vdelecroix

On 9.1.beta5 we get something else than the ticket description

```sage: solve(sin(x^2)/x == 0)
---------------------------------------------------------------------------
IndexError                                Traceback (most recent call last)
<ipython-input-1-e922184d1fd1> in <module>()
----> 1 solve(sin(x**Integer(2))/x == Integer(0))

/opt/sage/local/lib/python3.7/site-packages/sage/symbolic/relation.py in solve(f, *args, **kwds)
1016         x = args
1017     else:
-> 1018         x = args
1019     if isinstance(x, (list, tuple)):
1020         for i in x:

IndexError: tuple index out of range
```

### comment:21 Changed 22 months ago by vdelecroix

• Description modified (diff)

### comment:22 follow-ups: ↓ 23 ↓ 24 Changed 22 months ago by vdelecroix

• Reviewers set to Vincent Delecroix
• Status changed from needs_review to needs_info

Your solution is somehow complicated and provides a wrong answer. Why not prefer

```sage: solve(sin(x^2)/x^3 == 0, x, algorithm="sympy")
Complement(ConditionSet(x, Eq(sin(x**2), 0), Complexes), FiniteSet(0))
```

### comment:23 in reply to: ↑ 22 Changed 22 months ago by gh-varenyamBakshi

Your solution is somehow complicated and provides a wrong answer. Why not prefer

```sage: solve(sin(x^2)/x^3 == 0, x, algorithm="sympy")
Complement(ConditionSet(x, Eq(sin(x**2), 0), Complexes), FiniteSet(0))
```

the syntax you are providing is not user friendly. I would prefer the previous syntax.

### comment:24 in reply to: ↑ 22 ; follow-up: ↓ 25 Changed 21 months ago by gh-Shlokatadistance

Your solution is somehow complicated and provides a wrong answer. Why not prefer

```sage: solve(sin(x^2)/x^3 == 0, x, algorithm="sympy")
Complement(ConditionSet(x, Eq(sin(x**2), 0), Complexes), FiniteSet(0))
```

I think the syntax simplicity can be judged in the later stages, I do want to ask are you able to obtain the answer from this? because I can't seem to be getting this work, even after a certain modification, such as additional functions for testing and new variable.

### comment:25 in reply to: ↑ 24 ; follow-up: ↓ 26 Changed 21 months ago by vdelecroix

Your solution is somehow complicated and provides a wrong answer. Why not prefer

```sage: solve(sin(x^2)/x^3 == 0, x, algorithm="sympy")
Complement(ConditionSet(x, Eq(sin(x**2), 0), Complexes), FiniteSet(0))
```

I think the syntax simplicity can be judged in the later stages, I do want to ask are you able to obtain the answer from this? because I can't seem to be getting this work, even after a certain modification, such as additional functions for testing and new variable.

I don't understand your question. `Complement(ConditionSet(x, Eq(sin(x**2), 0), Complexes), FiniteSet(0))` is the answer. Not in a very nice form, but a valid answer.

### comment:26 in reply to: ↑ 25 ; follow-up: ↓ 27 Changed 21 months ago by gh-Shlokatadistance

Your solution is somehow complicated and provides a wrong answer. Why not prefer

```sage: solve(sin(x^2)/x^3 == 0, x, algorithm="sympy")
Complement(ConditionSet(x, Eq(sin(x**2), 0), Complexes), FiniteSet(0))
```

I think the syntax simplicity can be judged in the later stages, I do want to ask are you able to obtain the answer from this? because I can't seem to be getting this work, even after a certain modification, such as additional functions for testing and new variable.

I don't understand your question. `Complement(ConditionSet(x, Eq(sin(x**2), 0), Complexes), FiniteSet(0))` is the answer. Not in a very nice form, but a valid answer.

Ahh my bad, I was trying to obtain a more numeric based answer, I did see that the second statement did resemble the solution

### comment:27 in reply to: ↑ 26 Changed 21 months ago by vdelecroix

Your solution is somehow complicated and provides a wrong answer. Why not prefer

```sage: solve(sin(x^2)/x^3 == 0, x, algorithm="sympy")
Complement(ConditionSet(x, Eq(sin(x**2), 0), Complexes), FiniteSet(0))
```

I think the syntax simplicity can be judged in the later stages, I do want to ask are you able to obtain the answer from this? because I can't seem to be getting this work, even after a certain modification, such as additional functions for testing and new variable.

I don't understand your question. `Complement(ConditionSet(x, Eq(sin(x**2), 0), Complexes), FiniteSet(0))` is the answer. Not in a very nice form, but a valid answer.

Ahh my bad, I was trying to obtain a more numeric based answer, I did see that the second statement did resemble the solution

Ideally, it should be possible to convert it to the parametrized set `{sqrt(2*n*pi): n in ZZ \ {0}}`.

### comment:28 Changed 21 months ago by gh-Shlokatadistance

Yes exactly, from what I reckon the procedure is simply returning like a set, and I think that has to do with the way the conditions were defined. I think by providing a few other cases on the same will help us resolve this issue, something along the lines of

```def condition_set():
for x in solution_set # this can be solution set for our trigonometric functions
a = solve(the given problem)
ans = pi_set(a) # pi_set is the set of our pi value, based on a random value
return ans
```

Something along these lines , of course this is just a suggestion

### comment:29 Changed 19 months ago by mkoeppe

• Milestone changed from sage-9.1 to sage-9.2

### comment:30 Changed 13 months ago by mkoeppe

• Milestone changed from sage-9.2 to sage-9.3

### comment:31 Changed 8 months ago by mkoeppe

• Milestone changed from sage-9.3 to sage-9.4

Moving this ticket to 9.4, as it seems unlikely that it will be merged in 9.3, which is in the release candidate stage

### comment:32 Changed 5 months ago by mkoeppe

• Milestone changed from sage-9.4 to sage-9.5

Setting a new milestone for this ticket based on a cursory review.

Note: See TracTickets for help on using tickets.