(EXPT <not-a-double> <complex-double>) uses intermediate single-float values

Bug #741564 reported by Nikodemus Siivola on 2011-03-24
This bug affects 1 person
Affects Status Importance Assigned to Milestone

Bug Description

Error detected by Maxima's test-suite, analysis by Lutz Euler below.

Maxima's calculation of the zeta function with the argument of the
test case is done using the lisp function "float-zeta", defined in
maxima-5.23.1/src/combin.lisp. The formula contains a sum of several
terms each containing a quotient of something by "expt" of a small
integer and the argument to the zeta function, the latter converted
to complex double-float. These "expt"s are calculated directly using
SBCL's "expt". Here is a trace:

 Maxima 5.23.1 http://maxima.sourceforge.net
 using Lisp SBCL

 (%i1) to_lisp();

 Type (to-maxima) to restart, ($quit) to quit Maxima.

 MAXIMA> (trace expt)

 MAXIMA> (float-zeta #$(3+%i)$)
 0: (EXPT 4 3.0)
 0: EXPT returned 64.0
 0: (EXPT 2 #C(-2.0 -1.0))
 0: EXPT returned #C(0.19230972430417587 -0.15974031883619208)
 0: (EXPT #C(7.2421875 1.0) #C(1.25 0.5))
 0: EXPT returned #C(4.418528303829868 10.318276280305101)
 0: (EXPT 1 #C(3.0 1.0))
 0: EXPT returned #C(1.0 0.0)
 0: (EXPT -1 0)
 0: EXPT returned 1
 0: (EXPT -1 0)
 0: EXPT returned 1
 0: (EXPT 21 #C(3.0 1.0))
 0: EXPT returned #C(-9217.405235813078 897.5555997887233)
 0: (EXPT 2 #C(3.0 1.0))
 0: EXPT returned #C(6.1539112363389945 5.11169025143816)
 ... [many more calls to (expt <small int> #C(3.0 1.0))]
 #C(1.1072144060921958 -0.14829086482826961)

The test case expects the following result:

 #C(1.10721440843141 - 0.1482908671781754)

As a simple reduced test case without maxima we have:

 (expt 2 #C(3.0d0 1.0d0))

SBCL gives:

 #C(6.1539112363389945d0 5.11169025143816d0)

Clisp (2.48) gives:

 #C(6.153911210911777d0 5.111690210509078d0)

and Clisp with more precision (setf (system::long-float-digits) 500):

 (expt 2 #C(3.0l0 1.0l0))

The definition of "expt" is in sbcl/src/code/irrat.lisp.

(defun expt (base power)
 (number-dispatch ((base number) (power number))
   (((foreach fixnum (or bignum ratio) single-float double-float)
    (if (and (zerop base) (plusp (realpart power)))
        (* base power)
        (exp (* power (log base)))))

The calculation of (log base), base being an integer, yields a
single-float as required by the standard. For the purpose of "expt"
it may be a bug to use (log base) as the standard says in section "Rule of Float and Rational Contagion":

 "When rationals and floats are combined by a numerical function,
 the rational is first converted to a float of the same format."

For a quick test I replaced the calculation in the SBCL source as follows,

        (exp (* power (log (coerce base 'double-float))))

then compiled SBCL and maxima. This SBCL calculates:

 * (expt 2 #C(3.0d0 1.0d0))
 #C(6.153911210911775d0 5.111690210509077d0)

This seems to differ from Clisp's result only in the last bit of the
real and imaginary part each.

The zeta value calculated by maxima is then:

 (%i1) zeta (3 + %i), numer=true;
 (%o1) 1.107214408431409 - .1482908671781753 %i

The resulting maxima passes its test suite with no unexpected errors.

In case it matters, the above is under x86-64, except when using
SBCL 0.8.12, which is x86.

If "expt" indeed needs to be modified a correct fix seems a bit more
complicated as currently there are two clauses dealing with complex
exponents, both not distinguishing between single and double floats.
The standard seems to require to look at all argument types and to use
single-float calculations if all arguments are integer or rational or
single-float, be it real or complex, and to use double-float calculations
if at least one argument is double-float real or complex?

Also, the current implementation of "expt" allows (with a complex
exponent) bignums or ratios as the base that are too big or too small
to be represented as a single-float, provided their logarithm can
(and the exponent is such that the result fits in its type).
If one doesn't want to lose this property, a function for log of
ratios and integers returning a double-float would need to be defined.
This may be easy as "log" already contains the needed logic, only
unfortunately always coercing the result to single-float.

On the other hand I believe the standard does not require this, so
hopefully nobody depends on this accidental property. And, when the
exponent is real, bignum/ratio bases too large and too small don't work
anyway, currently.

On the third hand, as "log" (intentionally, seemingly) deals with bignums
and ratios that don't fit in a float, maybe "expt" should, too, even in
the case of a real exponent?

Moreover, "log" is implemented the way it is to be more exact in the case
that its argument is a ratio very near to one. This carries over to "expt":
The original SBCL calculates:

* (expt (/ (expt 2 64) (1+ (expt 2 64))) #c(10 10))
#C(1.0 -5.421011e-19)

whereas with the above hack we have the less exact

#C(1.0d0 0.0d0)

(Sorry, can't resist: Did you notice that "log" returns zero for rational
arguments very near to one if the integer length of numerator and
denominator differs by one?
(log (/ (expt 2 64) (1- (expt 2 64)))) -> 0.0
(log (/ (1+ (expt 2 64)) (expt 2 64))) -> 5.421011e-20
which result would be correct for the former ratio, too.)



tags: added: floating-point library
Lutz Euler (lutz-euler) wrote :

Here is a patch.

After thinking a lot more about what the standard may require and
looking around what other math functions do in SBCL when given mixed
single-precision/rational and double-precision arguments, I have come to
the conclusion that the way I want to fix this bug is indeed to convert
the BASE to DOUBLE-FLOAT or (COMPLEX DOUBLE-FLOAT), respectively, in
the cases in question, before taking the LOG. This does not overdo the
amount of changes and leads to the most consistent behaviour.

I believe that I have found all combinations of argument types of EXPT
that wrongly use the single-precision LOG for the BASE. The patch
should correct EXPT's behaviour for all of them.

For the details see the commit messages and the comments in the code.

The patch consists of two commits:

The first commit patches EXPT and adds tests.
I have tested that it compiles and passes SBCL's test suite under linux
on x86_64 and x86. Also, maxima's test suite passes with no unexpected
errors (only tested on x86_64).

The second commit adds a compile-time check to NUMBER-DISPATCH (more
I was prompted to do that when the NUMBER-DISPATCH form in EXPT, being
already quite complex, grew 50 percent more complex when I changed it
and when I immediately made an error there that didn't hinder the build,
that SBCL's test suite didn't catch but that did lead to lots of new
errors in maxima's test suite. The new check in GENERATE-NUMBER-DISPATCH
would have caught this type of error earlier.



tags: added: review

 status fixcommited

Applied the patches, thank you.

commit b90e13dea92ee66f06f66baf17c3e3e23c89575f
Author: Lutz Euler <email address hidden>
Date: Fri Jul 1 18:06:17 2011 +0200

    Make EXPT use double-precision throughout in more cases

    lp#741564 notes that a Maxima test case fails because the result of
    (EXPT <fixnum> <(complex double)>) is much less precise than expected.
    This is caused by EXPT using an intermediate single-float value here.

    This behaviour actually occurs for all the following combinations
    of argument types:

      (EXPT <(or rational single-float)> <(complex double-float)>)

      (EXPT <(or (complex rational) (complex single-float))>
            <(or (complex double-float) double-float)>)

    In all these cases the first step EXPT does is to calculate (LOG BASE)
    in single precision.

    Refine the type dispatch clauses in EXPT to separate these cases
    and coerce BASE to DOUBLE-FLOAT or (COMPLEX DOUBLE-FLOAT) there,
    as appropriate, before applying LOG. Add tests.

    Fixes lp#741564.

    Signed-off-by: Christophe Rhodes <email address hidden>

 status fixcommitted

> status fixcommited


Changed in sbcl:
status: Confirmed → Fix Committed
Changed in sbcl:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers