diff mbox

PR libstdc++/80506 fix constant used in condition

Message ID 20170424134316.GA16538@redhat.com
State New
Headers show

Commit Message

Jonathan Wakely April 24, 2017, 1:43 p.m. UTC
We use the wrong constant for the Marsaglia Tsang algorithm.

	PR libstdc++/80506
	* include/bits/random.tcc (gamma_distribution::operator()): Fix magic
	number used in loop condition.

Tested powerpc64le-linux, committed to trunk.
commit aa4da6523b7bfab7ba92c2e9e505155e1ce432a7
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Mon Apr 24 13:10:49 2017 +0100

    PR libstdc++/80506 fix constant used in condition
    
    	PR libstdc++/80506
    	* include/bits/random.tcc (gamma_distribution::operator()): Fix magic
    	number used in loop condition.

Comments

Paolo Carlini April 26, 2017, 9:13 a.m. UTC | #1
Hi,

On 24/04/2017 15:43, Jonathan Wakely wrote:
> We use the wrong constant for the Marsaglia Tsang algorithm.
>
>     PR libstdc++/80506
>     * include/bits/random.tcc (gamma_distribution::operator()): Fix magic
>     number used in loop condition.
>
> Tested powerpc64le-linux, committed to trunk.

Thanks Jon for handling this. It would still be very nice to have a 
testcase...

Cheers,
Paolo.
Paolo Carlini April 26, 2017, 9:14 a.m. UTC | #2
.. or maybe using the wrong constant only impacts the performance?!?

Paolo.
Jonathan Wakely April 26, 2017, 9:16 a.m. UTC | #3
On 26/04/17 11:14 +0200, Paolo Carlini wrote:
>.. or maybe using the wrong constant only impacts the performance?!?

Yes, I think so. I did some very simple sanity tests and the numbers
were identical before and after.
Ed Smith-Rowland May 6, 2018, 1:19 a.m. UTC | #4
On 04/26/2017 05:16 AM, Jonathan Wakely wrote:
> On 26/04/17 11:14 +0200, Paolo Carlini wrote:
>> .. or maybe using the wrong constant only impacts the performance?!?
>
> Yes, I think so. I did some very simple sanity tests and the numbers
> were identical before and after.
>
>
>
I was backporting this and saw that __generate_impl does this twice more.

For trunk and branch-8 I have these patches.

OK?
2018-05-07  Edward Smith-Rowland  <3dw4rd@verizon.net>

	Moar PR libstdc++/80506
	* include/bits/random.tcc (gamma_distribution::__generate_impl()):
	Fix magic number used in loop condition.
Index: include/bits/random.tcc
===================================================================
--- include/bits/random.tcc	(revision 259965)
+++ include/bits/random.tcc	(working copy)
@@ -2408,7 +2408,7 @@
 		  __v = __v * __v * __v;
 		  __u = __aurng();
 		}
-	      while (__u > result_type(1.0) - 0.331 * __n * __n * __n * __n
+	      while (__u > result_type(1.0) - 0.0331 * __n * __n * __n * __n
 		     && (std::log(__u) > (0.5 * __n * __n + __a1
 					  * (1.0 - __v + std::log(__v)))));
 
@@ -2429,7 +2429,7 @@
 		  __v = __v * __v * __v;
 		  __u = __aurng();
 		}
-	      while (__u > result_type(1.0) - 0.331 * __n * __n * __n * __n
+	      while (__u > result_type(1.0) - 0.0331 * __n * __n * __n * __n
 		     && (std::log(__u) > (0.5 * __n * __n + __a1
 					  * (1.0 - __v + std::log(__v)))));
Jonathan Wakely May 7, 2018, 12:22 p.m. UTC | #5
On 05/05/18 21:19 -0400, Ed Smith-Rowland wrote:
>On 04/26/2017 05:16 AM, Jonathan Wakely wrote:
>>On 26/04/17 11:14 +0200, Paolo Carlini wrote:
>>>.. or maybe using the wrong constant only impacts the performance?!?
>>
>>Yes, I think so. I did some very simple sanity tests and the numbers
>>were identical before and after.
>>
>>
>>
>I was backporting this and saw that __generate_impl does this twice more.
>
>For trunk and branch-8 I have these patches.
>
>OK?

OK, thanks for noticing the additional cases.
diff mbox

Patch

diff --git a/libstdc++-v3/include/bits/random.tcc b/libstdc++-v3/include/bits/random.tcc
index df05ebe..63d1c02 100644
--- a/libstdc++-v3/include/bits/random.tcc
+++ b/libstdc++-v3/include/bits/random.tcc
@@ -2356,7 +2356,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	    __v = __v * __v * __v;
 	    __u = __aurng();
 	  }
-	while (__u > result_type(1.0) - 0.331 * __n * __n * __n * __n
+	while (__u > result_type(1.0) - 0.0331 * __n * __n * __n * __n
 	       && (std::log(__u) > (0.5 * __n * __n + __a1
 				    * (1.0 - __v + std::log(__v)))));