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
Fix atan2(a, a) #1808
base: master
Are you sure you want to change the base?
Fix atan2(a, a) #1808
Changes from all commits
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 |
---|---|---|
|
@@ -522,6 +522,13 @@ RCP<const Basic> sign(const RCP<const Basic> &arg) | |
return mul(s, | ||
make_rcp<const Sign>(Mul::from_dict(one, std::move(dict)))); | ||
} | ||
if (is_a<Pow>(*arg)) { | ||
RCP<const Pow> pow_arg = rcp_static_cast<const Pow>(arg); | ||
RCP<const Basic> s = sign(pow_arg->get_base()); | ||
if (not is_a<Sign>(*s) and not eq(*s, *pow_arg->get_base())) { | ||
return sign(pow(s, pow_arg->get_exp())); | ||
} | ||
} | ||
return make_rcp<const Sign>(arg); | ||
} | ||
|
||
|
@@ -1571,54 +1578,23 @@ RCP<const Basic> ATan2::create(const RCP<const Basic> &a, | |
RCP<const Basic> atan2(const RCP<const Basic> &num, const RCP<const Basic> &den) | ||
{ | ||
if (eq(*num, *zero)) { | ||
if (is_a_Number(*den)) { | ||
RCP<const Number> den_new = rcp_static_cast<const Number>(den); | ||
if (den_new->is_negative()) | ||
return pi; | ||
else if (den_new->is_positive()) | ||
return zero; | ||
else { | ||
return Nan; | ||
} | ||
if (eq(*den, *zero)) { | ||
return Nan; | ||
} | ||
return mul(div(pi, im2), sub(sign(den), one)); | ||
} else if (eq(*den, *zero)) { | ||
if (is_a_Number(*num)) { | ||
RCP<const Number> num_new = rcp_static_cast<const Number>(num); | ||
if (num_new->is_negative()) | ||
return div(pi, im2); | ||
else | ||
return div(pi, i2); | ||
} | ||
return mul(div(pi, i2), sign(num)); | ||
} | ||
RCP<const Basic> index; | ||
bool b = inverse_lookup(inverse_tct, div(num, den), outArg(index)); | ||
RCP<const Basic> divided = div(num, den); | ||
RCP<const Basic> index_b; | ||
bool b = inverse_lookup(inverse_tct, divided, outArg(index_b)); | ||
if (b) { | ||
// Ideally the answer should depend on the signs of `num` and `den` | ||
// Currently is_positive() and is_negative() is not implemented for | ||
// types other than `Number` | ||
// Hence this will give exact answers in case when num and den are | ||
// numbers in SymEngine sense and when num and den are positive. | ||
// for the remaining cases in which we just return the value from | ||
// the lookup table. | ||
// TODO: update once is_positive() and is_negative() is implemented | ||
// in `Basic` | ||
if (is_a_Number(*den) and is_a_Number(*num)) { | ||
RCP<const Number> den_new = rcp_static_cast<const Number>(den); | ||
RCP<const Number> num_new = rcp_static_cast<const Number>(num); | ||
|
||
if (den_new->is_positive()) { | ||
return div(pi, index); | ||
} else if (den_new->is_negative()) { | ||
if (num_new->is_negative()) { | ||
return sub(div(pi, index), pi); | ||
} else { | ||
return add(div(pi, index), pi); | ||
} | ||
} else { | ||
return div(pi, index); | ||
} | ||
SYMENGINE_ASSERT(is_a_Number(*index_b)); | ||
RCP<const Number> index = rcp_static_cast<const Number>(index_b); | ||
if (index->is_positive()) { | ||
return add(div(pi, index), mul(div(pi, i2), sub(sign(den), one))); | ||
} else { | ||
return div(pi, index); | ||
return sub(div(pi, index), mul(div(pi, i2), sub(sign(den), one))); | ||
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. Are complex values of |
||
} | ||
} else { | ||
return make_rcp<const ATan2>(num, den); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2218,7 +2218,7 @@ TEST_CASE("Atan2: functions", "[functions]") | |
REQUIRE(eq(*r1, *r2)); | ||
|
||
r1 = atan2(add(one, sqrt(i2)), im1); | ||
r2 = div(mul(pi, i3), integer(-8)); | ||
r2 = div(mul(pi, i5), integer(8)); | ||
REQUIRE(eq(*r1, *r2)); | ||
|
||
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. We should have a test for 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. And also |
||
r1 = atan2(sub(sqrt(i2), one), i1); | ||
|
@@ -2230,7 +2230,7 @@ TEST_CASE("Atan2: functions", "[functions]") | |
REQUIRE(eq(*r1, *r2)); | ||
|
||
r1 = atan2(sqrt(add(i5, mul(i2, sqrt(i5)))), im1); | ||
r2 = div(mul(pi, im2), i5); | ||
r2 = div(mul(pi, i3), i5); | ||
REQUIRE(eq(*r1, *r2)); | ||
|
||
r1 = atan2(y, x)->diff(x); | ||
|
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 don't think this is correct. If we do
atan2(x, 0)
this will becomepi/2 * sign(x)
, but for x = 0 this is should beNaN
. Sympy keeps this case unevaluated asatan2(x, 0)
.