Merge pull request #448 from genn-team/less_aggressive_name_validation

Less aggressive name validation
genn-team · Aug 11, 2021 · 78faf0d · 78faf0d
2 parents f3ebe2a + 4a91262
commit 78faf0d
Show file tree

Hide file tree

Showing 11 changed files with 66 additions and 26 deletions.
diff --git a/include/genn/genn/currentSource.h b/include/genn/genn/currentSource.h
@@ -70,7 +70,7 @@ class GENN_EXPORT CurrentSource
         m_ExtraGlobalParamLocation(currentSourceModel->getExtraGlobalParams().size(), defaultExtraGlobalParamLocation)
     {
         // Validate names
-        Utils::validateVarPopName(name, "Current source");
+        Utils::validatePopName(name, "Current source");
         getCurrentSourceModel()->validate();
     }
 

diff --git a/include/genn/genn/customUpdate.h b/include/genn/genn/customUpdate.h
@@ -58,8 +58,8 @@ class GENN_EXPORT CustomUpdateBase
         m_Batched(false)
     {
         // Validate names
-        Utils::validateVarPopName(name, "Custom update");
-        Utils::validateVarPopName(updateGroupName, "Custom update group name");
+        Utils::validatePopName(name, "Custom update");
+        Utils::validatePopName(updateGroupName, "Custom update group name");
         getCustomUpdateModel()->validate();
     }
 

diff --git a/include/genn/genn/gennUtils.h b/include/genn/genn/gennUtils.h
@@ -55,9 +55,14 @@ GENN_EXPORT bool isTypePointerToPointer(const std::string &type);
 GENN_EXPORT std::string getUnderlyingType(const std::string &type);
 
 //--------------------------------------------------------------------------
-//! \brief Is the variable/population name valid? GeNN variables and population names must obey C variable naming rules
+//! \brief Is the variable name valid? GeNN variable names must obey C variable naming rules
 //--------------------------------------------------------------------------
-GENN_EXPORT void validateVarPopName(const std::string &name, const std::string &description);
+GENN_EXPORT void validateVarName(const std::string &name, const std::string &description);
+
+//--------------------------------------------------------------------------
+//! \brief Is the population name valid? GeNN population names obey C variable naming rules but can start with a number
+//--------------------------------------------------------------------------
+GENN_EXPORT void validatePopName(const std::string &name, const std::string &description);
 
 //--------------------------------------------------------------------------
 //! \brief Are all the parameter names in vector valid? GeNN variables and population names must obey C variable naming rules
@@ -71,7 +76,7 @@ template<typename T>
 void validateVecNames(const std::vector<T> &vec, const std::string &description)
 {
     for(const auto &v : vec) {
-        validateVarPopName(v.name, description);
+        validateVarName(v.name, description);
     }
 }
 

diff --git a/include/genn/genn/neuronGroup.h b/include/genn/genn/neuronGroup.h
@@ -207,7 +207,7 @@ class GENN_EXPORT NeuronGroup
         m_SpikeRecordingEnabled(false), m_SpikeEventRecordingEnabled(false)
     {
         // Validate names
-        Utils::validateVarPopName(name, "Neuron group");
+        Utils::validatePopName(name, "Neuron group");
         getNeuronModel()->validate();
     }
 

diff --git a/src/genn/genn/gennUtils.cc b/src/genn/genn/gennUtils.cc
@@ -94,7 +94,7 @@ std::string getUnderlyingType(const std::string &type)
     }
 }
 //--------------------------------------------------------------------------
-void validateVarPopName(const std::string &name, const std::string &description)
+void validateVarName(const std::string &name, const std::string &description)
 {
     // Empty names aren't valid
     if(name.empty()) {
@@ -114,10 +114,25 @@ void validateVarPopName(const std::string &name, const std::string &description)
     }
 }
 //--------------------------------------------------------------------------
+void validatePopName(const std::string &name, const std::string &description)
+{
+    // Empty names aren't valid
+    if(name.empty()) {
+        throw std::runtime_error(description + " name invalid: cannot be empty");
+    }
+
+    // If any characters aren't underscores or alphanumeric, name isn't valud
+    if(std::any_of(name.cbegin(), name.cend(),
+                   [](char c) { return (c != '_') && !std::isalnum(c); }))
+    {
+        throw std::runtime_error(description + " name invalid: '" + name + "' contains an illegal character");
+    }
+}
+//--------------------------------------------------------------------------
 void validateParamNames(const std::vector<std::string> &paramNames)
 {
     for(const std::string &p : paramNames) {
-        validateVarPopName(p, "Parameter");
+        validateVarName(p, "Parameter");
     }
 }
 }   // namespace utils
diff --git a/src/genn/genn/synapseGroup.cc b/src/genn/genn/synapseGroup.cc
@@ -440,7 +440,7 @@ SynapseGroup::SynapseGroup(const std::string &name, SynapseMatrixType matrixType
         m_ConnectivityExtraGlobalParamLocation(connectivityInitialiser.getSnippet()->getExtraGlobalParams().size(), defaultExtraGlobalParamLocation), m_PSModelTargetName(name)
 {
     // Validate names
-    Utils::validateVarPopName(name, "Synapse group");
+    Utils::validatePopName(name, "Synapse group");
     getWUModel()->validate();
     getPSModel()->validate();
 

diff --git a/tests/unit/currentSource.cc b/tests/unit/currentSource.cc
@@ -97,7 +97,7 @@ TEST(CurrentSource, InvalidName)
     auto *pop = model.addNeuronPopulation<NeuronModels::Izhikevich>("Pop", 10, paramVals, varVals);
 
     try {
-        model.addCurrentSource<CurrentSourceModels::DC>("6CS", "Pop", {1.0}, {});
+        model.addCurrentSource<CurrentSourceModels::DC>("CS-2", "Pop", {1.0}, {});
         FAIL();
     }
     catch(const std::runtime_error &) {

diff --git a/tests/unit/customUpdate.cc b/tests/unit/customUpdate.cc
@@ -596,7 +596,7 @@ TEST(CustomUpdates, InvalidName)
     Sum::VarReferences sumVarReferences1(createVarRef(ng1, "V"), createVarRef(ng1, "U"));
 
     try {
-        model.addCustomUpdate<Sum>("1Sum", "CustomUpdate",
+        model.addCustomUpdate<Sum>("Sum-1", "CustomUpdate",
                                    {}, sumVarValues, sumVarReferences1);
         FAIL();
     }
@@ -617,7 +617,7 @@ TEST(CustomUpdates, InvalidUpdateGroupName)
     Sum::VarReferences sumVarReferences1(createVarRef(ng1, "V"), createVarRef(ng1, "U"));
 
     try {
-        model.addCustomUpdate<Sum>("Sum", "1CustomUpdate",
+        model.addCustomUpdate<Sum>("Sum", "CustomUpdate-1",
                                    {}, sumVarValues, sumVarReferences1);
         FAIL();
     }

diff --git a/tests/unit/gennUtils.cc b/tests/unit/gennUtils.cc
@@ -10,10 +10,20 @@
 //--------------------------------------------------------------------------
 namespace
 {
-void validateVarPopNameDeathTest(const std::string &name)
+void validatePopNameDeathTest(const std::string &name)
 {
     try {
-        Utils::validateVarPopName(name, "test");
+        Utils::validatePopName(name, "test");
+        FAIL();
+    }
+
+    catch(const std::runtime_error &) {
+    }
+}
+void validateVarNameDeathTest(const std::string &name)
+{
+    try {
+        Utils::validateVarName(name, "test");
         FAIL();
     }
 
@@ -27,16 +37,26 @@ void validateVarPopNameDeathTest(const std::string &name)
 //--------------------------------------------------------------------------
 TEST(GeNNUtils, ValidateVarPopName)
 {
-    Utils::validateVarPopName("test", "test");
-    Utils::validateVarPopName("Test", "test");
-    Utils::validateVarPopName("test123", "test");
-    Utils::validateVarPopName("test_123", "test");
-    Utils::validateVarPopName("_test_123", "test");
+    Utils::validateVarName("test", "test");
+    Utils::validateVarName("Test", "test");
+    Utils::validateVarName("test123", "test");
+    Utils::validateVarName("test_123", "test");
+    Utils::validateVarName("_test_123", "test");
+
+    Utils::validatePopName("test", "test");
+    Utils::validatePopName("Test", "test");
+    Utils::validatePopName("test123", "test");
+    Utils::validatePopName("test_123", "test");
+    Utils::validatePopName("_test_123", "test");
+    Utils::validatePopName("1test", "test");
 
-    validateVarPopNameDeathTest("");
-    validateVarPopNameDeathTest("1test");
-    validateVarPopNameDeathTest("test.test");
-    validateVarPopNameDeathTest("test-test");
+    validateVarNameDeathTest("");
+    validateVarNameDeathTest("1test");
+    validateVarNameDeathTest("test.test");
+    validateVarNameDeathTest("test-test");
+    validatePopNameDeathTest("");
+    validatePopNameDeathTest("test.test");
+    validatePopNameDeathTest("test-test");
 }
 //--------------------------------------------------------------------------
 TEST(GeNNUtils, ValidateParamNames)

diff --git a/tests/unit/neuronGroup.cc b/tests/unit/neuronGroup.cc
@@ -154,7 +154,7 @@ TEST(NeuronGroup, InvalidName)
 {
     ModelSpec model;
     try {
-        model.addNeuronPopulation<NeuronModels::SpikeSource>("0Neurons", 10, {}, {});
+        model.addNeuronPopulation<NeuronModels::SpikeSource>("Neurons-0", 10, {}, {});
      FAIL();
     }
     catch(const std::runtime_error &) {

diff --git a/tests/unit/synapseGroup.cc b/tests/unit/synapseGroup.cc
@@ -702,7 +702,7 @@ TEST(SynapseGroup, InvalidName)
     auto *post = model.addNeuronPopulation<NeuronModels::Izhikevich>("Post", 10, paramVals, varVals);
     try {
         model.addSynapsePopulation<WeightUpdateModels::StaticPulse, PostsynapticModels::DeltaCurr>(
-            "6Syn", SynapseMatrixType::DENSE_GLOBALG, NO_DELAY,
+            "Syn-6", SynapseMatrixType::DENSE_GLOBALG, NO_DELAY,
             "Pre", "Post",
             {}, {1.0},
             {}, {});