Re: [boost] [complex] Feedback and Potential Review Manager

Subject: Re: [boost] [complex] Feedback and Potential Review Manager
From: Matthieu Schaller (matthieu.schaller_at_[hidden])
Date: 2012-05-01 05:48:44

Hi Chris,
> First of all, great work with this, Matthieu.
> I believe the community desperately requires this
> facility.
Thanks for your support.
> 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.
Interesting tests. I am glad you could run the code without obvious 
problems.
> 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().
You are right, M_PI_2 is not standard although present in gcc, icc and 
Visual C++. I will move the constants to boost::math::constant. LN_10 
exists under the name one_div_log10_e as Paul Bristow mentioned.
> 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.
Agreed, I should stick to the standard.
However, I will keep the imaginary constructor explicit. I do not want 
spurious Real -> Imaginary conversions.
> 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.
Great idea. I have done it for pow() but it is true that other operators 
may benefit from this. This optimization will only be available to POD 
types but T==double is probably the primary concern.
> 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.

I didn't get any error message but I will explicitly add the type for 
compatibility.

> 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?

I will have a look. Division is faster than exponentiation but the 
compiler may do smart things behind our back. I will try both versions. 
Thanks for this suggestion

> 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.
This would be interesting to test. For what it is worth, I have tested 
the class with boost::rational but this does not allow to test the more 
complicated functions.

> Paul Bristow:
> PS Might Matthieu be qualified as a review manager for Boost.Multiprecision?  Boost badly needs this
> reviewed soon (and preferably accepted!)
I am afraid I am not qualified for software assessment. Furthermore, I 
have never taken part to a boost review.

Regards,

Matthieu

-- 
Matthieu Schaller