Re: [libstdc++, PATCH] PR libstdc++/83140 - assoc_legendre returns negated value when m is odd.
which always causes a failure: +FAIL: tr1/5_numerical_facilities/special_functions/02_assoc_legendre/check_value.cc (test for excess errors) Excess errors: Undefined first referenced symbol in file main/usr/lib/crt1.o ld: fatal: symbol referencing errors +UNRESOLVED: tr1/5_numerical_facilities/special_functions/02_assoc_legendre/chec k_value.cc compilation failed to produce executable Please fix. Rainer Done with 260168. Ed
Re: [libstdc++, PATCH] PR libstdc++/83140 - assoc_legendre returns negated value when m is odd.
Hi Ed, > On 05/10/2018 01:44 PM, Rainer Orth wrote: >> Hi Ed, >> > 2018-05-07 Edward Smith-Rowland <3dw...@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. >> something went badly wrong with the regeneration of this last file: both >> in your attached patch and in what you checked in, the file is empty. >> >> Rainer >> > I had hosed up the ChangeLog! > > CL change committed as 260149. > > New Log attached. you misunderstood: it's not the ChangeLog that had a problem (well; I don't think you you break long path names in there, but that's a nit at most), but the regeneration step went wrong and you checked an *empty testcase* into the repo -rw-r--r-- 1 ro gcc 0 May 11 15:09 libstdc++-v3/testsuite/tr1/5_numerical_facilities/special_functions/02_assoc_legendre/check_value.cc which always causes a failure: +FAIL: tr1/5_numerical_facilities/special_functions/02_assoc_legendre/check_value.cc (test for excess errors) Excess errors: Undefined first referenced symbol in file main/usr/lib/crt1.o ld: fatal: symbol referencing errors +UNRESOLVED: tr1/5_numerical_facilities/special_functions/02_assoc_legendre/chec k_value.cc compilation failed to produce executable Please fix. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [libstdc++, PATCH] PR libstdc++/83140 - assoc_legendre returns negated value when m is odd.
On 05/10/2018 01:44 PM, Rainer Orth wrote: Hi Ed, 2018-05-07 Edward Smith-Rowland <3dw...@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. something went badly wrong with the regeneration of this last file: both in your attached patch and in what you checked in, the file is empty. Rainer I had hosed up the ChangeLog! CL change committed as 260149. New Log attached. Sorry. 2018-05-10 Edward Smith-Rowland <3dw...@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_value.cc: Regen. * testsuite/tr1/5_numerical_facilities/special_functions/ 02_assoc_legendre/check_value.cc: Regen.
Re: [libstdc++, PATCH] PR libstdc++/83140 - assoc_legendre returns negated value when m is odd.
Hi Ed, >>> 2018-05-07 Edward Smith-Rowland <3dw...@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. something went badly wrong with the regeneration of this last file: both in your attached patch and in what you checked in, the file is empty. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Quo Vadis tr1? Was: [libstdc++, PATCH] PR libstdc++/83140 - assoc_legendre returns negated value when m is odd.
All,
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.
I am in favour of splitting new versions of the special functions out of
tr1 and into std/bits.
I personally am itching to use at least C++11 for implementation.
We have been defaulting to C++11 for, IIRC, two releases (Hence my
-Tp{+1} slip LOL).
I have a lot of work towards this that I wanted to get into 9 anyway.
This would end the last useful thing in tr1 that's not better
implemented elsewhere. There are certainly people using tr1. Can we
deprecate the whole namespace? That might be too noisy. I think we
should be done with maths bugs in Bugzilla pretty soon. We should do
whatever we decide relatively early in 9.
Ed
Also, I think 83566 should go into tr1 first because it's not a
signature change.
Re: [libstdc++, PATCH] PR libstdc++/83140 - assoc_legendre returns negated value when m is odd.
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 <3dw...@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
@@ -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
_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 .
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.
2018-05-10 Edward Smith-Rowland <3dw...@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 260114)
+++ 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
Re: [libstdc++, PATCH] PR libstdc++/83140 - assoc_legendre returns negated value when m is odd.
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 <3dw...@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
@@ -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
_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 .
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?
[libstdc++, PATCH] PR libstdc++/83140 - assoc_legendre returns negated value when m is odd.
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 <3dw...@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
@@ -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
_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})
{
if (__x < _Tp(-1) || __x > _Tp(+1))
@@ -160,7 +163,7 @@
_Tp __fact = _Tp(1);
for (unsigned int __i = 1; __i <= __m; ++__i)
{
- __p_mm *= -__fact * __root;
+ __p_mm *= __phase * __fact * __root;
__fact += _Tp(2);
}
}
@@ -205,8 +208,10 @@
* but this factor is rather large for large @f$ l @f$ and @f$ m @f$
* and so this function is stable for larger differences of @f$ l @f$
* and @f$ m @f$.
+ * @note Unlike the case for __assoc_legendre_p the Condon-Shortley
+ * phase factor @f$ (-1)^m @f$ is present here.
*
- * @param l The order of the spherical associated Legendre function.
+ * @param l The degree of the spherical associated Legendre function.
* @f$ l >= 0 @f$.
* @param m The order of the spherical associated Legendre function.
* @f$ m <= l @f$.
@@ -265,19 +270,15 @@
const _Tp __lnpre_val =
-_Tp(0.25L) * __numeric_constants<_Tp>::__lnpi()
+ _Tp(0.5L) * (__lnpoch + __m * __lncirc);
- _Tp __sr = std::sqrt((_Tp(2) + _Tp(1) / __m)
- / (_Tp(4) * __numeric_constants<_Tp>::__pi()));
+ const _Tp __sr = std::sqrt((_Tp(2) + _Tp(1) / __m)
+ / (_Tp(4) * __numeric_constants<_Tp>::__pi()));
_Tp __y_mm = __sgn * __sr * std::exp(__lnpre_val);
_Tp __y_mp1m = __y_mp1m_factor * __y_mm;
if (__l == __m)
-{
- return __y_mm;
-}
+return __y_mm;
else if (__l == __m + 1)
-{
- return __y_mp1m;
-}
+return __y_mp1m;
else
{
_Tp __y_lm = _Tp(0);
Index: testsuite/special_functions/02_assoc_legendre/check_value.cc
===
--- testsuite/special_functions/02_assoc_legendre/check_value.cc
(revision 259973)
+++ testsuite/special_functions/02_assoc_legendre/check_value.cc
(working copy)
@@ -1,6 +1,6 @@
// { dg-do run { target c++11 } }
-// {
