Skip to content

Commit

Permalink
Use linear congruential random number generator from C++11.
Browse files Browse the repository at this point in the history
  • Loading branch information
@egorpugin
egorpugin committed Dec 30, 2020
1 parent a0509b2 commit 2252936
Showing 1 changed file with 5 additions and 15 deletions.
20 changes: 5 additions & 15 deletions include/tesseract/helpers.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,19 +24,17 @@
#include <cstdio>
#include <cstring>
#include <functional>
#include <random>
#include <string>

namespace tesseract {

// A simple linear congruential random number generator, using Knuth's
// constants from:
// http://en.wikipedia.org/wiki/Linear_congruential_generator.
// A simple linear congruential random number generator.
class TRand {
public:
TRand() = default;
// Sets the seed to the given value.
void set_seed(uint64_t seed) {
seed_ = seed;
e.seed(seed);
}
// Sets the seed using a hash of a string.
void set_seed(const std::string& str) {
Expand All @@ -46,8 +44,7 @@ class TRand {

// Returns an integer in the range 0 to INT32_MAX.
int32_t IntRand() {
Iterate();
return seed_ >> 33;
return e();

This comment has been minimized.

Sign in to view

Copy link
@stweil

stweil Jan 1, 2021

Member

Are you sure that this still returns an integer in the range 0 to INT32_MAX?

This comment has been minimized.

Sign in to view

Copy link
@egorpugin

egorpugin Jan 1, 2021 •
edited
Loading

Author Contributor

I think e() returns uint (32 bit).
Main goal was to replace old implementation with standard one.
I tried to remove this class to use C++ <random> solely. But this class has extra methods, so I replaced custom code with engine from C++.

We can replace return type to auto or remove this class eventually with usage of some other random engines like mersenne or still LCG.
}
// Returns a floating point value in the range [-range, range].
double SignedRand(double range) {
Expand All @@ -59,14 +56,7 @@ class TRand {
}

private:
// Steps the generator to the next value.
void Iterate() {
seed_ *= 6364136223846793005ULL;
seed_ += 1442695040888963407ULL;
}

// The current value of the seed.
uint64_t seed_{1};
std::minstd_rand e;
};

// Remove newline (if any) at the end of the string.
Expand Down

3 comments on commit 2252936

@stweil
Copy link
Member

@stweil stweil commented on 2252936 Jan 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reproducible results are very important for my work. Changing to a different random generator also changes all processes which use that generator. What do we gain by using the new code?

@egorpugin
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can revert if you'd like to.

@egorpugin
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should match previous behavior.
We can add a test with original impl and such std test to compare some values if you'd like to keep the same RNG.

  int32_t IntRand() {
    return e() >> 33;
  }
 private:
  std::linear_congruential_engine<std::uint64_t, 6364136223846793005ULL, 1442695040888963407ULL, 1ULL << 63> e;

Please sign in to comment.