Skip to content
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

Doctest fix for: Make atan2(0,0) return NaN #21614

Closed
rwst opened this issue Sep 30, 2016 · 13 comments
Closed

Doctest fix for: Make atan2(0,0) return NaN #21614

rwst opened this issue Sep 30, 2016 · 13 comments

Comments

@rwst
Copy link

rwst commented Sep 30, 2016

Same with tan and imag:

sage: real(sqrt(sin(x)))
sqrt(abs(sin(x)))*cos(1/2*arctan2(cos(real_part(x))*sinh(imag_part(x)), cosh(imag_part(x))*sin(real_part(x))))
sage: _.subs(x==0)
...
RuntimeError: arctan2_eval(): arctan2(0,0) encountered

SymPy expands similarly but gives NaN on substitution instead of an exception.

This all would not be of concern if not 3d plotting would likely need real/imag parts of a function, their workaround real(...,hold=True) works perfectly but this is not the general solution that is needed.

One solution would be to return NaN instead of throwing up.

Depends on #21623

Component: symbolics

Author: Ralf Stephan

Branch/Commit: fda5183

Reviewer: Jeroen Demeyer

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

@rwst rwst added this to the sage-7.4 milestone Sep 30, 2016
@rwst

This comment has been minimized.

@rwst
Copy link
Author

rwst commented Oct 1, 2016

Dependencies: pynac-0.6.91

@mezzarobba
Copy link
Member

comment:2

I'm a bit uncomfortable with the idea of returning NaN for exact (as opposed to floating-point) input... But more importantly, my feeling is that we need to decide and document once and for all how symbolic functions should behave depending on the type, parent and value of their argument(s), based on the many examples you collected, instead of coming up with ad hoc solutions every time.

@rwst
Copy link
Author

rwst commented Oct 1, 2016

comment:3

Then make it known in that documentation that I just decided that symbolic NaN is to be returned whenever the result is not defined, not even complex infinity. That pertains to expressions, not relations. It could also be returned with 0/0.

I think the SymPy folks have thought more about their ad hoc solution, and if I copy it it's not ad hoc, either.

Not a Number.

This serves as a place holder for numeric values that are indeterminate.
Most operations on NaN, produce another NaN.  Most indeterminate forms,
such as ``0/0`` or ``oo - oo` produce NaN.  Two exceptions are ``0**0``
and ``oo**0``, which all produce ``1`` (this is consistent with Python's
float).

NaN is loosely related to floating point nan, which is defined in the
IEEE 754 floating point standard, and corresponds to the Python
``float('nan')``.  Differences are noted below.

NaN is mathematically not equal to anything else, even NaN itself.  This
explains the initially counter-intuitive results with ``Eq`` and ``==`` in
the examples below.

NaN is not comparable so inequalities raise a TypeError.  This is in
constrast with floating point nan where all inequalities are false.

@rwst
Copy link
Author

rwst commented Oct 1, 2016

Branch: u/rws/make_atan2_0_0__return_nan

@rwst
Copy link
Author

rwst commented Oct 2, 2016

Commit: fda5183

@rwst
Copy link
Author

rwst commented Oct 2, 2016

New commits:

fda518321614: fix some real/imag part expansions

@rwst
Copy link
Author

rwst commented Oct 2, 2016

Changed dependencies from pynac-0.6.91 to #21623

@mezzarobba
Copy link
Member

comment:6

Replying to @rwst:

Then make it known in that documentation that I just decided that symbolic NaN is to be returned whenever the result is not defined, not even complex infinity. That pertains to expressions, not relations. It could also be returned with 0/0.

Sorry if my comment came off as a criticism, that was not the intent. What I'm trying to say is that I find this kind of tickets hard to review, because I'm unable to tell if the change is actually an improvement (because it fixes a perceived issue) or a regression (typically because it introduces an inconsistency across different parts of Sage).

@rwst
Copy link
Author

rwst commented Oct 3, 2016

comment:7

It is very hard to keep an overview of all details that can appear in "symbolics" (minus algebra). There are some bright people leading the SymPy effort. I think it's a good start to follow them.

@rwst
Copy link
Author

rwst commented Nov 6, 2016

Author: Ralf Stephan

@rwst rwst changed the title Make atan2(0,0) return NaN Doctest fix for: Make atan2(0,0) return NaN Nov 6, 2016
@jdemeyer
Copy link

Reviewer: Jeroen Demeyer

@vbraun
Copy link
Member

vbraun commented Nov 12, 2016

Changed branch from u/rws/make_atan2_0_0__return_nan to fda5183

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

4 participants