-
Notifications
You must be signed in to change notification settings - Fork 75
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Connections: apply synapsesCompetition() in adaptSegment() #584
base: master
Are you sure you want to change the base?
Conversation
instead of lower_bound, caused err in Debug. Seems faster
already used in TM makes SP 10% faster! (on MNIST 55->50s)
normally in learning.
and use it for synapsesCompetition()
This reverts commit 9a1bbf9.
src/htm/algorithms/Connections.cpp
Outdated
@@ -479,6 +475,9 @@ void Connections::adaptSegment(const Segment segment, | |||
} | |||
} | |||
|
|||
//balance synapses using competition on dendrite | |||
synapseCompetition(segment, 4, 10); //FIXME derive these numbers |
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.
calling synapseCompetition
from adaptSegment
- this works pretty well, with synapse pruning many synapses get removed which leads to a considerable speedup
- set the min/max parameters somehow smarter
- I guess those should ideally be around stimulusThreshold ?
- the tighter the boundary, the more syns get removed, the faster SP (on MNIST) is.
- but where is the line? What is the disadvantage? I'd think too tight boundary is more prone to noice?
@@ -510,7 +510,7 @@ class Connections : public Serializable | |||
*/ | |||
void synapseCompetition( const Segment segment, | |||
const SynapseIdx minimumSynapses, | |||
const SynapseIdx maximumSynapses); | |||
const SynapseIdx maximumSynapses); |
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.
TODO
- this should have some parametric default values
- replace (or provide overload) with percentages off the range [stimulusThreshold, maxSynapsesPerSegment]?
@@ -702,7 +702,7 @@ Real SpatialPooler::avgConnectedSpanForColumnND_(UInt column) const { | |||
void SpatialPooler::adaptSynapses_(const SDR &input, | |||
const SDR &active) { | |||
for(const auto &column : active.getSparse()) { | |||
connections_.adaptSegment(column, input, synPermActiveInc_, synPermInactiveDec_); | |||
connections_.adaptSegment(column, input, synPermActiveInc_, synPermInactiveDec_, true); |
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.
SP also does prune synapses (like TM)
- default to true/or even rm the parameter, when all use true now
This PR now gains importance with a problem I've discovered for NAB detector usingour HTM: htm-community/NAB#15 (comment) For this PR to be considered complete, we need to resolve "how do we choose the lower/upper bound for syn competition". Currently it is hardcoded. #584 (review) |
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.
Aiming to finish this PR soon..
@@ -519,6 +519,9 @@ void Connections::adaptSegment(const Segment segment, | |||
} | |||
} | |||
|
|||
//balance synapses using competition on dendrite | |||
synapseCompetition(segment, 8, 11); //FIXME derive these numbers |
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 is the only big remaining issue - how to choose good values for these?
Any tips @ctrl-z-9000-times ?
Connections.adaptSegment
calls newly addedsynapsesCompetition
Follow up to #466
Fixes #558