Updates:
Labels: -NeedsReview NeedsBetterPatch
Comment #17 on issue 1694 by smichr: solve has many issues with fractions
http://code.google.com/p/sympy/issues/detail?id=1694
A few comments:
1) I think having a separate function is a good idea. Now move all the
logic of
accepting or rejecting roots there. IMO, solve() should only have
"checksolution(...)" of "not checksolution(...)" and no other decisions;
all the
"agonizing about what to do" should appear in checksolution() and it should
emit a
True or False. For now, your checksolution could return True if neither of
the first
conditions are met. As it gets smarter it will be more discriminating about
what it
can say "True" and "False" to. Others may have other opinions.
2) I wouldn't make simplify() your first attempt at getting a zero. It
might be
smarter to try successively more costly methods: first just see if a simple
substitution works, then try expand, then try simplify. When you get to
testing some
polynomial solutions, symplifying them takes a long time and is not
necessary to get
a solution validated.
3) Your rejection of a cv_2 solution with negative exponent is too
restrictive: consider:
YOUR CHANGES
>>> from sympy import *
>>> var('x')
x
>>> solve((3 + x/(1 + x))**(-2),x)
[]
>>> ^Z
MASTER
>>> from sympy import *
>>> var('x')
x
>>> solve((3 + x/(1 + x))**(-2),x)
[-1]
And this really is a solution as a look at it's numerator and denominator
will show.
So you should perhaps be more careful about the criteria for concluding
that there is
no solution.
4) I don't know if you tried to make the GS_TRANSCENDENTAL change that is
noted in
solve (about checking only num or not), but maybe the same thing that you
did with
the P, Q section could be done there, too, and solutions tested.
5) Just as your code in solve, I would "PEP 8" (tinyurl.com/eapz7) the code
in the
solver tests by adding whitespace, especially after the argument's commas.
6) In comment 14 above, you asked about the 0 solution. What I was saying
was that it
was the *only* solution that is correct. All the others are unfiltered
incorrect
solutions. Perhaps modifications to your checksolution() will catch
that...if not,
the checker can be made smarter later and then those will be caught. As
long as a
change to sympy is moving us forward, I think it's ok if it's incomplete,
so even if
it doesn't catch those incorrect solutions (but catches others) I think it's
something to get added.
--
You received this message because you are listed in the owner
or CC fields of this issue, or because you starred this issue.
You may adjust your issue notification preferences at:
http://code.google.com/hosting/settings
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"sympy-issues" group.
To post to this group, send email to sympy-issues@googlegroups.com
To unsubscribe from this group, send email to
sympy-issues+unsubscr...@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/sympy-issues?hl=en
-~----------~----~----~----~------~----~------~--~---
