-
Notifications
You must be signed in to change notification settings - Fork 3
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
HTM core detector pandaVis - Bugfix distal synapses #36
Conversation
Second addition, while this PR is already opened, I discovered out big problems with order calls, that i have in all scripts (2D object recognition, hotgym example and this) It spend literally hours to get the execution order right in my head and i must say that it was very hard. 😫 But finally this is the right. I changed here the order of calls.
|
@@ -147,6 +148,7 @@ def __init__(self, *args, **kwargs): | |||
if PANDA_VIS_ENABLED: | |||
pandaServer.Start() | |||
self.BuildPandaSystem(parameters_numenta_comparable) | |||
self.firstStep = 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.
nit, could be, but you could use something like TM (or connections) .getIterationNum()
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 couldn't find iteration getter for TM, but for SP there is. Should i use that?
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'd say best would be connections.iterations()
..let me see if it's exposed to python- yes.
# activateDendrites calculates active segments | ||
self.tm.activateDendrites(learn=True) | ||
# predictive cells are calculated directly from active segments | ||
predictiveCells = self.tm.getPredictiveCells() |
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.
nit: does this not work for all cases (I mean including the 1st)?. activate dendrites & then cells
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.
Yeah, it works, but things get complicated, when you consider inputs. And time consistency.
--script start
Inputs are in time T.
Active columns are in time T.
Winners (active cells) are in time T.
Prediction cells must be in T+1.
--pandaBlockExecution
--endscript
And for vizualization pruposes in need this order if i want to prevent buffering previous values etc..
Also while using external distal inputs for TM this order is OK.
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.
And for vizualization pruposes in need this order if i want to prevent buffering previous values etc..
Also while using external distal inputs for TM this order is OK.
see above - predictive inputs shouldn't be a problem ✔️ , but I guess the order would be dendrites, then activate cells
(and then you get predictive at T ~ "active T+1"), so you need to do the buffering (can the visualizer do that internally?, as this is true for all HTMs.
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.
If i go with the path of buffering, there are several issues.
Vis has besides "do one step" also "GOTO iteration xxx" and if i will buffer, i need to stop one step before goal iteration and exectute that, send data and then move on to the destination iteration.
There is also the feature for requesting the proximal/distal synapses on user-mouse-click-demand for specific cell/column. If i am not wrong, activateCells grows/penalize synapses, so the state of synapses would be T+1 and again i would need to buffer that... but i can't know what user will want :/
Because these are bigdata - visualizer only requests data like synapses for specific cell/column that user mouse clicks.
I can't send all data, that would be computationally intensive...
Maybe in the future i would like to improve this to log historical data and keep track of some focused columns/cells, that would be awesome.. but for now i need to stay on the ground
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.
Vis has besides "do one step" also "GOTO iteration xxx" and if i will buffer, i need to stop one step before goal iteration and exectute tha
the implementation gets complicated, esp to support the "jumps". But..are the data/is the approach correct? Could you least empirically verify (with fixed seed) that the result state from compute()
vs "suggested approach here" is identical?
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.
If i am not wrong, activateCells grows/penalize synapses, so the state of synapses would be T+1 and again i would need to buffer that... but i can't know what user will want :/
you're right with the penalization. activateDendrites-activateCells
action-pair gives you "active cells for T", and "predictive cells for T" (which approximates state at T+1)
Maybe in the future i would like to improve this to log historical data and keep track of some focused columns/cells, that would be awesome.. but for now i need to stay on the ground
that would be cool! some video,backtrack feature for select cells/patterns.
@@ -332,6 +334,12 @@ def modelRun(self, ts, val): | |||
pass | |||
|
|||
if PANDA_VIS_ENABLED: | |||
# activateDendrites calculates active segments |
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 if-else starts to be complicated.
- what used to be
tm.compute()
https://github.com/htm-community/NAB/pull/36/files#diff-d9640827e1a8cfdf99cc729ed05d01ecR291 - became (understendable) split to
activate dendrites & activate cells
: https://github.com/htm-community/NAB/pull/36/files#diff-d9640827e1a8cfdf99cc729ed05d01ecL281-L286 - now the split to the 2nd (3rd) if-else branch is confusing.
Note, now reviewing it, without more deeply thinking about it and verifying, I'd stay on the safe side and say that the order should be as in TemporalMemory.cpp compute()
- so 1st dendrites, 2nd cells. That will give you valid results for both.
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 agree that it is confusing, certainly much more that it was.
But the problem is, if we leave here this
became (understendable) split to activate dendrites & activate cells : https://github.com/htm-community/NAB/pull/36/files#diff-d9640827e1a8cfdf99cc729ed05d01ecL281-L286
Then call for example self.tm_info.addData( self.tm.getActiveCells().flatten() )
then some pointer leakages can occur - it happened to me, i was hunting that bug and i figured out, that htm.core is not internally made to be able to do this. Note: i was using external distal inputs, i saw that the externalDistalInputs are literally added to the vector of ActiveCells and that is messing things up (actually it is then SDR of size let's say 65535 and the vector contains indexes like 65536 and more) so we can't read getActiveCells between activaeDendrites and activateCells
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.
If you think that would be better to create htmcore_detector_pandaVis.py instead to incorporating it to this script that is used as benchmark/optimizer i would not be against. I didn't know at the beggining that it will gets more complicated like this
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.
Then call for example self.tm_info.addData( self.tm.getActiveCells().flatten() )
then some pointer leakages can occur - it happened to me, i was hunting that bug and i figured out, that htm.core is not internally made to be able to do this.
can you make a reproducer for that? Sounds like something that should ideally be ironed out in the lib itself. Unless you're doing something that's marked as undefined/incorrect behavior (see c++'s doc for the relevant methods in TM for this please)
Note: i was using external distal inputs, i saw that the externalDistalInputs are literally added to the vector of ActiveCells and that is messing things up (actually it is then SDR of size let's say 65535 and the vector contains indexes like 65536 and more)
hmm, yes, this is sort of hacked-in code. But as I recall the code is correct, "external inputs" are indeed external, the cells' indices are out-of those that the TM/connections use. And if you use external inputs, you should expect that. I haven't worked with that code lately, so I'd certainly welcome feedback and discussion. We might try to find better ways to handle that.
so we can't read getActiveCells between activaeDendrites and activateCells
can't we?
From the activateDendrites
doc
- Calculate dendrite segment activity, using the current active cells. Call
- this method before calling getPredictiveCells, getActiveSegments, or
- getMatchingSegments. In each time step, only the first call to this
- method has an effect, subsequent calls assume that the prior results are
- still valid. [...]
All in all, I hope we can improve the API,doc, usage of the external dendrites with this.
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.
If you think that would be better to create htmcore_detector_pandaVis.py instead to incorporating it to this script that is used as benchmark/optimizer i would not be against.
let's see how it goes. I certainly don't, as I asked you if you could merge the two. On the other hand, things are getting to be more complicated than when we started. But also, once done, the detector code wouldn't be actively used (changed) by the users, merely changing the constants and calling it.
# activateDendrites calculates active segments | ||
self.tm.activateDendrites(learn=True) | ||
# predictive cells are calculated directly from active segments | ||
predictiveCells = self.tm.getPredictiveCells() |
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.
And for vizualization pruposes in need this order if i want to prevent buffering previous values etc..
Also while using external distal inputs for TM this order is OK.
see above - predictive inputs shouldn't be a problem ✔️ , but I guess the order would be dendrites, then activate cells
(and then you get predictive at T ~ "active T+1"), so you need to do the buffering (can the visualizer do that internally?, as this is true for all HTMs.
@Zbysekz can we continue your work here? Either by merging as is, or by revamping the API? |
I am now working on the new interface, baking data to the database. (already got working partially) This then would be much simplier, i am for cancelling this PR and then i start new one with the revamp when it will be ready. |
oh, that's great 👍 , I was worried the progress got lost. Could you post an example (pseudocode) how the new API would be used in 3rd party? As in here? |
I think it will be like this:
global variable for "pandaBaker" with database file specified Filling data into dicts
And custom data for plots user configurable window like:
I will probably use exactly this universality for raw anomaly: I will try to make this as simple as possible |
Small bugfix, as result clicking on cells was causing exception