Repository: BlueBrain/libsonata (language: python)

Issue:
simulation config: no default value for key `input: random_seed`
As discussed in [BBPBGLIB-1025](https://bbpteam.epfl.ch/project/issues/browse/BBPBGLIB-1025), the `random_seed` key of certain stimulus type should not have a default value set from the parser. This key is used to override a gid dependent seed computed internally by neurodamus.  Currently our parser set its default to be the `run:random_seed` which always disables the gid dependent seeding and leads to an anomalous behavior in the simulation.


Discussion / hints:
Are you looking at other places where this may be the case?  It sounds like we may need to audit the implementation vs the specification, so make sure other things like this aren't missed.

Fix patch:
diff --git a/include/bbp/sonata/config.h b/include/bbp/sonata/config.h
index 3b07150d..a75b21db 100644
--- a/include/bbp/sonata/config.h
+++ b/include/bbp/sonata/config.h
@@ -464,8 +464,8 @@ class SONATA_API SimulationConfig
         double riseTime{};
         /// The decay time of the bi-exponential shots (ms)
         double decayTime{};
-        /// Override the random seed to introduce correlations between cells
-        int randomSeed{};
+        /// Override the random seed to introduce correlations between cells, default = None
+        nonstd::optional<int> randomSeed{nonstd::nullopt};
         /// Timestep of generated signal in ms. Default is 0.25 ms
         double dt{};
         /// Rate of Poisson events (Hz)
@@ -482,8 +482,8 @@ class SONATA_API SimulationConfig
         double riseTime{};
         /// The decay time of the bi-exponential shots (ms)
         double decayTime{};
-        /// Override the random seed to introduce correlations between cells
-        int randomSeed{};
+        /// Override the random seed to introduce correlations between cells, default = None
+        nonstd::optional<int> randomSeed{nonstd::nullopt};
         /// Timestep of generated signal in ms. Default is 0.25 ms
         double dt{};
         /// The coefficient of variation (sd/mean) of gamma-distributed amplitudes
@@ -501,8 +501,8 @@ class SONATA_API SimulationConfig
         double riseTime{};
         /// The decay time of the bi-exponential shots (ms)
         double decayTime{};
-        /// Override the random seed to introduce correlations between cells
-        int randomSeed{};
+        /// Override the random seed to introduce correlations between cells, default = None
+        nonstd::optional<int> randomSeed{nonstd::nullopt};
         /// Timestep of generated signal in ms. Default is 0.25 ms
         double dt{};
         /// The coefficient of variation (sd/mean) of gamma-distributed amplitudes
@@ -520,8 +520,8 @@ class SONATA_API SimulationConfig
         double reversal{};
         /// Timestep of generated signal in ms. Default is 0.25 ms
         double dt{};
-        /// Override the random seed to introduce correlations between cells
-        int randomSeed{};
+        /// Override the random seed to introduce correlations between cells, default = None
+        nonstd::optional<int> randomSeed{nonstd::nullopt};
         /// Signal mean in nA (current_clamp) or uS (conductance)
         double mean{};
         /// Signal std dev in nA (current_clamp) or uS (conductance)
@@ -535,8 +535,8 @@ class SONATA_API SimulationConfig
         double reversal{};
         /// Timestep of generated signal in ms. Default is 0.25 ms
         double dt{};
-        /// Override the random seed to introduce correlations between cells
-        int randomSeed{};
+        /// Override the random seed to introduce correlations between cells, default = None
+        nonstd::optional<int> randomSeed{nonstd::nullopt};
         /// Signal mean as percentage of a cell’s threshold current (current_clamp) or inverse input
         /// resistance (conductance)
         double meanPercent{};
diff --git a/python/generated/docstrings.h b/python/generated/docstrings.h
index fc5c958d..8572f911 100644
--- a/python/generated/docstrings.h
+++ b/python/generated/docstrings.h
@@ -235,6 +235,8 @@ static const char *__doc_bbp_sonata_NodeSets_NodeSets_2 = R"doc()doc";
 
 static const char *__doc_bbp_sonata_NodeSets_NodeSets_3 = R"doc()doc";
 
+static const char *__doc_bbp_sonata_NodeSets_NodeSets_4 = R"doc()doc";
+
 static const char *__doc_bbp_sonata_NodeSets_fromFile = R"doc(Open a SONATA `node sets` file from a path */)doc";
 
 static const char *__doc_bbp_sonata_NodeSets_impl = R"doc()doc";
@@ -652,7 +654,9 @@ static const char *__doc_bbp_sonata_SimulationConfig_InputAbsoluteShotNoise_dt =
 
 static const char *__doc_bbp_sonata_SimulationConfig_InputAbsoluteShotNoise_mean = R"doc(Signal mean in nA (current_clamp) or uS (conductance).)doc";
 
-static const char *__doc_bbp_sonata_SimulationConfig_InputAbsoluteShotNoise_randomSeed = R"doc(Override the random seed to introduce correlations between cells)doc";
+static const char *__doc_bbp_sonata_SimulationConfig_InputAbsoluteShotNoise_randomSeed =
+R"doc(Override the random seed to introduce correlations between cells,
+default = None)doc";
 
 static const char *__doc_bbp_sonata_SimulationConfig_InputAbsoluteShotNoise_riseTime = R"doc(The rise time of the bi-exponential shots (ms))doc";
 
@@ -740,7 +744,9 @@ static const char *__doc_bbp_sonata_SimulationConfig_InputOrnsteinUhlenbeck_dt =
 
 static const char *__doc_bbp_sonata_SimulationConfig_InputOrnsteinUhlenbeck_mean = R"doc(Signal mean in nA (current_clamp) or uS (conductance))doc";
 
-static const char *__doc_bbp_sonata_SimulationConfig_InputOrnsteinUhlenbeck_randomSeed = R"doc(Override the random seed to introduce correlations between cells)doc";
+static const char *__doc_bbp_sonata_SimulationConfig_InputOrnsteinUhlenbeck_randomSeed =
+R"doc(Override the random seed to introduce correlations between cells,
+default = None)doc";
 
 static const char *__doc_bbp_sonata_SimulationConfig_InputOrnsteinUhlenbeck_reversal = R"doc(Reversal potential for conductance injection in mV. Default is 0)doc";
 
@@ -772,7 +778,9 @@ static const char *__doc_bbp_sonata_SimulationConfig_InputRelativeOrnsteinUhlenb
 R"doc(Signal mean as percentage of a cell’s threshold current
 (current_clamp) or inverse input resistance (conductance))doc";
 
-static const char *__doc_bbp_sonata_SimulationConfig_InputRelativeOrnsteinUhlenbeck_randomSeed = R"doc(Override the random seed to introduce correlations between cells)doc";
+static const char *__doc_bbp_sonata_SimulationConfig_InputRelativeOrnsteinUhlenbeck_randomSeed =
+R"doc(Override the random seed to introduce correlations between cells,
+default = None)doc";
 
 static const char *__doc_bbp_sonata_SimulationConfig_InputRelativeOrnsteinUhlenbeck_reversal = R"doc(Reversal potential for conductance injection in mV. Default is 0)doc";
 
@@ -794,7 +802,9 @@ static const char *__doc_bbp_sonata_SimulationConfig_InputRelativeShotNoise_mean
 R"doc(Signal mean as percentage of a cell’s threshold current
 (current_clamp) or inverse input resistance (conductance))doc";
 
-static const char *__doc_bbp_sonata_SimulationConfig_InputRelativeShotNoise_randomSeed = R"doc(Override the random seed to introduce correlations between cells)doc";
+static const char *__doc_bbp_sonata_SimulationConfig_InputRelativeShotNoise_randomSeed =
+R"doc(Override the random seed to introduce correlations between cells,
+default = None)doc";
 
 static const char *__doc_bbp_sonata_SimulationConfig_InputRelativeShotNoise_riseTime = R"doc(The rise time of the bi-exponential shots (ms))doc";
 
@@ -822,7 +832,9 @@ static const char *__doc_bbp_sonata_SimulationConfig_InputShotNoise_decayTime =
 
 static const char *__doc_bbp_sonata_SimulationConfig_InputShotNoise_dt = R"doc(Timestep of generated signal in ms. Default is 0.25 ms)doc";
 
-static const char *__doc_bbp_sonata_SimulationConfig_InputShotNoise_randomSeed = R"doc(Override the random seed to introduce correlations between cells)doc";
+static const char *__doc_bbp_sonata_SimulationConfig_InputShotNoise_randomSeed =
+R"doc(Override the random seed to introduce correlations between cells,
+default = None)doc";
 
 static const char *__doc_bbp_sonata_SimulationConfig_InputShotNoise_rate = R"doc(Rate of Poisson events (Hz))doc";
 
diff --git a/src/config.cpp b/src/config.cpp
index 23f23e47..ed1785aa 100644
--- a/src/config.cpp
+++ b/src/config.cpp
@@ -344,7 +344,6 @@ void parseVariantType(const nlohmann::json& it, variantValueType& var) {
 SimulationConfig::Input parseInputModule(const nlohmann::json& valueIt,
                                          const SimulationConfig::InputBase::Module module,
                                          const std::string& basePath,
-                                         int randomSeed,
                                          const std::string& debugStr) {
     using Module = SimulationConfig::InputBase::Module;
 
@@ -415,7 +414,7 @@ SimulationConfig::Input parseInputModule(const nlohmann::json& valueIt,
         parseCommon(ret);
         parseMandatory(valueIt, "rise_time", debugStr, ret.riseTime);
         parseMandatory(valueIt, "decay_time", debugStr, ret.decayTime);
-        parseOptional(valueIt, "random_seed", ret.randomSeed, {randomSeed});
+        parseOptional(valueIt, "random_seed", ret.randomSeed);
         parseOptional(valueIt, "dt", ret.dt, {0.25});
         parseMandatory(valueIt, "rate", debugStr, ret.rate);
         parseMandatory(valueIt, "amp_mean", debugStr, ret.ampMean);
@@ -428,7 +427,7 @@ SimulationConfig::Input parseInputModule(const nlohmann::json& valueIt,
 
         parseMandatory(valueIt, "rise_time", debugStr, ret.riseTime);
         parseMandatory(valueIt, "decay_time", debugStr, ret.decayTime);
-        parseOptional(valueIt, "random_seed", ret.randomSeed, {randomSeed});
+        parseOptional(valueIt, "random_seed", ret.randomSeed);
         parseOptional(valueIt, "dt", ret.dt, {0.25});
         parseMandatory(valueIt, "amp_cv", debugStr, ret.ampCv);
         parseMandatory(valueIt, "mean_percent", debugStr, ret.meanPercent);
@@ -441,7 +440,7 @@ SimulationConfig::Input parseInputModule(const nlohmann::json& valueIt,
 
         parseMandatory(valueIt, "rise_time", debugStr, ret.riseTime);
         parseMandatory(valueIt, "decay_time", debugStr, ret.decayTime);
-        parseOptional(valueIt, "random_seed", ret.randomSeed, {randomSeed});
+        parseOptional(valueIt, "random_seed", ret.randomSeed);
         parseOptional(valueIt, "dt", ret.dt, {0.25});
         parseMandatory(valueIt, "amp_cv", debugStr, ret.ampCv);
         parseMandatory(valueIt, "mean", debugStr, ret.mean);
@@ -473,7 +472,7 @@ SimulationConfig::Input parseInputModule(const nlohmann::json& valueIt,
         parseCommon(ret);
         parseMandatory(valueIt, "tau", debugStr, ret.tau);
         parseOptional(valueIt, "reversal", ret.reversal);
-        parseOptional(valueIt, "random_seed", ret.randomSeed, {randomSeed});
+        parseOptional(valueIt, "random_seed", ret.randomSeed);
         parseOptional(valueIt, "dt", ret.dt, {0.25});
 
         parseMandatory(valueIt, "mean", debugStr, ret.mean);
@@ -485,7 +484,7 @@ SimulationConfig::Input parseInputModule(const nlohmann::json& valueIt,
         parseCommon(ret);
         parseMandatory(valueIt, "tau", debugStr, ret.tau);
         parseOptional(valueIt, "reversal", ret.reversal);
-        parseOptional(valueIt, "random_seed", ret.randomSeed, {randomSeed});
+        parseOptional(valueIt, "random_seed", ret.randomSeed);
         parseOptional(valueIt, "dt", ret.dt, {0.25});
 
         parseMandatory(valueIt, "mean_percent", debugStr, ret.meanPercent);
@@ -1016,8 +1015,7 @@ class SimulationConfig::Parser
             InputBase::Module module;
             parseMandatory(valueIt, "module", debugStr, module);
 
-            const auto input =
-                parseInputModule(valueIt, module, _basePath, parseRun().randomSeed, debugStr);
+            const auto input = parseInputModule(valueIt, module, _basePath, debugStr);
             result[it.key()] = input;
 
             auto mismatchingModuleInputType = [&it]() {


Test patch:
diff --git a/python/tests/test_config.py b/python/tests/test_config.py
index 1614e355..ac1e4d33 100644
--- a/python/tests/test_config.py
+++ b/python/tests/test_config.py
@@ -449,6 +449,7 @@ def test_basic(self):
         self.assertEqual(self.config.input('ex_subthreshold').percent_less, 80)
         self.assertEqual(self.config.input('ex_shotnoise').rise_time, 0.4)
         self.assertEqual(self.config.input('ex_shotnoise').amp_mean, 70)
+        self.assertEqual(self.config.input('ex_shotnoise').random_seed, None)
 
         self.assertEqual(self.config.input('ex_hyperpolarizing').duration, 1000)
 
@@ -468,7 +469,7 @@ def test_basic(self):
         self.assertEqual(self.config.input('ex_rel_shotnoise').delay, 0)
         self.assertEqual(self.config.input('ex_rel_shotnoise').duration, 1000)
         self.assertEqual(self.config.input('ex_rel_shotnoise').node_set, "L5E")
-        self.assertEqual(self.config.input('ex_rel_shotnoise').random_seed, self.config.run.random_seed)
+        self.assertEqual(self.config.input('ex_rel_shotnoise').random_seed, 230522)
         self.assertEqual(self.config.input('ex_rel_shotnoise').dt, 0.25)
 
         self.assertEqual(self.config.input('ex_replay').input_type.name, 'spikes')
@@ -485,6 +486,7 @@ def test_basic(self):
         self.assertEqual(self.config.input('ex_abs_shotnoise').amp_cv, 0.63)
         self.assertEqual(self.config.input('ex_abs_shotnoise').mean, 50)
         self.assertEqual(self.config.input('ex_abs_shotnoise').sigma, 5)
+        self.assertEqual(self.config.input('ex_abs_shotnoise').random_seed, None)
 
         self.assertEqual(self.config.input('ex_OU').module.name, "ornstein_uhlenbeck")
         self.assertEqual(self.config.input('ex_OU').input_type.name, "conductance")
@@ -492,12 +494,14 @@ def test_basic(self):
         self.assertEqual(self.config.input('ex_OU').reversal, 10)
         self.assertEqual(self.config.input('ex_OU').mean, 50)
         self.assertEqual(self.config.input('ex_OU').sigma, 5)
+        self.assertEqual(self.config.input('ex_OU').random_seed, None)
 
         self.assertEqual(self.config.input('ex_rel_OU').input_type.name, "current_clamp")
         self.assertEqual(self.config.input('ex_rel_OU').tau, 2.8)
         self.assertEqual(self.config.input('ex_rel_OU').reversal, 0)
         self.assertEqual(self.config.input('ex_rel_OU').mean_percent, 70)
         self.assertEqual(self.config.input('ex_rel_OU').sd_percent, 10)
+        self.assertEqual(self.config.input('ex_rel_OU').random_seed, 230522)
 
         self.assertEqual(self.config.input('ex_seclamp').voltage, 1.1)
         self.assertEqual(self.config.input('ex_seclamp').series_resistance, 0.5)
diff --git a/tests/data/config/simulation_config.json b/tests/data/config/simulation_config.json
index f551f5aa..6f059e18 100644
--- a/tests/data/config/simulation_config.json
+++ b/tests/data/config/simulation_config.json
@@ -148,7 +148,8 @@
             "amp_cv": 0.63,
             "mean_percent": 70,
             "sd_percent": 40,
-            "node_set": "L5E"
+            "node_set": "L5E",
+            "random_seed": 230522
         },
         "ex_abs_shotnoise": {
             "input_type": "conductance",
@@ -198,7 +199,8 @@
             "tau": 2.8,
             "mean_percent": 70,
             "sd_percent": 10,
-            "node_set": "Column"
+            "node_set": "Column",
+            "random_seed": 230522
         }
     },
     "connection_overrides": [
diff --git a/tests/test_config.cpp b/tests/test_config.cpp
index 9b5e1e3a..967975ed 100644
--- a/tests/test_config.cpp
+++ b/tests/test_config.cpp
@@ -466,7 +466,7 @@ TEST_CASE("SimulationConfig") {
             CHECK(input.ampMean == 70);
             CHECK(input.ampVar == 40);
             CHECK(input.rate == 4);
-            CHECK(input.randomSeed == 201506);
+            CHECK(input.randomSeed == nonstd::nullopt);
             CHECK(input.riseTime == 0.4);
             CHECK(input.decayTime == 4);
         }
@@ -480,7 +480,7 @@ TEST_CASE("SimulationConfig") {
             CHECK(input.ampCv == 0.63);
             CHECK(input.meanPercent == 70);
             CHECK(input.sdPercent == 40);
-            CHECK(input.randomSeed == 201506);
+            CHECK(input.randomSeed == 230522);
             CHECK(input.riseTime == 0.4);
             CHECK(input.decayTime == 4);
         }
@@ -520,6 +520,7 @@ TEST_CASE("SimulationConfig") {
             CHECK(input.ampCv == 0.63);
             CHECK(input.mean == 50);
             CHECK(input.sigma == 5);
+            CHECK(input.randomSeed == nonstd::nullopt);
         }
         {
             const auto input = nonstd::get<SimulationConfig::InputOrnsteinUhlenbeck>(
@@ -530,6 +531,7 @@ TEST_CASE("SimulationConfig") {
             CHECK(input.reversal == 10);
             CHECK(input.mean == 50);
             CHECK(input.sigma == 5);
+            CHECK(input.randomSeed == nonstd::nullopt);
         }
         {
             const auto input = nonstd::get<SimulationConfig::InputRelativeOrnsteinUhlenbeck>(
@@ -540,6 +542,7 @@ TEST_CASE("SimulationConfig") {
             CHECK(input.reversal == 0);
             CHECK(input.meanPercent == 70);
             CHECK(input.sdPercent == 10);
+            CHECK(input.randomSeed == 230522);
         }
 
         CHECK(config.listInputNames() == std::set<std::string>{"ex_abs_shotnoise",