Issue876Test.Reset is broken

[osrf-migration]

Original report (archived issue) by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).

---

I approved the pull request, but I apparently never tested it, since the test doesn't pass.

$ test/regression/REGRESSION_876_random_number_generator 
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from Issue876Test
[ RUN      ] Issue876Test.Reset
Msg Waiting for master
Msg Connected to gazebo master @ http://127.0.0.1:11345
Msg Publicized address: 192.168.1.183
Dbg ServerFixture load in 0.09 seconds, timeout after 60 seconds
/home/scpeters/ws/gazebo/src/gazebo/test/regression/876_random_number_generator.cc:52: Failure
Value of: numReset[i]
  Actual: -5
Expected: num[i]
Which is: -1
[  FAILED  ] Issue876Test.Reset (1149 ms)
[----------] 1 test from Issue876Test (1149 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (1150 ms total)
[  PASSED  ] 0 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] Issue876Test.Reset

 1 FAILED TEST
$

I found this in my overnight testing of the gazebo_2.0 branch. It's not in Jenkins yet because gazebo_2.0 hasn't been merged to default recently enough.

[osrf-migration]

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).

---

• set component to "testing"

[osrf-migration]

Original comment by Jose Luis Rivero (Bitbucket: Jose Luis Rivero, GitHub: j-rivero).

---

I don't understand very well what is the propose of the check. To generate always the same random numbers after a world reset? This is not happening at all.

Maybe @nkoenig can help with this, he is listed as the author of the commit (87ccca6).

[osrf-migration]

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).

---

I took a look at this last week. The following patch adds an additional test, and a potential fix to the broken test. I haven't had a chance to test fully.

diff -r 0808e9cb7c51987967ca8fca9289c6e14e181e0c gazebo/math/Rand_TEST.cc
--- a/gazebo/math/Rand_TEST.cc  Thu Oct 17 10:56:38 2013 -0700
+++ b/gazebo/math/Rand_TEST.cc  Fri Oct 18 00:48:04 2013 -0400
@@ -47,3 +47,25 @@
     EXPECT_TRUE(math::equal(d, 0.985827));
   }
 }
+
+TEST(RandTest, SetSeed)
+{
+  int N = 10;
+  std::vector<int> first;
+  std::vector<int> second;
+
+  for (int i = 0; i < N; ++i)
+  {
+    math::Rand::SetSeed(i);
+    first.push_back(math::Rand::GetIntUniform(-10, 10));
+    second.push_back(math::Rand::GetIntUniform(-10, 10));
+  }
+
+  for (int i = 0; i < N; ++i)
+  {
+    math::Rand::SetSeed(i);
+    EXPECT_EQ(first[i], math::Rand::GetIntUniform(-10, 10));
+    EXPECT_EQ(second[i], math::Rand::GetIntUniform(-10, 10));
+    std::cout << first[i] << ' ' << second[i] << std::endl;
+  }
+}
diff -r 0808e9cb7c51987967ca8fca9289c6e14e181e0c test/regression/876_random_number_generator.cc
--- a/test/regression/876_random_number_generator.cc    Thu Oct 17 10:56:38 2013 -0700
+++ b/test/regression/876_random_number_generator.cc    Fri Oct 18 00:48:04 2013 -0400
@@ -33,6 +33,8 @@
   physics::WorldPtr world = physics::get_world("default");
   ASSERT_TRUE(world);

+  math::Rand::SetSeed(math::Rand::GetSeed());
+
   int sampleCount = 500;

   std::vector<int> num;

[osrf-migration]

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).

---

I've committed that patch (6ff5ea4) in branch issue_884. Currently it is required to reset the random seed right after loading the world in order to guarantee repeatability. I'm not sure if this satisfies the intent of #876.

[osrf-migration]

Original comment by Nate Koenig (Bitbucket: Nathan Koenig).

---

Awesome, thanks. Resetting the seed after loading the world is fine. As long as there is a way to get repeatable behavior.

[osrf-migration]

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).

---

Ok, I'll make a pull request.

[osrf-migration]

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).

---

pull request #784

[osrf-migration]

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).

---

• changed state from "new" to "resolved"

pull request #784

[osrf-migration]

Original comment by Nate Koenig (Bitbucket: Nathan Koenig).

---

• set version to "all"

[osrf-migration]

Original comment by Nate Koenig (Bitbucket: Nathan Koenig).

---

• changed state from "resolved" to "closed"