diff mbox

[v3] complex functions with expression template reals

Message ID alpine.DEB.2.02.1402231050480.29647@stedding.saclay.inria.fr
State New
Headers show

Commit Message

Marc Glisse Feb. 23, 2014, 10:32 a.m. UTC
Hello,

looking at this question:
http://stackoverflow.com/q/21737186/1918193
I was surprised to see that libstdc++'s std::complex basically just works 
with user-defined types, even weird expression template ones, although 
that's not a supported use afaik.

The only functions that fail seem to be exp and pow, both because they 
call polar with two arguments that have different (expression) types.

I am not proposing to make this a supported use, but the cost of this 
small patch seems very low, and if it makes a couple users happy...

Regtested with no problem on x86_64-linux-gnu, ok for stage 1?

2014-02-23  Marc Glisse  <marc.glisse@inria.fr>

 	* include/std/complex (__complex_exp, pow): Specify the template
 	parameter in calls to std::polar, for expression templates.

Comments

Paolo Carlini Feb. 23, 2014, 2:44 p.m. UTC | #1
On 02/23/2014 11:32 AM, Marc Glisse wrote:
> Hello,
>
> looking at this question:
> http://stackoverflow.com/q/21737186/1918193
> I was surprised to see that libstdc++'s std::complex basically just 
> works with user-defined types, even weird expression template ones, 
> although that's not a supported use afaik.
>
> The only functions that fail seem to be exp and pow, both because they 
> call polar with two arguments that have different (expression) types.
>
> I am not proposing to make this a supported use, but the cost of this 
> small patch seems very low, and if it makes a couple users happy...
>
> Regtested with no problem on x86_64-linux-gnu, ok for stage 1?
I would even be in favor of applying it now. Can we figure out simple 
(ie, not relying on boost...) testcases too?

Paolo.
Marc Glisse Feb. 23, 2014, 3:11 p.m. UTC | #2
On Sun, 23 Feb 2014, Paolo Carlini wrote:

> On 02/23/2014 11:32 AM, Marc Glisse wrote:
>> Hello,
>> 
>> looking at this question:
>> http://stackoverflow.com/q/21737186/1918193
>> I was surprised to see that libstdc++'s std::complex basically just works 
>> with user-defined types, even weird expression template ones, although 
>> that's not a supported use afaik.
>> 
>> The only functions that fail seem to be exp and pow, both because they call 
>> polar with two arguments that have different (expression) types.
>> 
>> I am not proposing to make this a supported use, but the cost of this small 
>> patch seems very low, and if it makes a couple users happy...
>> 
>> Regtested with no problem on x86_64-linux-gnu, ok for stage 1?
>
> I would even be in favor of applying it now. Can we figure out simple (ie, 
> not relying on boost...) testcases too?

I didn't try std::complex<std::valarray<X>>, maybe...

Otherwise, you need a type T with all the (real) math functions defined, 
and where every operation returns a different type (implicitly convertible 
to T). And then you want to call all the complex functions.

That seems doable, but way bigger than I'm willing to go for this 
feature. If you want to take over, be my guest ;-)
Paolo Carlini Feb. 24, 2014, 9:10 a.m. UTC | #3
Hi,

On 02/23/2014 04:11 PM, Marc Glisse wrote:
> On Sun, 23 Feb 2014, Paolo Carlini wrote:
>
>> On 02/23/2014 11:32 AM, Marc Glisse wrote:
>>> Hello,
>>>
>>> looking at this question:
>>> http://stackoverflow.com/q/21737186/1918193
>>> I was surprised to see that libstdc++'s std::complex basically just 
>>> works with user-defined types, even weird expression template ones, 
>>> although that's not a supported use afaik.
>>>
>>> The only functions that fail seem to be exp and pow, both because 
>>> they call polar with two arguments that have different (expression) 
>>> types.
>>>
>>> I am not proposing to make this a supported use, but the cost of 
>>> this small patch seems very low, and if it makes a couple users 
>>> happy...
>>>
>>> Regtested with no problem on x86_64-linux-gnu, ok for stage 1?
>>
>> I would even be in favor of applying it now. Can we figure out simple 
>> (ie, not relying on boost...) testcases too?
>
> I didn't try std::complex<std::valarray<X>>, maybe...
>
> Otherwise, you need a type T with all the (real) math functions 
> defined, and where every operation returns a different type 
> (implicitly convertible to T). And then you want to call all the 
> complex functions.
>
> That seems doable, but way bigger than I'm willing to go for this 
> feature. If you want to take over, be my guest ;-)
Another option would be just using boost/multiprecision/mpfr.hpp when 
available. In general, I think it makes sense to have a minimum of 
infrastructure enabling tests checking interoperability with boost. If 
only we had a check_v3_target_header { args } it would be most of it, 
but it doesn't seem we do?!? Anyway I guess we can take care of that 
post 4.9.0 and commit the straightforward code tweak now. Jon?

Paolo.

PS: Resending message, yesterday had issues with html
Jonathan Wakely Feb. 26, 2014, 9:57 a.m. UTC | #4
On 24 February 2014 09:10, Paolo Carlini wrote:
>
> Another option would be just using boost/multiprecision/mpfr.hpp when
> available. In general, I think it makes sense to have a minimum of
> infrastructure enabling tests checking interoperability with boost. If only
> we had a check_v3_target_header { args } it would be most of it, but it
> doesn't seem we do?!? Anyway I guess we can take care of that post 4.9.0 and
> commit the straightforward code tweak now. Jon?

I'd be happy with it for Stage 1, but I don't think we should apply it
now if it is only useful for people doing unusual (unsupported) things
with std::complex.
Paolo Carlini Feb. 26, 2014, 9:58 a.m. UTC | #5
On 02/26/2014 10:57 AM, Jonathan Wakely wrote:
> On 24 February 2014 09:10, Paolo Carlini wrote:
>> Another option would be just using boost/multiprecision/mpfr.hpp when
>> available. In general, I think it makes sense to have a minimum of
>> infrastructure enabling tests checking interoperability with boost. If only
>> we had a check_v3_target_header { args } it would be most of it, but it
>> doesn't seem we do?!? Anyway I guess we can take care of that post 4.9.0 and
>> commit the straightforward code tweak now. Jon?
> I'd be happy with it for Stage 1, but I don't think we should apply it
> now if it is only useful for people doing unusual (unsupported) things
> with std::complex.
Ok. Marc, please remember to apply it as soon as Stage 1 re-opens.

Paolo.
Marc Glisse April 11, 2014, 7:01 p.m. UTC | #6
On Wed, 26 Feb 2014, Paolo Carlini wrote:

> On 02/26/2014 10:57 AM, Jonathan Wakely wrote:
>> On 24 February 2014 09:10, Paolo Carlini wrote:
>>> Another option would be just using boost/multiprecision/mpfr.hpp when
>>> available. In general, I think it makes sense to have a minimum of
>>> infrastructure enabling tests checking interoperability with boost. If 
>>> only
>>> we had a check_v3_target_header { args } it would be most of it, but it
>>> doesn't seem we do?!? Anyway I guess we can take care of that post 4.9.0 
>>> and
>>> commit the straightforward code tweak now. Jon?
>> I'd be happy with it for Stage 1, but I don't think we should apply it
>> now if it is only useful for people doing unusual (unsupported) things
>> with std::complex.
> Ok. Marc, please remember to apply it as soon as Stage 1 re-opens.

r209321
(for a patch accepted long ago, I guess it is better to leave a trace in 
gcc-patches near the commit date)
Eric Botcazou April 12, 2014, 8:08 a.m. UTC | #7
> r209321
> (for a patch accepted long ago, I guess it is better to leave a trace in
> gcc-patches near the commit date)

Nope, see the doc, we have ChangeLog and gcc-cvs for this purpose.
Marc Glisse April 12, 2014, 8:20 a.m. UTC | #8
On Sat, 12 Apr 2014, Eric Botcazou wrote:

>> r209321
>> (for a patch accepted long ago, I guess it is better to leave a trace in
>> gcc-patches near the commit date)
>
> Nope, see the doc, we have ChangeLog and gcc-cvs for this purpose.

Those don't point to the conversation in gcc-patches where the patch was 
ok-ed by a maintainer (maybe we should start adding URLs pointing to the 
gcc-patches messages in commit logs?).

I also meant it as a "hello" to whoever reviewed the patch, but it is true 
they likely already read new ChangeLog entries or follow (a filtered 
version of) gcc-cvs.

Thanks, I won't do it again in the future :-)
diff mbox

Patch

Index: libstdc++-v3/include/std/complex
===================================================================
--- libstdc++-v3/include/std/complex	(revision 208045)
+++ libstdc++-v3/include/std/complex	(working copy)
@@ -728,21 +728,21 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 #else
   template<typename _Tp>
     inline complex<_Tp>
     cosh(const complex<_Tp>& __z) { return __complex_cosh(__z); }
 #endif
 
   // 26.2.8/3 exp(__z): Returns the complex base e exponential of x
   template<typename _Tp>
     inline complex<_Tp>
     __complex_exp(const complex<_Tp>& __z)
-    { return std::polar(exp(__z.real()), __z.imag()); }
+    { return std::polar<_Tp>(exp(__z.real()), __z.imag()); }
 
 #if _GLIBCXX_USE_C99_COMPLEX
   inline __complex__ float
   __complex_exp(__complex__ float __z) { return __builtin_cexpf(__z); }
 
   inline __complex__ double
   __complex_exp(__complex__ double __z) { return __builtin_cexp(__z); }
 
   inline __complex__ long double
   __complex_exp(const __complex__ long double& __z)
@@ -988,21 +988,21 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
     pow(const complex<_Tp>& __x, const _Tp& __y)
     {
 #ifndef _GLIBCXX_USE_C99_COMPLEX
       if (__x == _Tp())
 	return _Tp();
 #endif
       if (__x.imag() == _Tp() && __x.real() > _Tp())
         return pow(__x.real(), __y);
 
       complex<_Tp> __t = std::log(__x);
-      return std::polar(exp(__y * __t.real()), __y * __t.imag());
+      return std::polar<_Tp>(exp(__y * __t.real()), __y * __t.imag());
     }
 
   template<typename _Tp>
     inline complex<_Tp>
     __complex_pow(const complex<_Tp>& __x, const complex<_Tp>& __y)
     { return __x == _Tp() ? _Tp() : std::exp(__y * std::log(__x)); }
 
 #if _GLIBCXX_USE_C99_COMPLEX
   inline __complex__ float
   __complex_pow(__complex__ float __x, __complex__ float __y)
@@ -1025,22 +1025,22 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
   template<typename _Tp>
     inline complex<_Tp>
     pow(const complex<_Tp>& __x, const complex<_Tp>& __y)
     { return __complex_pow(__x, __y); }
 #endif
 
   template<typename _Tp>
     inline complex<_Tp>
     pow(const _Tp& __x, const complex<_Tp>& __y)
     {
-      return __x > _Tp() ? std::polar(pow(__x, __y.real()),
-				      __y.imag() * log(__x))
+      return __x > _Tp() ? std::polar<_Tp>(pow(__x, __y.real()),
+					   __y.imag() * log(__x))
 	                 : std::pow(complex<_Tp>(__x), __y);
     }
 
   /// 26.2.3  complex specializations
   /// complex<float> specialization
   template<>
     struct complex<float>
     {
       typedef float value_type;
       typedef __complex__ float _ComplexT;