forked from numenta/nupic.core-legacy
-
Notifications
You must be signed in to change notification settings - Fork 74
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
TM activeCells_ is a SDR #442
Open
breznak
wants to merge
7
commits into
master
Choose a base branch
from
tm_active_sdr
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
d3502c6
Revert "Revert "TM: activeCells_ is now a SDR""
breznak cd48099
Revert "Revert "TMRegion: fix testLinking fail""
breznak 2911cd2
TM:getActiveCells() python return SDR
breznak e720007
fix Debug code
breznak 34be516
Merge branch 'master_community' into tm_active_sdr
breznak b318d8a
Revert "fix Debug code"
breznak 5230991
TM.activeCells as SDR: Fix for external predictive inputs.
ctrl-z-9000-times File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
continuing the discussion from #437 (comment)
This code breaks because we're adding extraActive indices offsetted by numOfCells() which does not pass a check in the SDR used.
@ctrl-z-9000-times do you have a proposal how to proceed?
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.
Yes, make two new vectors named something like presynActive & presynWinner. Copy the TM's active and winner cells into it, and concatenate the extra inputs into those vectors too.
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 culpit!
ok, let's do it properly. and not add them to activeCells_.
Proposal: Should we use a member SDR extraActivations_ ?
but they do refer to cells in this TM, right?
Sorry, again, what is the meaing of externalActive/WinnerCells? I thought it's an "deus ex" activation of the TM's cells.
So, if I have a TM with only 3 cells {0,1,2}. we now encode extras with offset, so {0,1,2, ex3, ex4, ex5}.
Assume this state (dense repr): {101 |extra: 111}
Does this translate to:
Another question, how should we treat the extra activations in context of anomalies?
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.
No, external here means that it comes from outside of yhe TM.
For example i could have 2 brain areas with 2 TMs which are looking different sensors, and one TM can help the othermake predictions.
Another Example is to model L4 with a TM and to generate a location signal and feed it to the TM. Numenta did this in their "columns" paper.
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.
They should ve included if they are given.
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.
Another view is, that it's valid and working to have activeCells_,winnerCells_ as vector due to the extra* functionality, and we cannot do that with SDR that strictly checks the members are within its bounds. Also, having active/winners as SDR does not give some big advantage. So we could wery well just close this PR.
thoughts?
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.
I can work on this a bit.
I will change the name from extra to something more descriptive.
The TM has multiple outputs, all of which are valid. IMO its better to not have a default at all and to force the user to specify what they want
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.
👍 thank you!
you're talking about TM, I agree. I was talking about TMRegion where NetworkAPI forces us to take a default output. Is it that winner cells would be generally more useful/"the nicest"?
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.
It depends on what you're doing.
Winner cells are the cells which the TM has designated to represent the current state of its world, so they should be used for learning.
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.
Sounds that should be the default output of TMRegion, I'll update that.
Should anomaly also be made from Winners/Predictive?