[boost] [complex] Feedback and Potential Review Manager

Subject: [boost] [complex] Feedback and Potential Review Manager
From: Christopher Kormanyos (e_float_at_[hidden])
Date: 2012-04-29 17:47:24

Hi Matthieu,


I would like to provide a detailed feedback on
the proposed Boost.Complex.

I am terribly sorry about the late response on this,
as I was not doing well in winter when this topic was
more active.

First of all, great work with this, Matthieu.
I believe the community desperately requires this
facility.


Boost.Complex appears to have two main goals:
A) Provide acceleration for pure imaginary numbers.
B) Allow complex to be used with user-defined types.

For goal B), I have briefly tested parts of the Boost.Complex
code with the cpp_dec_float multiprecision data type
proposed for Boost.Multiprecision by John Maddock.

My Tests include:
* Test some elementary transcendental functions.
* Test the overall package via computation of orthogonal polynomials.* My test file (complex.cpp) is included in this e-mail.
* Visual Studio 2010, SP1.
* Proposed Boost.Multiprecision and boost trunk.

Remarks:
1) Various functions use macros like M_PI_2.
These macros do not provide enough precision
for a multiprecision type. And they will be inefficient
for some fixed-point types. Perhaps you need to
"bite the bullet" and use boost::math::constants.
(Is M_PI_2 even part of C++, or is it only lurking
around in <math.h> of GCC?)

2) Ditto for the LN_10 and LN_2 constants.
Does boost::math::constants support ln_ten()?
I do know that it has ln_two().

3) I disagree with
    explicit complex(const T& re = T(0.), const T& im = T(0.))
I believe this constructor should be non-explicit.
See ISO/IEC 14882:2011, Section 26.4.2.

4) I would really like to see global operators for
seamless interaction of complex<T> with integer PODs.
The proposed code of boost::complex forces explicit construction
from integer "all the way" to boost::complex<T> just to, for example,
multiply with 2. This is extremely costly as most multiprecision
or fixed-point types have specially optimized routines
for multiplication with integer PODs.

Perhaps something like this would be good

template<typename T>
complex<T> operator*(const complex<T>& z, int n)
{
  T re(z.real());
  T im(z.imag());
  re *= n;
  im *= n;
  return complex<T>(re, im);
}

5) In the exp() function, polar() can not be resolved
without explicit template parameter.
Instead of this:

template<typename T>
inline complex<T> exp(const complex<T>& x)
{
    return polar(exp(x.real()), x.imag());
}

I needed this:

template<typename T>
inline complex<T> exp(const complex<T>& x)
{
    return polar<T>(exp(x.real()), x.imag());
}

I don't know who's right here, your code or the compiler.

6) The tanh() function and some other functions
compute the exponent twice. By that I mean both
exp(x) as well as exp(-x). I believe that division would
be faster if exp(-x) were computed with (1 / exp(x)).
Or is there an issue with branch cuts that mandates the
potentially redundant calculation?

7) I hope to find time to test this code further with
a tiny fixed-point class. But I might be too busy for
that extra work.

REVIEW PROGRESS:
I may potentially be qualified for review manager.
I do know the math and the code. I am also well-versed
with all aspects of software review, assessment and
audit situations.

BUTTT!!! I'm not as good at C++ as some of you folks
and I'm a real beginner at boost.

Nonetheless, If nobody better steps forward, I could
potentially manage the review of Boost.Complex.

Best regards, Chris.