Re: [libstdc++, PATCH] PR libstdc++/83140 - assoc_legendre returns negated value when m is odd.

[Ed Smith-Rowland <3dw4rd at verizon dot net>]

On 05/09/2018 05:30 AM, Jonathan Wakely wrote:
> On 07/05/18 12:39 -0400, Ed Smith-Rowland wrote:
> > All,
> >
> > We were using a different convention for P_l^m assoc_legendre(int l, 
> > int m, FloatTp x)
> >
> >  - the so-called Condon-Shortley convention which includes (-1)^m.  
> > This unfortunately is common.
> >
> > This factor is taken out to match the standard.  The underlying 
> > __detail code has an arg that allows you to flip this
> >
> > - mostly to highlight the subtle difference.
> >
> > The related sph_legendre is unaffected by this (our impl and the 
> > standard include the C-S phase).
> >
> > OK for trunk and branches?
> >
> > Ed
>
> > 2018-05-07  Edward Smith-Rowland  <3dw4rd@verizon.net>
> >
> >     PR libstdc++/83140 - assoc_legendre returns negated value when m 
> > is odd
> >     * include/tr1/legendre_function.tcc (__assoc_legendre_p): Add 
> > __phase
> >     argument defaulted to +1.  Doxy comments on same.
> >     * testsuite/special_functions/02_assoc_legendre/
> >     check_assoc_legendre.cc: Regen.
> >     * testsuite/tr1/5_numerical_facilities/special_functions/
> >     02_assoc_legendre/check_tr1_assoc_legendre.cc: Regen.
>
> > Index: include/tr1/legendre_function.tcc
> > ===================================================================
> > --- include/tr1/legendre_function.tcc    (revision 259973)
> > +++ include/tr1/legendre_function.tcc    (working copy)
> > @@ -65,7 +65,7 @@
> >   namespace __detail
> >   {
> >     /**
> > -     *   @brief  Return the Legendre polynomial by recursion on order
> > +     *   @brief  Return the Legendre polynomial by recursion on degree
> >      *           @f$ l @f$.
> >      *
> >      *   The Legendre function of @f$ l @f$ and @f$ x @f$,
> > @@ -74,7 +74,7 @@
> >      *     P_l(x) = \frac{1}{2^l l!}\frac{d^l}{dx^l}(x^2 - 1)^{l}
> >      *   @f]
> >      *
> > -     *   @param  l  The order of the Legendre polynomial.  @f$l >= 
> > 0@f$.
> > +     *   @param  l  The degree of the Legendre polynomial. @f$l >= 
> > 0@f$.
> >      *   @param  x  The argument of the Legendre polynomial. @f$|x| 
> > <= 1@f$.
> >      */
> >     template<typename _Tp>
> > @@ -127,16 +127,19 @@
> >      *     P_l^m(x) = (1 - x^2)^{m/2}\frac{d^m}{dx^m}P_l(x)
> >      *   @f]
> >      *
> > -     *   @param  l  The order of the associated Legendre function.
> > +     *   @param  l  The degree of the associated Legendre function.
> >      *              @f$ l >= 0 @f$.
> >      *   @param  m  The order of the associated Legendre function.
> >      *              @f$ m <= l @f$.
> >      *   @param  x  The argument of the associated Legendre function.
> >      *              @f$ |x| <= 1 @f$.
> > +     *   @param  phase  The phase of the associated Legendre function.
> > +     *                  Use -1 for the Condon-Shortley phase 
> > convention.
> >      */
> >     template<typename _Tp>
> >     _Tp
> > -    __assoc_legendre_p(unsigned int __l, unsigned int __m, _Tp __x)
> > +    __assoc_legendre_p(unsigned int __l, unsigned int __m, _Tp __x,
> > +               _Tp __phase = _Tp{+1})
>
> This list-init isn't valid for C++98 i.e. when used via <tr1/cmath>.
> GCC seems to allow it, but Clang won't.
>
> We could consider dropping the TR1 support, and just provide these
> functions for ISO/IEC 29124:2010 in C++11 (or later) and for C++17.
> But that decision should be taken separately, and should only happen
> on trunk anyway so we need to use _Tp(+1) here.
>
> OK for trunk with _Tp(+1) instead of _Tp{+1}.
>
> Do we want to change the result of these functions on the branches?
> How likely is it that changing it will affect somebody's calcuations
> in a way that they don't expect from a minor release on a branch?
Here are the files applied for 260115.

As to backporting...  I did a Google and found rather more activity 
around these functions - especially legendre - than I remembered last 
time I searched.  I thought these functions were languishing, but 
apparently not.  Still low pings on ellint_3.

I *would* like to change branch 8 because it's just out.

I think I should curb my enthusiasm for branches 7 and 6.

Ed.

Attachment: CL_pr83140
Description: Text document

Attachment: patch_pr83140
Description: Text document