Skip to content

Commit

Permalink
Random: ensure seed is non-zero
Browse files Browse the repository at this point in the history
Random has accepted a seed value of zero and then thrown an exception.
This is not necessary, since we can simply use a non-zero seed value.
  • Loading branch information
PaulPrice committed Jan 23, 2018
1 parent 29d3a87 commit 27a6a78
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 21 deletions.
23 changes: 6 additions & 17 deletions include/lsst/afw/math/Random.h
Original file line number Diff line number Diff line change
Expand Up @@ -100,32 +100,28 @@ class Random {
// -- Constructor --------
/**
* Creates a random number generator that uses the given algorithm to produce random numbers,
* and seeds it with the specified value. Passing a seed-value of zero will cause the
* generator to be seeded with an algorithm specific default value. The default value for
* and seeds it with the specified value. The default value for
* `algorithm` is MT19937, corresponding to the "Mersenne Twister" algorithm by
* Makoto Matsumoto and Takuji Nishimura.
*
* @param[in] algorithm the algorithm to use for random number generation
* @param[in] seed the seed value to initialize the generator with
*
* @throws lsst::pex::exceptions::InvalidParameterError
* Thrown if the requested algorithm is not supported or a seed value of zero
* (corresponding to an algorithm specific seed) is chosen.
* Thrown if the requested algorithm is not supported.
* @throws lsst::pex::exceptions::MemoryError
* Thrown if memory allocation for internal generator state fails.
*/
explicit Random(Algorithm algorithm = MT19937, unsigned long seed = 1);
/**
* Creates a random number generator that uses the algorithm with the given name to produce
* random numbers, and seeds it with the specified value. Passing a seed-value of zero will
* cause the generator to be seeded with an algorithm specific default value.
* random numbers, and seeds it with the specified value.
*
* @param[in] algorithm the name of the algorithm to use for random number generation
* @param[in] seed the seed value to initialize the generator with
*
* @throws lsst::pex::exceptions::InvalidParameterError
* Thrown if the requested algorithm is not supported or a seed value of zero
* (corresponding to an algorithm specific seed) is chosen.
* Thrown if the requested algorithm is not supported.
* @throws lsst::pex::exceptions::MemoryError
* Thrown if memory allocation for internal generator state fails.
*/
Expand All @@ -145,8 +141,6 @@ class Random {
* Thrown if the requested algorithm is not supported.
* @throws lsst::pex::exceptions::MemoryError
* Thrown if memory allocation for internal generator state fails.
* @throws lsst::pex::exceptions::RuntimeError
* Thrown if the "rngSeed" policy value cannot be converted to an unsigned long int.
*/
explicit Random(std::shared_ptr<pex::policy::Policy> const policy);
// Use compiler generated destructor and shallow copy constructor/assignment operator
Expand Down Expand Up @@ -194,8 +188,7 @@ class Random {
static std::vector<std::string> const &getAlgorithmNames();
/**
* @returns The integer this random number generator was seeded with.
* @note A seed value of 0 indicates that the random number generator
* was seeded with an algorithm specific default value.
* @note The seed is guaranteed not to be zero.
*/
unsigned long getSeed() const;

Expand Down Expand Up @@ -290,9 +283,6 @@ class Random {

/**
* Initializes the underlying GSL random number generator.
*
* @throws lsst::pex::exceptions::InvalidParameterError
* Thrown if a seed value of zero (corresponding to an algorithm specific seed) is chosen.
*/
void initialize();
/**
Expand All @@ -301,8 +291,7 @@ class Random {
* @param[in] algorithm the algorithm to use for random number generation
*
* @throws lsst::pex::exceptions::InvalidParameterError
* Thrown if the requested algorithm is not supported or a seed value of zero
* (corresponding to an algorithm specific seed) is chosen.
* Thrown if the requested algorithm is not supported.
*/
void initialize(std::string const &);
};
Expand Down
9 changes: 5 additions & 4 deletions src/math/Random.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
*/

#include <cstdlib>
#include <limits>

#include "boost/format.hpp"

Expand Down Expand Up @@ -62,14 +63,14 @@ char const *const Random::_seedEnvVarName = "LSST_RNG_SEED";
// -- Private helper functions --------

void Random::initialize() {
if (_seed == 0) {
throw LSST_EXCEPT(ex::InvalidParameterError, (boost::format("Invalid RNG seed: %lu") % _seed).str());
}
::gsl_rng *rng = ::gsl_rng_alloc(_gslRngTypes[_algorithm]);
if (rng == 0) {
throw LSST_EXCEPT(ex::MemoryError, "gsl_rng_alloc() failed");
}
::gsl_rng_set(rng, _seed);
// This seed is guaranteed to be non-zero.
// We want to give a non-zero seed to GSL to avoid it choosing its own.
unsigned long int useSeed = _seed == 0 ? std::numeric_limits<unsigned long int>::max() : _seed;
::gsl_rng_set(rng, useSeed);
_rng.reset(rng, ::gsl_rng_free);
}

Expand Down

0 comments on commit 27a6a78

Please sign in to comment.