From aef93565ebcea0e461b5a14cfff35fcd60f1c627 Mon Sep 17 00:00:00 2001 From: Marek Otahal Date: Thu, 11 Jan 2018 04:17:10 +0100 Subject: [PATCH 1/4] SP: add test for different constructor vs setter behavior SP behaves differently when constructor vs setters are used to set the same parameters! --- .../unit/algorithms/SpatialPoolerTest.cpp | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/test/unit/algorithms/SpatialPoolerTest.cpp b/src/test/unit/algorithms/SpatialPoolerTest.cpp index 88f3882a2e..7e7ae7a196 100644 --- a/src/test/unit/algorithms/SpatialPoolerTest.cpp +++ b/src/test/unit/algorithms/SpatialPoolerTest.cpp @@ -2222,6 +2222,34 @@ namespace { EXPECT_EQ(0, countNonzero(activeColumns)); } + + TEST(SpatialPoolerTest, testDifferentConstructorVsSetterBehavior) + { + /** this test exposes wrong behavior, where SP created via constructor + behaves differently to a SP via setters (setXXX()), both with the same + params. + */ + SpatialPooler spConstruct{std::vector{10} /* input*/, std::vector{2048}/* SP output cols XXX sensitive*/, + /*pot radius*/ 20, //each col sees + /*pot pct*/ 0.5, //XXX sensitive + /*global inhibition*/ false, //XXX sensitive + /*Real localAreaDensity=*/0.02, //2% active cols + /*UInt numActiveColumnsPerInhArea=*/0, //mutex with above ^^ //XXX sensitive +}; + + SpatialPooler sp{std::vector{10} /* input*/, std::vector{2048}/* SP output cols */}; +sp.setPotentialRadius(20); +sp.setPotentialPct(0.5); +sp.setGlobalInhibition(false); +sp.setLocalAreaDensity(0.02); //2% active cols +sp.setNumActiveColumnsPerInhArea(0); //mutex with above ^^ + + +// EXPECT_EQ(spConstruct, sp); //FIXME how compare 2 SP +check_spatial_eq(spConstruct, sp); +} + + TEST(SpatialPoolerTest, testSaveLoad) { const char* filename = "SpatialPoolerSerialization.tmp"; From 00b247af1e27dd3e16594f039dfe4f4dbba9e3b8 Mon Sep 17 00:00:00 2001 From: Marek Otahal Date: Fri, 12 Jan 2018 12:32:46 +0100 Subject: [PATCH 2/4] SP: fix setActiveColumnsPerInhArea behavior this sets the num active columns, but also has to disable mutex localAreaDensity, according to the API it's done by = -1; but the setter wrongly used =0; fixed --- src/nupic/algorithms/SpatialPooler.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/nupic/algorithms/SpatialPooler.cpp b/src/nupic/algorithms/SpatialPooler.cpp index bd8b366173..316ea46a7f 100644 --- a/src/nupic/algorithms/SpatialPooler.cpp +++ b/src/nupic/algorithms/SpatialPooler.cpp @@ -219,7 +219,7 @@ void SpatialPooler::setNumActiveColumnsPerInhArea( { NTA_ASSERT(numActiveColumnsPerInhArea > 0); numActiveColumnsPerInhArea_ = numActiveColumnsPerInhArea; - localAreaDensity_ = 0; + localAreaDensity_ = -1; //disabling } Real SpatialPooler::getLocalAreaDensity() const @@ -231,7 +231,7 @@ void SpatialPooler::setLocalAreaDensity(Real localAreaDensity) { NTA_ASSERT(localAreaDensity > 0 && localAreaDensity <= 1); localAreaDensity_ = localAreaDensity; - numActiveColumnsPerInhArea_ = 0; + numActiveColumnsPerInhArea_ = 0; //disabling } UInt SpatialPooler::getStimulusThreshold() const From 3514f275b237905421b7504811fabf1a84e96b73 Mon Sep 17 00:00:00 2001 From: Marek Otahal Date: Fri, 12 Jan 2018 13:51:12 +0100 Subject: [PATCH 3/4] SP: add setter test for potentialRadius also causes difference --- src/test/unit/algorithms/SpatialPoolerTest.cpp | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/src/test/unit/algorithms/SpatialPoolerTest.cpp b/src/test/unit/algorithms/SpatialPoolerTest.cpp index 7e7ae7a196..17b2d3090f 100644 --- a/src/test/unit/algorithms/SpatialPoolerTest.cpp +++ b/src/test/unit/algorithms/SpatialPoolerTest.cpp @@ -2242,11 +2242,23 @@ sp.setPotentialRadius(20); sp.setPotentialPct(0.5); sp.setGlobalInhibition(false); sp.setLocalAreaDensity(0.02); //2% active cols -sp.setNumActiveColumnsPerInhArea(0); //mutex with above ^^ +//! sp.setNumActiveColumnsPerInhArea(0); //mutex with above ^^ + // test 1: these two mutex settings had err compared to the API + // (localAreaDensity & numActiveColumnsPerInhArea) + sp.setNumActiveColumnsPerInhArea(5); + EXPECT_EQ(sp.getNumActiveColumnsPerInhArea(), 5); + EXPECT_FLOAT_EQ(sp.getLocalAreaDensity(), -1); //-1 means disabled -// EXPECT_EQ(spConstruct, sp); //FIXME how compare 2 SP -check_spatial_eq(spConstruct, sp); + sp.setLocalAreaDensity(0.02); + EXPECT_EQ(sp.getNumActiveColumnsPerInhArea(), 0); //0 means disabled + EXPECT_FLOAT_EQ(sp.getLocalAreaDensity(), 0.02); + + //test 2: compare potentialRadius + EXPECT_EQ(sp.getPotentialRadius(), spConstruct.getPotentialRadius()); + + // EXPECT_EQ(spConstruct, sp); //FIXME how compare 2 SP + check_spatial_eq(spConstruct, sp); } From d3d2a40783779585468412364ba1b851cebfd161 Mon Sep 17 00:00:00 2001 From: Marek Otahal Date: Fri, 12 Jan 2018 13:59:51 +0100 Subject: [PATCH 4/4] SP: add bounds check for setter PotentialPct, PotentialRadius this fixes the issue with different SP states when constructor/setter is used. --- src/nupic/algorithms/SpatialPooler.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/nupic/algorithms/SpatialPooler.cpp b/src/nupic/algorithms/SpatialPooler.cpp index 316ea46a7f..3617aeb08e 100644 --- a/src/nupic/algorithms/SpatialPooler.cpp +++ b/src/nupic/algorithms/SpatialPooler.cpp @@ -186,7 +186,7 @@ UInt SpatialPooler::getPotentialRadius() const void SpatialPooler::setPotentialRadius(UInt potentialRadius) { - potentialRadius_ = potentialRadius; + potentialRadius_ = std::min(potentialRadius, numInputs_); //crop to numInputs (input size) } Real SpatialPooler::getPotentialPct() const @@ -196,6 +196,7 @@ Real SpatialPooler::getPotentialPct() const void SpatialPooler::setPotentialPct(Real potentialPct) { + NTA_ASSERT(potentialPct > 0 && potentialPct <= 1); //bounds check potentialPct_ = potentialPct; }