On Sat, 9 Aug 2014, Ulrich Drepper wrote:
How about the patch below?
Looks good, with two details:
+ template<typename _RealType>
+ class uniform_on_sphere_helper<2, _RealType>
+ {
+ typedef typename uniform_on_sphere_distribution<2, _RealType>::
+ result_type result_type;
+
+ public:
+ template<typename _NormalDistribution,
+ typename _UniformRandomNumberGenerator>
+ result_type operator()(_NormalDistribution&,
+ _UniformRandomNumberGenerator& __urng)
+ {
+ result_type __ret;
+ _RealType __sq;
+ std::__detail::_Adaptor<_UniformRandomNumberGenerator,
+ _RealType> __aurng(__urng);
+
+ do
+ {
+ __ret[0] = __aurng();
I must be missing something obvious, but normal_distribution uses:
__x = result_type(2.0) * __aurng() - 1.0;
to get a number between -1 and 1, and I don't see where you do this
rescaling. Does __aurng() already return a number in the right interval in
this context? If so we may want to update our naming of variables to make
that clearer.
+ __ret[1] = __aurng();
+
+ __sq = __ret[0] * __ret[0] + __ret[1] * __ret[1];
+ }
+ while (__sq == _RealType(0) || __sq > _RealType(1));
+
+ // Yes, we do not just use sqrt(__sq) because hypot() is more
+ // accurate.
+ auto __norm = std::hypot(__ret[0], __ret[1]);
Assuming the 2 coordinates are obtained through a rescaling x->2*x-1, if
__sq is not exactly 0, it must be between 2^-103 and 1 (for ieee
double), so I am not sure hypot gains that much (at least in my mind
hypot was mostly a gain close to 0 or infinity, but maybe it has more
advantages). It can only hurt speed though, so not a big issue.
--
Marc Glisse
