Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sagetrac-cwitty mannequin opened this issue Mar 20, 2008 · 30 comments
Open

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

sagetrac-cwitty mannequin opened this issue Mar 20, 2008 · 30 comments

Comments

@sagetrac-cwitty
Copy link
Mannequin

sagetrac-cwitty mannequin commented Mar 20, 2008

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

Component: calculus

Stopgaps: todo

Author: Matt Torrence

Branch/Commit: u/gh-Torrencem/2617_solve_check_domain @ 2cc6fd5

Reviewer: Vincent Delecroix

Issue created by migration from https://trac.sagemath.org/ticket/2617

@sagetrac-cwitty sagetrac-cwitty mannequin added this to the sage-5.11 milestone Mar 20, 2008
@sagetrac-mabshoff
Copy link
Mannequin

sagetrac-mabshoff mannequin commented Apr 12, 2008

comment:1

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

@kcrisman
Copy link
Member

comment:2

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

@robert-marik
Copy link
Mannequin

robert-marik mannequin commented Oct 8, 2009

comment:3

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.

@kcrisman
Copy link
Member

kcrisman commented Oct 8, 2009

comment:4

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.

@kcrisman
Copy link
Member

comment:5

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.

@robert-marik
Copy link
Mannequin

robert-marik mannequin commented Dec 22, 2009

comment:6

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

  1. Takes only explicit solutions

  2. 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?

@kcrisman
Copy link
Member

comment:7

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()))$

@jdemeyer jdemeyer modified the milestones: sage-5.11, sage-5.12 Aug 13, 2013
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.1, sage-6.2 Jan 30, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.2, sage-6.3 May 6, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.3, sage-6.4 Aug 10, 2014
@sagetrac-jakobkroeker
Copy link
Mannequin

sagetrac-jakobkroeker mannequin commented Aug 19, 2015

Stopgaps: todo

@sagetrac-zonova
Copy link
Mannequin

sagetrac-zonova mannequin commented May 20, 2016

comment:14

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?

@kcrisman
Copy link
Member

comment:15

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!

@sagetrac-zonova
Copy link
Mannequin

sagetrac-zonova mannequin commented Nov 27, 2016

comment:16

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?

@kcrisman
Copy link
Member

comment:17

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.

@Torrencem
Copy link
Mannequin

Torrencem mannequin commented Jul 7, 2019

Commit: 2cc6fd5

@videlec
Copy link
Contributor

videlec commented Feb 22, 2020

comment:20

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

@videlec

This comment has been minimized.

@videlec
Copy link
Contributor

videlec commented Feb 22, 2020

comment:22

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

@videlec
Copy link
Contributor

videlec commented Feb 22, 2020

Reviewer: Vincent Delecroix

@varenyamBakshi
Copy link
Mannequin

varenyamBakshi mannequin commented Feb 22, 2020

comment:23

Replying to @videlec:

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.

@Shlokatadistance
Copy link
Mannequin

Shlokatadistance mannequin commented Mar 2, 2020

comment:24

Replying to @videlec:

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.

@videlec
Copy link
Contributor

videlec commented Mar 2, 2020

comment:25

Replying to @Shlokatadistance:

Replying to @videlec:

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.

@Shlokatadistance
Copy link
Mannequin

Shlokatadistance mannequin commented Mar 2, 2020

comment:26

Replying to @videlec:

Replying to @Shlokatadistance:

Replying to @videlec:

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

@videlec
Copy link
Contributor

videlec commented Mar 2, 2020

comment:27

Replying to @Shlokatadistance:

Replying to @videlec:

Replying to @Shlokatadistance:

Replying to @videlec:

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}}.

@Shlokatadistance
Copy link
Mannequin

Shlokatadistance mannequin commented Mar 21, 2020

comment:28

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

@mkoeppe mkoeppe modified the milestones: sage-9.1, sage-9.2 May 7, 2020
@mkoeppe mkoeppe modified the milestones: sage-9.2, sage-9.3 Oct 24, 2020
@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 2, 2021

comment:31

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

@mkoeppe mkoeppe modified the milestones: sage-9.3, sage-9.4 Apr 2, 2021
@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 19, 2021

comment:32

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

@mkoeppe mkoeppe modified the milestones: sage-9.4, sage-9.5 Jul 19, 2021
@mkoeppe
Copy link
Contributor

mkoeppe commented Dec 18, 2021

comment:33

Stalled in needs_review or needs_info; likely won't make it into Sage 9.5.

@mkoeppe mkoeppe modified the milestones: sage-9.5, sage-9.6 Dec 18, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.6, sage-9.7 May 3, 2022
@mkoeppe mkoeppe modified the milestones: sage-9.7, sage-9.8 Sep 19, 2022
@mkoeppe mkoeppe removed this from the sage-9.8 milestone Jan 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants