From c907b6b7107a8f0a2756ba3e424b4913df3048d4 Mon Sep 17 00:00:00 2001
From: Jussi Lindgren <openvibe-dev@inria.fr>
Date: Wed, 16 Sep 2015 11:41:16 +0200
Subject: [PATCH] Modules: Fixing issues with nonrepeatable experiments due to
randomness
- Changed System::Math to use its own simple random generator as dependencies can reset rand() seed on some systems
- Fixed System::Math algorithm to randomize numbers with ceilings to get a more uniform result
- Changed classifier trainer to use System::Math for shuffling
- Changed stimulation voter to use System::Math for tie breaking
- Added a few (unrelated) safety checks to Pairwise Decision algorithm
---
.../platform/designer/src/ovd_main.cpp | 3 +-
modules/system/src/ovCMath.cpp | 75 ++++++++++++++++---
.../ovpCAlgorithmPairwiseDecision.cpp | 8 +-
.../ovpCBoxAlgorithmClassifierTrainer.cpp | 4 +-
.../ovpCBoxAlgorithmStimulationVoter.cpp | 15 ++--
5 files changed, 84 insertions(+), 21 deletions(-)
diff --git a/applications/platform/designer/src/ovd_main.cpp b/applications/platform/designer/src/ovd_main.cpp
index 70f4c82c23..fb39a2d2af 100644
--- a/applications/platform/designer/src/ovd_main.cpp
+++ b/applications/platform/designer/src/ovd_main.cpp
@@ -577,7 +577,8 @@ int go(int argc, char ** argv)
if(l_oConfiguration.m_oFlag.count(CommandLineFlag_RandomSeed))
{
- System::Math::initializeRandomMachine(atol(l_oConfiguration.m_oFlag[CommandLineFlag_RandomSeed].c_str()));
+ const int32 l_i32Seed = atol(l_oConfiguration.m_oFlag[CommandLineFlag_RandomSeed].c_str());
+ System::Math::initializeRandomMachine(static_cast<const uint32>(l_i32Seed));
}
else
{
diff --git a/modules/system/src/ovCMath.cpp b/modules/system/src/ovCMath.cpp
index 199ea3a94a..244882d667 100644
--- a/modules/system/src/ovCMath.cpp
+++ b/modules/system/src/ovCMath.cpp
@@ -1,13 +1,64 @@
+/**
+ *
+ * @fixme This class could benefit from a serious overhaul, e.g. using randomness from some established library or C11.
+ *
+ * - Here Linear Congruential Generator is re-implemented to avoid third-party dependencies messing the up the rand() state.
+ * This happened before when we used the global srand() / rand(). It made hard to make repeatable experiments on some
+ * platforms. The generated randomness from the introduced home-made class is not super but it should be sufficient for
+ * OpenViBE's present use-cases.
+ *
+ * Other notes
+ *
+ * - The randomFloat32() and randomFloat64() functions might not be working as expected (verify)
+ * - Due to generative process, values generated above l_ui32RandMax may not be dense (verify)
+ * - randomUInteger32WithCeiling() may not be dense either
+ *
+ */
#include "ovCMath.h"
#include <cstdlib>
#include <cstring>
using namespace System;
+class RandomGenerator
+{
+private:
+ uint32 l_ui32NextValue;
+
+public:
+ static const uint32 l_ui32RandMax = 0x7FFFFFFF; // (2^32)/2-1 == 2147483647
+
+ RandomGenerator(uint32 seed = 1) : l_ui32NextValue( seed ) {}
+
+ int32 rand(void)
+ {
+ // Pretty much C99 convention and parameters for a Linear Congruential Generator
+ l_ui32NextValue = (l_ui32NextValue * 1103515245 + 12345) & l_ui32RandMax;
+ return static_cast<int32>(l_ui32NextValue);
+ }
+
+ void setSeed(uint32 seed)
+ {
+ l_ui32NextValue = seed;
+ }
+
+ uint32 getSeed(void) const
+ {
+ return l_ui32NextValue;
+ }
+};
+
+// Should be only accessed via Math:: calls defined below
+static RandomGenerator g_oRandomGenerator;
+
boolean Math::initializeRandomMachine(const uint64 ui64RandomSeed)
{
- srand((unsigned int)ui64RandomSeed);
+ g_oRandomGenerator.setSeed(static_cast<uint32>(ui64RandomSeed));
+
+ // For safety, we install also the C++ basic random engine (it might be useg by dependencies, old code, etc)
+ srand(static_cast<unsigned int>(ui64RandomSeed));
+
return true;
}
@@ -28,16 +79,22 @@ uint32 Math::randomUInteger32(void)
uint64 Math::randomUInteger64(void)
{
- uint64 r1=rand();
- uint64 r2=rand();
- uint64 r3=rand();
- uint64 r4=rand();
+ const uint64 r1=g_oRandomGenerator.rand();
+ const uint64 r2=g_oRandomGenerator.rand();
+ const uint64 r3=g_oRandomGenerator.rand();
+ const uint64 r4=g_oRandomGenerator.rand();
return (r1<<24)^(r2<<16)^(r3<<8)^(r4);
}
uint32 Math::randomUInteger32WithCeiling(uint32 ui32upperLimit)
{
- return static_cast<int32>(randomUInteger64()) % ui32upperLimit;
+ // float in range [0,1]
+ const float64 l_f64Temp = g_oRandomGenerator.rand() / static_cast<float64>(g_oRandomGenerator.l_ui32RandMax);
+
+ // static_cast is effectively floor(), so below we get output range [0,upperLimit-1], without explicit subtraction of 1
+ const uint32 l_ui32ReturnValue = static_cast<uint32>( ui32upperLimit * l_f64Temp );
+
+ return l_ui32ReturnValue;
}
int8 Math::randomSInterger8(void)
@@ -62,20 +119,20 @@ int64 Math::randomSInterger64(void)
float32 Math::randomFloat32(void)
{
- uint32 r=randomUInteger32();
+ const uint32 r=randomUInteger32();
float32 fr;
::memcpy(&fr, &r, sizeof(fr));
return fr;
}
float32 Math::randomFloat32BetweenZeroAndOne(void) {
- float32 fr = static_cast<float32>(rand()) / static_cast<float32>(RAND_MAX);
+ const float32 fr = static_cast<float32>(g_oRandomGenerator.rand()) / static_cast<float32>(g_oRandomGenerator.l_ui32RandMax);
return fr;
}
float64 Math::randomFloat64(void)
{
- uint64 r=randomUInteger64();
+ const uint64 r=randomUInteger64();
float64 fr;
::memcpy(&fr, &r, sizeof(fr));
return fr;
diff --git a/plugins/processing/classification/src/algorithms/ovpCAlgorithmPairwiseDecision.cpp b/plugins/processing/classification/src/algorithms/ovpCAlgorithmPairwiseDecision.cpp
index a96c936ef0..d881b7be2e 100644
--- a/plugins/processing/classification/src/algorithms/ovpCAlgorithmPairwiseDecision.cpp
+++ b/plugins/processing/classification/src/algorithms/ovpCAlgorithmPairwiseDecision.cpp
@@ -33,11 +33,15 @@ boolean CAlgorithmPairwiseDecision::process()
{
TParameterHandler < XML::IXMLNode* > op_pConfiguration(this->getInputParameter(OVP_Algorithm_Classifier_Pairwise_InputParameterId_Configuration));
XML::IXMLNode* l_pTempNode = (XML::IXMLNode*)op_pConfiguration;
- return this->loadConfiguration(*l_pTempNode);
+ if(l_pTempNode != NULL)
+ {
+ return this->loadConfiguration(*l_pTempNode);
+ }
+ return false;
}
else if(this->isInputTriggerActive(OVP_Algorithm_Classifier_Pairwise_InputTriggerId_Parametrize))
{
- this->parametrize();
+ return this->parametrize();
}
return true;
}
diff --git a/plugins/processing/classification/src/box-algorithms/ovpCBoxAlgorithmClassifierTrainer.cpp b/plugins/processing/classification/src/box-algorithms/ovpCBoxAlgorithmClassifierTrainer.cpp
index df241c6fd0..aa3391fd98 100644
--- a/plugins/processing/classification/src/box-algorithms/ovpCBoxAlgorithmClassifierTrainer.cpp
+++ b/plugins/processing/classification/src/box-algorithms/ovpCBoxAlgorithmClassifierTrainer.cpp
@@ -1,6 +1,7 @@
#include "ovpCBoxAlgorithmClassifierTrainer.h"
#include <system/ovCMemory.h>
+#include <system/ovCMath.h>
#include <fstream>
#include <sstream>
@@ -326,8 +327,7 @@ boolean CBoxAlgorithmClassifierTrainer::process(void)
if (l_bRandomizeVectorOrder)
{
this->getLogManager() << LogLevel_Info << "Randomizing the feature vector set\n";
- random_shuffle(m_vFeatureVectorIndex.begin(), m_vFeatureVectorIndex.end());
-
+ random_shuffle(m_vFeatureVectorIndex.begin(), m_vFeatureVectorIndex.end(), System::Math::randomUInteger32WithCeiling);
}
if(m_ui64PartitionCount>=2)
diff --git a/plugins/processing/stimulation/src/box-algorithms/ovpCBoxAlgorithmStimulationVoter.cpp b/plugins/processing/stimulation/src/box-algorithms/ovpCBoxAlgorithmStimulationVoter.cpp
index 11e6ebeac9..b0c8afec23 100644
--- a/plugins/processing/stimulation/src/box-algorithms/ovpCBoxAlgorithmStimulationVoter.cpp
+++ b/plugins/processing/stimulation/src/box-algorithms/ovpCBoxAlgorithmStimulationVoter.cpp
@@ -1,6 +1,7 @@
#include "ovpCBoxAlgorithmStimulationVoter.h"
#include <openvibe/ovITimeArithmetics.h>
+#include <system/ovCMath.h>
#include <vector>
#include <string>
@@ -40,12 +41,12 @@ boolean CBoxAlgorithmStimulationVoter::initialize(void)
ip_pMemoryBuffer.initialize(m_pDecoder->getInputParameter(OVP_GD_Algorithm_StimulationStreamDecoder_InputParameterId_MemoryBufferToDecode));
op_pStimulationSet.initialize(m_pDecoder->getOutputParameter(OVP_GD_Algorithm_StimulationStreamDecoder_OutputParameterId_StimulationSet));
- m_ui64MinimumVotes = FSettingValueAutoCast(*this->getBoxAlgorithmContext(), 0);
- m_f64TimeWindow = FSettingValueAutoCast(*this->getBoxAlgorithmContext(), 1);
- m_oClearVotes = ((uint64)FSettingValueAutoCast(*this->getBoxAlgorithmContext(), 2));
- m_oOutputDateMode = ((uint64)FSettingValueAutoCast(*this->getBoxAlgorithmContext(), 3));
- m_ui64RejectClassLabel = FSettingValueAutoCast(*this->getBoxAlgorithmContext(), 4);
- m_oRejectClass_CanWin = ((uint64)FSettingValueAutoCast(*this->getBoxAlgorithmContext(), 5));
+ m_ui64MinimumVotes = FSettingValueAutoCast(*this->getBoxAlgorithmContext(), 0);
+ m_f64TimeWindow = FSettingValueAutoCast(*this->getBoxAlgorithmContext(), 1);
+ m_oClearVotes = ((uint64)FSettingValueAutoCast(*this->getBoxAlgorithmContext(), 2));
+ m_oOutputDateMode = ((uint64)FSettingValueAutoCast(*this->getBoxAlgorithmContext(), 3));
+ m_ui64RejectClassLabel = FSettingValueAutoCast(*this->getBoxAlgorithmContext(), 4);
+ m_oRejectClass_CanWin = ((uint64)FSettingValueAutoCast(*this->getBoxAlgorithmContext(), 5));
this->getLogManager() << LogLevel_Debug << "Vote clear mode " << m_oClearVotes << ", timestamp at " << m_oOutputDateMode << ", reject mode " << m_oRejectClass_CanWin << "\n";
@@ -177,7 +178,7 @@ boolean CBoxAlgorithmStimulationVoter::process(void)
}
else if( l_ui64StimulusVotes == l_ui64MaxVotes ) {
// Break ties arbitrarily
- if(rand() > RAND_MAX/2 )
+ if(System::Math::randomFloat32BetweenZeroAndOne() > 0.5)
{
l_ui64ResultClassLabel = l_ui64StimulusType;
l_ui64MaxVotes = l_ui64StimulusVotes;
--
GitLab