log of NaN in RealField and ComplexField results in infinite loop

[pfili]

If you have a RealField or ComplexField NaN value and attempt to compute log, the fact that NaN is considered < 0 results in RealField.log calling ComplexField.log, but then ComplexField.log calls RealField.log again, but again on the values NaN for the absolute value. This results in an infinite loop. Example code for the code:

x = RealField()(NaN)
x.log() # Results in infinite loop

This patch fixes the log function in RealField and ComplexField to return NaN if fed a number which is NaN (in either the real or the imaginary coordinate).

CC: @sagetrac-atowsley

Component: numerical

Keywords: sage-days55

Author: Paul Fili, Adam Towsley

Reviewer: Benjamin Hutz

Merged: sage-5.13.beta3

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

[bhutz]

comment:3

A couple things

1. the patch needs a commit message

2. the patch name should be trac_15388_.patch

3. Please add a doctest to each log function to demonstrate the fix

4. lines 2063, 2064 which are commented out look like they can just be removed as they add no value to the code

[bhutz]

Reviewer: Ben Hutz

[bhutz]

comment:5

long doctest looks fine with this, so I'm happy with the code change, just make the minor changes suggested above.

Just to note, I had a few unrelated longtest failures on 5.12 that failed both with and without the patch.

sage -t --long /home/bhutz/sage/sage-5.12/devel/sage/sage/tests/cmdline.py  # 11 doctests failed
sage -t --long /home/bhutz/sage/sage-5.12/devel/sage/sage/calculus/desolvers.py  # 8 doctests failed
sage -t --long /home/bhutz/sage/sage-5.12/devel/sage/doc/en/constructions/calculus.rst  # 4 doctests failed
sage -t --long /home/bhutz/sage/sage-5.12/devel/sage/doc/en/prep/Quickstarts/Differential-Equations.rst  # 2 doctests failed
sage -t --long /home/bhutz/sage/sage-5.12/devel/sage/sage/misc/interpreter.py  # 1 doctest failed
sage -t --long /home/bhutz/sage/sage-5.12/devel/sage/sage/misc/trace.py  # 2 doctests failed
sage -t --long /home/bhutz/sage/sage-5.12/devel/sage/sage/tests/interrupt.pyx  # Time out

[pfili]

comment:7

It is probably best not to have an example involving NaN in the log's documentation, as it is such a basic function and this case is rather exceptional. (Although we still want it to work in case a log appears in someone's code after an NaN has already appeared, as happened to us when we found this bug.)

[pfili]

comment:8

We have made the changes Ben suggested, except for adding a doctest, which seems undesirable as noted above.

[bhutz]

comment:10

This looks good.

[jdemeyer]

Work Issues: doctest

[jdemeyer]

comment:11

Replying to @pfili:

Although we still want it to work in case a log appears in someone's code after an NaN has already appeared, as happened to us when we found this bug.

Which is exactly why a doctest is needed.

[pfili]

comment:12

I have added a doctest that explains the behavior for NaN.

[bhutz]

comment:13

Ok. This still looks fine and now we have the doctests.

[jdemeyer]

comment:14

You should add a meaningful commit message (use hg qrefresh -e for that)

[jdemeyer]

Changed work issues from doctest to none

[pfili]

Fixes the behavior of log(NaN) in real and complex fields

[pfili]

comment:15

Attachment: trac_15388_nan_fix.patch.gz

Commit message has been added.

[bhutz]

comment:16

ok. still looks good, but with a commit message this time.

[jdemeyer]

Merged: sage-5.13.beta3

[jdemeyer]

Changed reviewer from Ben Hutz to Benjamin Hutz