-
Notifications
You must be signed in to change notification settings - Fork 75
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
SP fix global inhibition returns less than numDesired columns #542
base: master
Are you sure you want to change the base?
Conversation
the global inhibition should return numDesired active cols, but it returns much less once sub threshold cols are removed
globa inh does not return nDesired columns how it should, even if it could have.
src/htm/algorithms/SpatialPooler.cpp
Outdated
{ | ||
if (overlaps_[a] == overlaps_[b]) { | ||
same_overlap++; | ||
return a > b; //but we also need this for deterministic results |
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.
problem 1: we are removing columns with the same overlap from the top n selection.
src/htm/algorithms/SpatialPooler.cpp
Outdated
// Do a partial sort to divide the winners from the losers. This sort is | ||
// faster than a regular sort because it stops after it partitions the | ||
// elements about the Nth element, with all elements on their correct side of | ||
// the Nth element. | ||
std::nth_element( | ||
/* | ||
std::nth_element( | ||
activeColumns.begin(), | ||
activeColumns.begin() + numDesired, | ||
activeColumns.end(), | ||
compare); | ||
// Remove the columns which lost the competition. | ||
activeColumns.resize(numDesired); |
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.
Problem 2: here we crop to numDesired, but too early..
activeColumns.begin(), | ||
activeColumns.begin() + numDesired, | ||
activeColumns.end(), | ||
compare); | ||
// Remove the columns which lost the competition. | ||
activeColumns.resize(numDesired); | ||
*/ | ||
// Finish sorting the winner columns by their overlap. | ||
std::sort(activeColumns.begin(), activeColumns.end(), compare); | ||
// Remove sub-threshold winners | ||
while( !activeColumns.empty() && | ||
overlaps[activeColumns.back()] < stimulusThreshold_) | ||
activeColumns.pop_back(); |
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.
..since this loop further discards weak columns.
I don't think that this is really a problem... Global Inhibition should only return fewer than num-desired active mini-columns if there are fewer than num-desired mini-columns which meet the activation threshold. |
well, then 2 is not a problem (just update the doc)
this still is. there can be "same (overlap)" columns > threshold, which we discard in the comparison fn. |
Actually disagree. Well, agree with the explanation, but that is a problem now:
|
Inhibition reduces the number of active columns.
Global inhibition should return constant sparsity (translated to only numDesired columns survives the inhibition).
This is almost never the case, even if we could, g inh returns fewer columns.
This PR is just WIP now that debugs the problem, I'm looking for best way to fix it.
Cropping to nth_element (to numDesired) might be premature optimization, as after that we remove sub threshold, zero, etc columns.
Some items to look into in this PR:
computeActivity
returns only sparse array (removes zero overlaps) -- this might be nice optimization, as everywhere instead of iterating through 0..numColumns, we'd go << SDR.sizeif overlaps[a]==overlaps[b]: return a< b
removes duplicit entries. That is wrong, as 2 (or more) same overlaps are valid, and might be in the top n columns to keep.