Implement New SP Boosting Rules #1164

ywcui1990 · 2016-11-29T18:04:29Z

Modify the updateBoostFactors function in spatial pooler
Update related spatial pooler test
Round boost factor to a couple decimal places to avoid float point differences
Correct misc typos in spatial_pooler.hpp

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 Python SpatialPooler

numenta-ci · 2016-11-29T18:04:36Z

By analyzing the blame information on this pull request, we identified @scottpurdy, @rcrowder and @mrcslws to be potential reviewers

mrcslws · 2016-11-29T20:00:40Z

src/nupic/algorithms/SpatialPooler.cpp

  {
-    if (minActiveDutyCycles_[i] <= 0)
+    vector<Real> targetDensity(numColumns_, 0);


This vector is really only needed in the local inhibition case. Maybe it'd be better to do the boostFactor computation in two places, and avoid creating this vector every time step in the "global inhibition" case?

In other words, there will be two different lines:

// Global inhibition Real boostFactor = exp(-(activeDutyCycles_[i] - density) * maxBoost_); // Local inhibition Real boostFactor = exp(-(activeDutyCycles_[i] - targetDensity[i]) * maxBoost_);

mrcslws · 2016-11-29T20:41:08Z

src/nupic/algorithms/SpatialPooler.cpp

    {
-      continue;
+      Real density = localAreaDensity_;


I think it'd be more readable (and infinitesimally faster) to handle the two cases in if/else blocks.

Real density; if (numActiveColumnsPerInhArea_ > 0) { // ... } else { density = localAreaDensity_; }

mrcslws · 2016-11-29T20:49:21Z

src/nupic/algorithms/SpatialPooler.cpp

+      {
+        UInt numNeighbors = 0;
+        Real localActivityDensity = 0;
+        for (UInt neighbor : WrappingNeighborhood(i, inhibitionRadius_,


Should this obey the wrapAround_ parameter? i.e. I think there should be one case that uses the Neighborhood and the other that uses the WrappingNeighborhood.

mrcslws · 2016-11-29T21:07:14Z

It might be helpful to have a updateBoostFactorsLocal_ and a updateBoostFactorsGlobal_, similar to how inhibitColumns_ delegates to inhibitColumnsLocal_ and inhibitColumnsGlobal_.

ywcui1990 · 2016-11-30T00:55:44Z

@mrcslws I have included your comments and implements updateBoostFactorsLocal_ and updateBoostFactorsGlobal_. I have made the same change in nupic

mrcslws · 2016-11-30T01:07:15Z

There's a @param maxBoost description in SpatialPooler.hpp that needs the same docstring update.

mrcslws

I think this will be my final feedback. Note that I'm not qualified to review my own EPSILON / round changes. If you merge those, you're my reviewer. :)

mrcslws · 2016-11-30T01:09:41Z

src/nupic/algorithms/SpatialPooler.cpp

-      boostFactors_[i] = 1.0;
-      continue;
+      for (UInt neighbor : Neighborhood(i, inhibitionRadius_,
+                                                columnDimensions_))


Nit: spacing. columnDimensions_ should line up with i.

mrcslws · 2016-11-30T01:16:03Z

src/nupic/algorithms/SpatialPooler.cpp

    }
-    boostFactors_[i] = ((1 - maxBoost_) / minActiveDutyCycles_[i] *
-                        activeDutyCycles_[i]) + maxBoost_;
+    targetDensity[i] = localActivityDensity / numNeighbors;


I'm now realizing that this vector doesn't need to exist here either. We could just combine the 2 loops and avoid having to create this vector.

In other words, we could replace this line with:

Real targetDensity = localActivityDensity / numNeighbors; Real boostFactor = exp(-(activeDutyCycles_[i] - targetDensity) * maxBoost_); // Avoid floating point mismatches between implementations. boostFactors_[i] = round(boostFactor * 100.0) / 100.0;

Note that it makes sense to use the array of targetDensities in Python, since that allows us to do batch numpy operations rather than doing math in Python, which is slow. But in C we don't get any benefit from the vector (that I can see).

mrcslws

Looks good! I'm curious to know what you / others think of this floating point strategy, having a PERMANENCE_EPSILON for permanences and rounding boost factors to the nearest hundredth.

scottpurdy

Done with first pass.

scottpurdy · 2016-11-30T18:19:38Z

src/nupic/algorithms/SpatialPooler.cpp

@@ -813,7 +815,7 @@ void SpatialPooler::updatePermanencesForColumn_(vector<Real>& perm,
  numConnected = 0;
  for (UInt i = 0; i < perm.size(); ++i)
  {
-    if (perm[i] >= synPermConnected_)
+    if (perm[i] >= synPermConnected_ - PERMANENCE_EPSILON)


I don't understand this. If the two values are exactly equal (use of >= instead of > makes me think that is significant) then the epsilon will result in the wrong outcome. If there is some case where different platforms have a slight difference then I don't even see how this would help.

As we discussed, you can think of floating point math as creating a bell curve of possible results. If the "correct" answer is 0.5, the floating point math might result in numbers between 0.4999996 and 0.5000004. With this EPSILON, we move the threshold so that it sits on one side of the entire bell curve, so results will be consistent no matter where it landed in the bell curve.

I do think >= makes sense, because then the code still works correctly if PERMANENCE_EPSILON is set to 0.

scottpurdy · 2016-11-30T18:21:05Z

src/nupic/algorithms/SpatialPooler.cpp

+
+void SpatialPooler::updateBoostFactorsGlobal_()
+{
+  Real targetDensity;


I think it is better to use Real32. We don't need 64 bit precision so better to get deterministic results with explicit # of bits.

A good thing about using Real is that the Python code can then use GetNTAReal() if it wants to use the same precision in its numpy arrays.

scottpurdy · 2016-11-30T18:21:52Z

src/nupic/algorithms/SpatialPooler.cpp

+  Real targetDensity;
+  if (numActiveColumnsPerInhArea_ > 0)
+  {
+    UInt inhibitionArea = pow((Real) (2 * inhibitionRadius_ + 1),


Same, use explicit UInt32

scottpurdy · 2016-11-30T18:22:57Z

src/nupic/algorithms/SpatialPooler.cpp

+                              (Real) columnDimensions_.size());
+    inhibitionArea = min(inhibitionArea, numColumns_);
+    targetDensity = ((Real) numActiveColumnsPerInhArea_) / inhibitionArea;
+    targetDensity = min(targetDensity, (Real) 0.5);


Why the max value? Could use a comment explaining

This is inherited from the inhibitColumns_ function. I actually don't have a good explanation for this logic. Would it be better if we do a parameter check during initialization and throw an error if the targetDensity > 0.5?

Hmm hard to say. @mrcslws ?

Here's what I presume. For local inhibition, the inhibitionRadius_ isn't really a parameter, it changes with the statistics of the data. So if you're using the numActiveColumnsPerInhArea_ parameter, you can accidentally wind up in situations where you're activating way more columns than you want, because the inhibition areas are small.

This does not apply to global inhibition unless someone goes in and manually changes the inhibition radius. ::initialize will set this radius to cover the whole space. So in this code, the inhibition radius is predictable. So I don't think we should perform min check here.

Similarly, I would argue that this logic in inhibitColumns_ should be moved to inhibitColumnsLocal_, and inhibitColumnsGlobal_ should just obey the parameters. Though maybe that's out of scope for this change.

Ultimately this is all a hack that exists because the numActiveColumnsPerInhArea parameter is awkward when mixed with local inhibition. With local inhibition it's probably best to use the localAreaDensity parameter instead.

I'm fine with whatever you guys agree on.

I agree with @mrcslws and I think this change is out of scope of this PR. Marcus, can you create a separate issue for this and close this PR?

Cool, I opened numenta/nupic-legacy#3420

scottpurdy · 2016-11-30T18:24:00Z

src/nupic/algorithms/SpatialPooler.cpp

+
+  for (UInt i = 0; i < numColumns_; ++i)
+  {
+    Real boostFactor = exp(-(activeDutyCycles_[i] - targetDensity)


Would it be simpler to replace:

-(activeDutyCycles_[i] - targetDensity)

with:

(targetDensity - activeDutyCycles_[i])

And why the exp? I understand the subtraction (figure out if duty cycle is higher or lower than target density) and the multiplication (scale the difference from target density from a fraction to the magnitude scale specified by max boost), but I don't understand the exp after that.

The exp ensures several things. First, the boostFactors are always positive. Second, the boostFactor will be one if the activeDutyCycle matches targetDensity. Third, it is monotonic and continuous, so weak columns are boosted and strong columns are suppressed. There are other functions that satisfy the three properties but I prefer exp for its simplicity.

scottpurdy · 2016-11-30T18:29:58Z

src/nupic/algorithms/SpatialPooler.cpp

+                           * maxBoost_);
+
+    // Avoid floating point mismatches between implementations.
+    boostFactors_[i] = round(boostFactor * 100.0) / 100.0;


One way to avoid float precision errors is to just use integers instead. It's a little less readable since you have to read comments to understand what the scale means but avoids lines like this. I wouldn't recommend making this fairly large refactor in this PR, just pointing it out as a perhaps cleaner way to implement things.

Also, you might be able to avoid this line if you use the fixed precision variables types (UInt32, Real32).

Keep in mind that none of these changes from UInt to UInt32 or Real to Real32 will have any effect. It's only when we build with NTA_BIG_INTEGER or NTA_DOUBLE_PRECISION that these become UInt64 / Real64, respectively.

scottpurdy · 2016-11-30T18:31:16Z

src/nupic/algorithms/SpatialPooler.hpp

@@ -944,11 +944,11 @@ namespace nupic

            The column is identified by its index, which reflects the row in
            the matrix, and the permanence is given in 'dense' form, i.e. a full
-            arrray containing all the zeros as well as the non-zero values. It is in
+            array containing all the zeros as well as the non-zero values. It is in


I prefer the pirate speak

Yuwei Cui and others added 12 commits September 23, 2016 14:02

update SP boosting rule

b1bf168

update SP doc

9ad8711

resolve merge conflict

089dbf9

Merge branch 'master' into boostingRes

bfba2b5

Merge branch 'master' into BoostingRes

9d95da0

update boosting rules of spatial pooler

1f57637

Merge branch 'master' into BoostingRes

1b8bcc8

Merge branch 'master' into BoostingRes

35dd74b

Do not update boost factors if maxBoost <=1

d1ad2af

update spatial pooler tests

204face

Avoid floating point differences with Python SpatialPooler

45eb5bf

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.

Merge pull request #1 from mrcslws/ywcui1990-BoostingRes

928e73f

Avoid floating point differences with Python SpatialPooler

ywcui1990 mentioned this pull request Nov 29, 2016

[RES-411] Implement New SP Boosting Rules in NuPIC numenta/nupic-legacy#3411

Merged

mrcslws reviewed Nov 29, 2016

View reviewed changes

Implement update boost factors local and global

fc42101

update docstring for maxBoost parameter

35a4867

mrcslws reviewed Nov 30, 2016

View reviewed changes

do not create targetDensity array in updateBoostFactors

0e06012

mrcslws approved these changes Nov 30, 2016

View reviewed changes

scottpurdy suggested changes Nov 30, 2016

View reviewed changes

simplify boost factor calculation

be8dffc

scottpurdy approved these changes Nov 30, 2016

View reviewed changes

mrcslws mentioned this pull request Dec 1, 2016

SP with global inhibition should always obey numActiveColumnsPerInhArea numenta/nupic-legacy#3420

Open

mrcslws merged commit 3e71bb5 into numenta:master Dec 1, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement New SP Boosting Rules #1164

Implement New SP Boosting Rules #1164

ywcui1990 commented Nov 29, 2016 •

edited

Loading

numenta-ci commented Nov 29, 2016

mrcslws Nov 29, 2016

mrcslws Nov 29, 2016

mrcslws Nov 29, 2016

mrcslws commented Nov 29, 2016

ywcui1990 commented Nov 30, 2016

mrcslws commented Nov 30, 2016

mrcslws left a comment

mrcslws Nov 30, 2016

mrcslws Nov 30, 2016

mrcslws Nov 30, 2016

mrcslws left a comment

scottpurdy left a comment

scottpurdy Nov 30, 2016

mrcslws Nov 30, 2016

scottpurdy Nov 30, 2016

mrcslws Nov 30, 2016

scottpurdy Nov 30, 2016

scottpurdy Nov 30, 2016

ywcui1990 Nov 30, 2016

scottpurdy Nov 30, 2016

mrcslws Nov 30, 2016

scottpurdy Dec 1, 2016

ywcui1990 Dec 1, 2016

mrcslws Dec 1, 2016

scottpurdy Nov 30, 2016

scottpurdy Nov 30, 2016

ywcui1990 Nov 30, 2016

scottpurdy Nov 30, 2016

mrcslws Nov 30, 2016

scottpurdy Nov 30, 2016

Implement New SP Boosting Rules #1164

Implement New SP Boosting Rules #1164

Conversation

ywcui1990 commented Nov 29, 2016 • edited Loading

numenta-ci commented Nov 29, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrcslws commented Nov 29, 2016

ywcui1990 commented Nov 30, 2016

mrcslws commented Nov 30, 2016

mrcslws left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrcslws left a comment

Choose a reason for hiding this comment

scottpurdy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ywcui1990 commented Nov 29, 2016 •

edited

Loading