[RES-411] Implement New SP Boosting Rules in NuPIC #3411
Conversation
|
By analyzing the blame information on this pull request, we identified @subutai, @scottpurdy and @oxtopus to be potential reviewers |
|
Watching this... |
| @@ -259,6 +258,9 @@ def __init__(self, | |||
| raise InvalidSPParamValueError( | |||
| "Input dimensions must match column dimensions") | |||
|
|
|||
| if maxBoost < 1.0: | |||
There was a problem hiding this comment.
docstring for maxBoost needs to be updated.
| boost factor is linearly interpolated between the points (dutyCycle:0, | ||
| boost:maxFiringBoost) and (dutyCycle:minDuty, boost:1.0). | ||
| becoming active, and hence encourage participation of more columns in the | ||
| learning process. This is a line defined as: |
There was a problem hiding this comment.
"The boosting function is a curve defined as:"
| if self._maxBoost > 1: | ||
| if self._globalInhibition: | ||
| if (self._localAreaDensity > 0): | ||
| density = self._localAreaDensity |
There was a problem hiding this comment.
Just use variable targetDensity here. Then you don't need the line targetDensity = density below.
| targetDensity | ||
| """ | ||
| if self._maxBoost > 1: | ||
| if self._globalInhibition: |
There was a problem hiding this comment.
Add comment explaining intent of logic below
|
|
||
| self._boostFactors[self._activeDutyCycles > | ||
| self._minActiveDutyCycles] = 1.0 | ||
| self._boostFactors = numpy.exp(-( |
There was a problem hiding this comment.
This line could be pretty slow to compute as it is computing exp() for 2048 columns on every time step. It could slow down the spatial pooler's compute time.
| @@ -681,7 +683,7 @@ def getPotential(self, columnIndex, potential): | |||
| def setPotential(self, columnIndex, potential): | |||
| """Sets the potential mapping for a given column. 'potential' size | |||
| must match the number of inputs, and must be greater than _stimulusThreshold """ | |||
| assert(column < self._numColumns) | |||
| assert(columnIndex < self._numColumns) | |||
| @@ -207,7 +205,7 @@ def verifySDRProperties(self): | |||
| def boostTestPhase1(self): | |||
|
|
|||
| y = numpy.zeros(self.columnDimensions, dtype = uintType) | |||
|
|
|||
| self.sp._maxBoost = 1 | |||
There was a problem hiding this comment.
Why do you need to set maxBoost to 1? In an end to end test like this it would be better to keep maxBoost fixed throughout since that is how it is normally run. You can update the test to reflect the desired behavior.
| @@ -343,8 +335,8 @@ def testBoostingPY(self): | |||
| self.boostTestLoop("py") | |||
|
|
|||
|
|
|||
| def testBoostingCPP(self): | |||
| self.boostTestLoop("cpp") | |||
| # def testBoostingCPP(self): | |||
There was a problem hiding this comment.
Put in TODO: comment with a link to Github issue URL
|
@subutai I have incorporated your comments and have updated the spatial_pooler_boost_test. It is ready for review again. |
|
@cogmission This is an important change. |
… ywcui1990-boostingRes
For permanances, use an EPSILON. For boosting, round the boost factor to a couple decimal places. Floating point differences have a larger effect on boosting because it's multiplicative.
Avoid floating point differences with C++ SpatialPooler
|
Jeez... Even the ability to check checks - check failed... :-P |
|
@subutai This is ready for review. @scottpurdy The corresponding changes in nupic.core has been merged. The spatial pooler compatibility tests and boost tests pass on my machine with the master branch of nupic.core. Why do the tests still fail on Travis? |
|
👍, once tests pass. |
|
@ywcui1990 we need to do a new nupic.core release, and then you'll have to modify the requirements.txt as part of these PR to use the new nupic.bindings. |
|
@mrcslws Thanks for the instructions. I will wait for the next release of nupic.core and update the requirement.txt after that. |
|
Here's a nupic.core PR that fixes the remaining compatibility test issue. numenta/nupic.core-legacy#1170 |
It's proving to be too difficult. So our options are: 1. Use shared C code to calculate boost factors and duty cycles for Python and C++ SpatialPoolers 2. Don't try to make them equal. Change the compatibility test to make sure the boost factor is right "enough", and then force them to be exactly equal, similar to what we already do with permanences. This change takes the second approach, which I think is the better approach.
Rather than using a mathy heuristic with an "arbitration" constant, explicitly handle the tie-break case. This mirrors a change in the C++ code. It's needed for compatibility.
Stop trying to get identical boost factors between py and cpp
…ding rules in boosting
@subutai
Fixes #3412
Depends on: numenta/nupic.core-legacy#1164
_updateBoostFactorsfunction in spatial_pooler.pyNote that the spatial_pooler_compatability_test.py fails at this moment, it requires the same change of boosting rules in nupic.core, which is implemented in numenta/nupic.core-legacy#1164