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
added functionality to Q.real and Q.imaginary and fixed issue:2324 #1156
Changes from 9 commits
0707cec
3f6e26e
f9c4904
f92f3e4
7f6716b
93a0b8a
5a1e114
f30988a
9a57cd2
9bb61ed
b0edda5
c870c5c
146be6c
fb1b7f1
0c990c9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ | |
""" | ||
from sympy.assumptions import Q, ask | ||
from sympy.assumptions.handlers import CommonHandler | ||
from sympy import I | ||
|
||
|
||
class AskIntegerHandler(CommonHandler): | ||
|
@@ -191,7 +192,7 @@ class AskRealHandler(CommonHandler): | |
|
||
@staticmethod | ||
def _number(expr, assumptions): | ||
return not expr.as_real_imag()[1] | ||
return not expr.as_real_imag()[1].evalf() | ||
|
||
@staticmethod | ||
def Add(expr, assumptions): | ||
|
@@ -230,21 +231,32 @@ def Pow(expr, assumptions): | |
Positive**Real -> Real | ||
Real**(Integer/Even) -> Real if base is nonnegative | ||
Real**(Integer/Odd) -> Real | ||
Real**Imaginary -> ? | ||
Imaginary**Real -> ? | ||
Real**Real -> ? | ||
""" | ||
if expr.is_number: | ||
return AskRealHandler._number(expr, assumptions) | ||
if ask(Q.real(expr.base), assumptions): | ||
if ask(Q.integer(expr.exp), assumptions): | ||
return True | ||
elif expr.exp.is_Rational: | ||
if (expr.exp.q % 2 == 0): | ||
return ask(Q.real(expr.base), assumptions) and \ | ||
not ask(Q.negative(expr.base), assumptions) | ||
else: | ||
if ask(Q.imaginary(expr.base), assumptions): | ||
if ask(Q.real(expr.exp), assumptions): | ||
if ask(Q.odd(expr.exp), assumptions): | ||
return False | ||
elif ask(Q.even(expr.exp), assumptions): | ||
return True | ||
elif ask(Q.real(expr.base), assumptions): | ||
if ask(Q.imaginary(expr.exp), assumptions): | ||
if expr.base == 1 or expr.base == -1: | ||
return True | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this line is not covered with a test There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @smichr they are covered in the tests 1. ask(Q.real(I3)) and 2. ask(@.real(I2)) etc There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was basing my comment on a coverage report. So unless I mistook this line for another, at the time I wrote the comment, it wasn't covered. This might mean that an expression you thought you were testing auto-evaluated to something else and didn't hit the line of code you thought it would. Perhaps ask(Q.real(Pow(I, 3, evaluate=False))) is necessary, for example. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's for the lines 248 and 249, they are already been covered in 238 and 239, I have made necessary changes and removed unwanted code |
||
elif ask(Q.real(expr.exp), assumptions): | ||
if ask(Q.positive(expr.base), assumptions): | ||
if expr.exp.is_Rational and \ | ||
ask(Q.even(expr.exp.q), assumptions): | ||
return ask(Q.positive(expr.base), assumptions) | ||
elif ask(Q.integer(expr.exp), assumptions): | ||
return True | ||
elif ask(Q.positive(expr.base), assumptions): | ||
return True | ||
elif ask(Q.negative(expr.base), assumptions): | ||
return False | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this line is not covered with a test |
||
|
||
@staticmethod | ||
def Rational(expr, assumptions): | ||
|
@@ -422,7 +434,7 @@ class AskImaginaryHandler(CommonHandler): | |
@staticmethod | ||
def _number(expr, assumptions): | ||
# helper method | ||
return not expr.as_real_imag()[0] | ||
return not expr.as_real_imag()[0].evalf() | ||
|
||
@staticmethod | ||
def Add(expr, assumptions): | ||
|
@@ -468,7 +480,38 @@ def Mul(expr, assumptions): | |
return False | ||
return result | ||
|
||
Pow = Add | ||
@staticmethod | ||
def Pow(expr, assumptions): | ||
""" | ||
Imaginary**integer -> Imaginary if integer % 2 == 1 | ||
Imaginary**integer -> real if integer % 2 == 0 | ||
Imaginary**Imaginary -> ? | ||
Imaginary**Real -> ? | ||
""" | ||
if expr.is_number: | ||
return AskImaginaryHandler._number(expr, assumptions) | ||
if ask(Q.imaginary(expr.base), assumptions): | ||
if ask(Q.real(expr.exp), assumptions): | ||
if ask(Q.odd(expr.exp), assumptions): | ||
return True | ||
elif ask(Q.even(expr.exp), assumptions): | ||
return False | ||
elif ask(Q.real(expr.base), assumptions): | ||
if ask(Q.imaginary(expr.exp), assumptions): | ||
if expr.base == 1 or expr.base == -1: | ||
return False | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not covered with a test |
||
elif ask(Q.real(expr.exp), assumptions): | ||
if expr.exp.is_Rational and \ | ||
ask(Q.even(expr.exp.q), assumptions): | ||
return ask(Q.negative(expr.base),assumptions) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not covered with a test |
||
elif ask(Q.integer(expr.exp), assumptions): | ||
return False | ||
elif ask(Q.positive(expr.base), assumptions): | ||
return False | ||
elif ask(Q.negative(expr.base), assumptions): | ||
return True | ||
|
||
|
||
|
||
@staticmethod | ||
def Number(expr, assumptions): | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why was this change made? Can you evalf to a lower precision, say 2, and still get the answer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@smichr sorry for the late response, could you please explain what you mean by lower precision. Just to explain why I have added evalf is for the case of ask(Q.real((-1)**I))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are you just seeing if the imaginary part is zero or not? might .evalf(2) work? evalf() will compute to 15 digits, or something like that. So if the answer involves a complicated imaginary part there is no need to calculate it to high precision just to learn that it is not zero.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes I am checking whether its imaginary or not, sure I will reduce the precision to 2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something you have to be careful of is expressions that may not be zero but have no significance in their evaluation. It's been a while, but if you look at my work in round or int (and commits added around that time by me) you will see me returning None when the answer is calculated without precision. I think I put some notes in the commit messages about that. Have you used gitk to browse the commit log? It's really nice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think so. Commits dealing with equals, too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's a pertinent piece of code:
A problem with this, however, is that for
q=(-1)**I
q.is_real comes back as False. That's a bug. Otherwise, the idea from then, i =
part that tries to be careful about reaching a conclusion. If the precision is 1 then you don't know what the number is and in the assumption system, you should return None in that case, not True or False as is done in the above.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You asked whether it makes sense to return None...not if you can figure out what sort of number you are working with. But not all expressions that are zero can be, with a given piece of code, be proved to be zero. And we shouldn't use a numerical approximation to guess that it is zero. It might just be an expression with a very small value. In the code above, if we know something is a number, we try to evaluate it to 2 digits of precision. But if we only get 1 -- and it doesn't matter if we had attempted more, we would still get 1 -- it's because the evaluation system couldn't be sure that that value was or wasn't zero. For most non-pathological cases, the above is going to return True or False. It's going to have trouble when it gets an expression where the imaginary part is a zero that can't be proved to be so.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the information, I will go through my code and merge the pull made by you, test it and will get back to you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
btw, issue 3068 has some examples where difficulties in evalf'ing are encountered.
http://code.google.com/p/sympy/issues/detail?id=3068