From e169e2791f1875cd2d70005264eace178d186edc Mon Sep 17 00:00:00 2001 From: neworderofjamie Date: Tue, 10 Aug 2021 17:33:29 +0100 Subject: [PATCH 1/2] no reason population names/custom update group names shouldn't start with numbers --- include/genn/genn/currentSource.h | 2 +- include/genn/genn/customUpdate.h | 4 ++-- include/genn/genn/gennUtils.h | 11 ++++++++--- include/genn/genn/neuronGroup.h | 2 +- src/genn/genn/gennUtils.cc | 19 +++++++++++++++++-- src/genn/genn/synapseGroup.cc | 2 +- 6 files changed, 30 insertions(+), 10 deletions(-) diff --git a/include/genn/genn/currentSource.h b/include/genn/genn/currentSource.h index f367d29268..aa31a0252c 100644 --- 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 index d8bed255c6..ca42704ed2 100644 --- 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 index 290e69a9ab..cda4a87f29 100644 --- 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 void validateVecNames(const std::vector &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 index a917253851..8bd4db2489 100644 --- 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 index 04e9c70ec6..21b3ef5060 100644 --- 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 ¶mNames) { 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 index 36fadc2b7b..23e39b4948 100644 --- 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(); From 4a91262cbe6e9aabe2d3553219df530fabf09ae0 Mon Sep 17 00:00:00 2001 From: neworderofjamie Date: Tue, 10 Aug 2021 17:33:35 +0100 Subject: [PATCH 2/2] updated tests --- tests/unit/currentSource.cc | 2 +- tests/unit/customUpdate.cc | 4 ++-- tests/unit/gennUtils.cc | 42 +++++++++++++++++++++++++++---------- tests/unit/neuronGroup.cc | 2 +- tests/unit/synapseGroup.cc | 2 +- 5 files changed, 36 insertions(+), 16 deletions(-) diff --git a/tests/unit/currentSource.cc b/tests/unit/currentSource.cc index c1d745609d..f18b2f5647 100644 --- a/tests/unit/currentSource.cc +++ b/tests/unit/currentSource.cc @@ -97,7 +97,7 @@ TEST(CurrentSource, InvalidName) auto *pop = model.addNeuronPopulation("Pop", 10, paramVals, varVals); try { - model.addCurrentSource("6CS", "Pop", {1.0}, {}); + model.addCurrentSource("CS-2", "Pop", {1.0}, {}); FAIL(); } catch(const std::runtime_error &) { diff --git a/tests/unit/customUpdate.cc b/tests/unit/customUpdate.cc index 7658f8c61a..78862e3891 100644 --- 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("1Sum", "CustomUpdate", + model.addCustomUpdate("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", "1CustomUpdate", + model.addCustomUpdate("Sum", "CustomUpdate-1", {}, sumVarValues, sumVarReferences1); FAIL(); } diff --git a/tests/unit/gennUtils.cc b/tests/unit/gennUtils.cc index 26a2a608f1..6ba8ea9d2a 100644 --- 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 index 13b28bfe4a..c3d141ae6c 100644 --- a/tests/unit/neuronGroup.cc +++ b/tests/unit/neuronGroup.cc @@ -154,7 +154,7 @@ TEST(NeuronGroup, InvalidName) { ModelSpec model; try { - model.addNeuronPopulation("0Neurons", 10, {}, {}); + model.addNeuronPopulation("Neurons-0", 10, {}, {}); FAIL(); } catch(const std::runtime_error &) { diff --git a/tests/unit/synapseGroup.cc b/tests/unit/synapseGroup.cc index d238f5ebfe..b37eb190e7 100644 --- a/tests/unit/synapseGroup.cc +++ b/tests/unit/synapseGroup.cc @@ -702,7 +702,7 @@ TEST(SynapseGroup, InvalidName) auto *post = model.addNeuronPopulation("Post", 10, paramVals, varVals); try { model.addSynapsePopulation( - "6Syn", SynapseMatrixType::DENSE_GLOBALG, NO_DELAY, + "Syn-6", SynapseMatrixType::DENSE_GLOBALG, NO_DELAY, "Pre", "Post", {}, {1.0}, {}, {});