Opened 13 years ago
Last modified 2 weeks ago
#2617 needs_info defect
solve() can return undefined points as "solutions"
Reported by: | cwitty | Owned by: | was |
---|---|---|---|
Priority: | major | Milestone: | sage-9.4 |
Component: | calculus | Keywords: | |
Cc: | Merged in: | ||
Authors: | Matt Torrence | Reviewers: | Vincent Delecroix |
Report Upstream: | N/A | Work issues: | |
Branch: | u/gh-Torrencem/2617_solve_check_domain (Commits, GitHub, GitLab) | Commit: | 2cc6fd5ac64ec0a86dea43286261b8135f303d4c |
Dependencies: | Stopgaps: | todo |
Description (last modified by )
Consider the following examples (reported by Dean Moore here: http://groups.google.com/group/sage-support/browse_thread/thread/5555e780a76b3343#)
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.)
Change History (31)
comment:1 Changed 13 years ago by
- Priority changed from major to critical
comment:2 Changed 12 years ago by
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
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
Replying to 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"
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 11 years ago by
- Report Upstream set to N/A
Replying to kcrisman:
Replying to 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"
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 11 years ago by
I had an idea to introduce new option to solve, which
- Takes only explicit solutions
- 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 11 years ago by
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
- Priority changed from critical to major
comment:9 Changed 8 years ago by
- Milestone changed from sage-5.11 to sage-5.12
comment:10 Changed 7 years ago by
- Milestone changed from sage-6.1 to sage-6.2
comment:11 Changed 7 years ago by
- Milestone changed from sage-6.2 to sage-6.3
comment:12 Changed 7 years ago by
- Milestone changed from sage-6.3 to sage-6.4
comment:13 Changed 6 years ago by
- Stopgaps set to todo
comment:14 Changed 5 years ago by
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 5 years ago by
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 4 years ago by
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 4 years ago by
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 22 months ago by
- 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)
See the added documentation example:
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 16 months ago by
- Milestone changed from sage-8.9 to sage-9.1
Ticket retargeted after milestone closed
comment:20 Changed 14 months ago by
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[0] 1019 if isinstance(x, (list, tuple)): 1020 for i in x: IndexError: tuple index out of range
comment:21 Changed 14 months ago by
- Description modified (diff)
comment:22 follow-ups: ↓ 23 ↓ 24 Changed 14 months ago by
- 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 14 months ago by
Replying to 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))
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 14 months ago by
Replying to 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.
comment:25 in reply to: ↑ 24 ; follow-up: ↓ 26 Changed 14 months ago by
Replying to gh-Shlokatadistance:
Replying to 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 14 months ago by
Replying to vdelecroix:
Replying to gh-Shlokatadistance:
Replying to 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
comment:27 in reply to: ↑ 26 Changed 14 months ago by
Replying to gh-Shlokatadistance:
Replying to vdelecroix:
Replying to gh-Shlokatadistance:
Replying to 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 13 months ago by
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 12 months ago by
- Milestone changed from sage-9.1 to sage-9.2
comment:30 Changed 6 months ago by
- Milestone changed from sage-9.2 to sage-9.3
comment:31 Changed 2 weeks ago by
- 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
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