Skip to content
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

New method Connections::synapseCompetiton #466

Merged
merged 11 commits into from
Jul 12, 2019
Merged

New method Connections::synapseCompetiton #466

merged 11 commits into from
Jul 12, 2019

Conversation

ctrl-z-9000-times
Copy link
Collaborator

@ctrl-z-9000-times ctrl-z-9000-times added feature new feature, extending API research new functionality of HTM theory, research idea labels May 11, 2019
@ctrl-z-9000-times ctrl-z-9000-times self-assigned this May 11, 2019
Copy link
Member

@breznak breznak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like the new research this brings in.
And the throughout tests 👍
A few nits to the implementation.

But I don't see this actually used anywhere. Can you add a bool toggle to the SP,TM to actually make use of the new feature? Ideally show it in the MNIST exeriment?

src/nupic/algorithms/Connections.cpp Outdated Show resolved Hide resolved
src/nupic/algorithms/Connections.cpp Outdated Show resolved Hide resolved
void Connections::synapseCompetition(
const Segment segment,
const SynapseIdx minimumSynapses,
const SynapseIdx maximumSynapses)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we also have something like Connections' minConnectedSynapsesPerSegment ? If so, please also check against that

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a method Connections::raisePermanencesToThreshold. What about it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not a method, constants. Whether connections have in itself defined some limits, in the meaning of what minimumSynapses are.

@ctrl-z-9000-times
Copy link
Collaborator Author

But I don't see this actually used anywhere. Can you add a bool toggle to the SP,TM to actually make use of the new feature? Ideally show it in the MNIST exeriment?

I have integrated it into the MNIST experiment on the branch columnPooler2. The awkward part is that nothing in that branch is ready for public consumption.

@breznak
Copy link
Member

breznak commented May 13, 2019

I have integrated it into the MNIST experiment on the branch columnPooler2. The awkward part is that nothing in that branch is ready for public consumption.

yes, and as this is useful general-purpose and could conceptually be useful in SP/TM as well. Where should it be called?

  • Can we call "synaptic competition" (maybe call it synapticInhibition, as it's more descriptive and inline with the inhibition at cell level?) at Connections::updateSynapsesPermanence() (esp if said method is reworked to accept a vector (now its called in for-loop for each synapse)
  • and make SP/TM have a flag to toggle it, or I'd even say make it the defaults.

@ctrl-z-9000-times
Copy link
Collaborator Author

could be useful in TM

I don't know about the TM, I haven't tested this at all. The TM uses different methods to control the number of synapses in its potential pool; it add synapses when it wants more, and it does not add more synapses than it needs.

maybe call it synapticInhibition, as it's more descriptive and inline with the inhibition at cell level?

I don't think that inhibitory cells have anything to do with controlling the number of synapses. I also think that competition is more descriptive - it states what it does, not how it does it.

@breznak
Copy link
Member

breznak commented May 13, 2019

I don't know about the TM, I haven't tested this at all. The TM uses different methods to control the number of synapses in its potential pool;

can you add your experiment as an example? If we can visualize synapse utilization in SP on the MNIST task, we can compare with using this feature and potentialy make it default.

  • it states what it does, not how it does it.

that's right 👍

@ctrl-z-9000-times
Copy link
Collaborator Author

If we can visualize synapse utilization in SP

This is done by saving the connections to file (which the MNIST example already does), and calling
$ python3 -m nupic.examples.connections_analysis $CONNECTIONS

@breznak
Copy link
Member

breznak commented May 13, 2019

$ python3 -m nupic.examples.connections_analysis $CONNECTIONS

oh, I forgot about that script! Will try it out

Copy link
Member

@breznak breznak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A really nice feature! and great tests 👍

  • a number of, but small, nits; soon we're ready to go
  • please link the doc/motivation you've posted somewhere
  • can you add the python script with histogram if permanences?
  • and most importantly, ok if this is to be used for ColumnPooler, but I'd love to see at least experiment using it with current SP,TM. (if works well either as default, or with a toggle)

Thanks 🆒

src/htm/algorithms/Connections.cpp Outdated Show resolved Hide resolved
// Sort the potential pool by permanence values, and look for the synapse with
// the N'th greatest permanence, where N is the desired number of connected
// synapses. Then calculate how much to change the N'th synapses permance by
// such that it becomes a connected synapse. After that there will be exactly
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how much to change the N'th synapses permance by such that it becomes a connected synapse

Unrelated to acceptance of this PR, but: what is the biological mechanism for this? I am doing some work on at SP now and realized I "dont like" raisePermanencesToThreshold. The biological synapse cannot have this, and grow bit by bit (synPermActiveInc).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ctrl-z-9000-times ping on this PR, I'm looking forward to have it merge, when you have time to integrated the feedback 👼

Still thinking about this PR:

    1. please make it used in SP, TM.
    1. consider the following:
    • instead of bumpSegment that puts all misperforming synapses in line (to min/max-desired)
    • have them die out.
    • if too few synapses on segment, create a random new. Or create each time one is removed/dead.

src/htm/algorithms/Connections.cpp Show resolved Hide resolved
src/htm/algorithms/Connections.cpp Outdated Show resolved Hide resolved
src/htm/algorithms/Connections.cpp Outdated Show resolved Hide resolved
src/test/unit/algorithms/ConnectionsTest.cpp Outdated Show resolved Hide resolved
src/test/unit/algorithms/ConnectionsTest.cpp Show resolved Hide resolved
src/test/unit/algorithms/ConnectionsTest.cpp Outdated Show resolved Hide resolved
emptySegment.ncon = 0;
emptySegment.min = 3;
emptySegment.max = 100;
emptySegment.dont_check = true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we create a new synapse on an empty seg, if needed, rather than failing?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The synapse competition method does not have access to the current inputs. The call would need to specify which presynapses to use.

src/test/unit/algorithms/ConnectionsTest.cpp Outdated Show resolved Hide resolved
This was referenced Jul 3, 2019
@ctrl-z-9000-times
Copy link
Collaborator Author

Breznak,

I did everything except to actually use it in the SpatialPooler.
I'd like to merge this anyways since it's ready.

@breznak
Copy link
Member

breznak commented Jul 12, 2019

I'd like to merge this anyways since it's ready.

the new test is feiling with Debug.

except to actually use it in the SpatialPooler.

that's a pity, but fine, I'll try this later.

  • this could be used for SP,TM, right?
  • called on each syn update (would be probably expensive), or just with some probability?
  • where should this be called? I'd say randomly on updateSynapsePermanence?

breznak
breznak previously approved these changes Jul 12, 2019
@ctrl-z-9000-times
Copy link
Collaborator Author

Thanks for approving!

this could be used for SP,TM, right?

Yes, the SP could use this. The TM should not need it, I don't think this is applicable to the TM's segments.

called on each syn update (would be probably expensive), or just with some probability?

Your right: this does not need to be called too often.
Maybe only on random cycles, or if iteration % 10 == 0

where should this be called? I'd say randomly on updateSynapsePermanence?

Call after each call to adaptSegment

Copy link
Member

@breznak breznak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @ctrl-z-9000-times for this new functionality! Looking forward for seeing it used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature new feature, extending API ready research new functionality of HTM theory, research idea
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants