-
Notifications
You must be signed in to change notification settings - Fork 276
SP: add test for different constructor vs setter behavior #1387
Conversation
SP behaves differently when constructor vs setters are used to set the same parameters!
sp.setPotentialPct(0.5); | ||
sp.setGlobalInhibition(false); | ||
sp.setLocalAreaDensity(0.02); //2% active cols | ||
sp.setNumActiveColumnsPerInhArea(0); //mutex with above ^^ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suspect an issue in these 2 values, which are mutually exclusive. the Num Active Cols per Inh Area should never be zero
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EDIT: fixed the difference
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
also causes difference
this fixes the issue with different SP states when constructor/setter is used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Provided test and fixed the setters, now behaves correctly and all tests pass.
@@ -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) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this caused the difference, because constructor does implicitly crop to bounds, while setter didn't.
TODO: do implicitly, or throw NTA_ASSERT?
@@ -196,6 +196,7 @@ Real SpatialPooler::getPotentialPct() const | |||
|
|||
void SpatialPooler::setPotentialPct(Real potentialPct) | |||
{ | |||
NTA_ASSERT(potentialPct > 0 && potentialPct <= 1); //bounds check |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
turn assert to always check?
@@ -219,7 +220,7 @@ void SpatialPooler::setNumActiveColumnsPerInhArea( | |||
{ | |||
NTA_ASSERT(numActiveColumnsPerInhArea > 0); | |||
numActiveColumnsPerInhArea_ = numActiveColumnsPerInhArea; | |||
localAreaDensity_ = 0; | |||
localAreaDensity_ = -1; //disabling |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixes incompatibility with API documentation
@rhyolight atomic and well tested PR, can we please review and merge asap? other PRs are blocked by this. |
Closing for freeze of this repo, moved to htm-community#5 |
SP behaves differently when constructor vs setters are used to set the
same parameters!
Fixes #1386