TM Connections integration #537

breznak · 2019-07-03T09:34:12Z

TM:

use adaptSegment from Connections
remove lastUsedIterationForSegment (for LRU), add SegmentData.lastUsed
Connections cleanup

For #373

use minPermanence as "zero", also avoid hard-coded crop for permanence, is done in connections.

merge the common code in if-else branches

was 0.0f, now minPermanence

make the code same as in connections

call connections.adaptSegment instead of reimplementing the logic

this is functionality used in TM

this matches signiture in Connections::adaptSegment and avoids need for conversions

segments with no synapses (can be removed when pruneZeroSynapses is set ON) are pruned too by calling `destroySegment`.

can set limit on max segments on cell, if reached, least recently used segments will be pruned to make space. Used by TM.

this field is used for createSegment which optionally (maxSegmentsPerCell > 0) removes excessive segments (least used first). Only used by TM.

breznak

Please review, help me discuss TODO items. Thank you 🙏

src/htm/algorithms/Connections.cpp

breznak · 2019-07-03T09:44:43Z

src/htm/algorithms/Connections.cpp

@@ -461,28 +459,32 @@ void Connections::adaptSegment(const Segment segment,
        update = -decrement;
      }

+    //prune permanences that reached zero
+    if (pruneZeroSynapses && synapseData.permanence + update < htm::minPermanence + htm::Epsilon) {
+      destroySynapse(synapse);


prune "dead" synapses that reached 0.

from TM

is synapse really dead when reached 0? -> yes, because cannot get any activation.

TODO this should be ON even for SP(?!)

how does this relate to New method Connections::synapseCompetiton #466 synapse competition? Can I use that instead here?

TODO if we remove synapses, ensure we do NOT go under stimulusThreshold See Smarter SP parameters #536 . If we do:

grow new synapse(s)?

destroy segment, and create new?

@ctrl-z-9000-times how is #466 with biological plausibility? I like how the competition helps keep synapses better balanced, but I don't see a natural mechanism. For this reason I started question raisePermanencesToThreshold, see if/how it's plausible? Or replace these "raise to threshold" with synaptic death & growth of new. ?

It seems plausible to me, that dendrites could have a desired number of synapses and that they could enforce this preference.

src/htm/algorithms/Connections.cpp

breznak · 2019-07-03T09:52:28Z

src/htm/algorithms/Connections.hpp

-  SynapseIdx numConnected;
+  CellIdx cell; //mother cell that this segment originates from
+  SynapseIdx numConnected; //number of permanences from `synapses` that are >= synPermConnected, ie connected synapses
+  UInt32 lastUsed = 0; //last used time (iteration). Used for segment pruning by "least recently used" (LRU) in `createSegment`


lastUsed moved here from TM lastUsedIterationForSegment:

reduced size from 64bit to 32b (4*10^9 iterations of the alg unlikely to exceed)

for TM same mem footprint, see Memory footprint #534

for SP currently unused, so slight increase (but only 4MB RAM per 1M columns -> neglible)

plans for SP to do pruning as well, the it'd be used

src/htm/algorithms/TemporalMemory.cpp

breznak · 2019-07-03T09:58:09Z

src/htm/algorithms/TemporalMemory.cpp

-    for (const auto &segment : activeSegments_) {
-      lastUsedIterationForSegment_[segment] = iteration_;
+    for (auto &segment : activeSegments_) {
+      connections.dataForSegment(segment).lastUsed = iteration_; //TODO the destroySegments based on LRU is expensive. Better random? or "energy" based on sum permanences? 


I don't know how, but it feels the removal of LRU is expensive. The pruning happens only rarely, while lastUsed needs to be written every activation. Is there a better way? (random, ...)

@ctrl-z-9000-times what about cummulative permanence for segment, as "energy", rather than LRU?

That's not a bad idea, but I'd want to see some kind of evidence that it works, and that it works at least as well as the current solution.

Another idea is to never destroy segments. Instead use the method "removeMinPermSynapses" to free up room on an existing segment.

and their reuse in destroyed buffers Maybe the buffers are not useful?

breznak

Added stats for pruned synapses, segments.
And some pretty info in hotgym (same as in MNIST example)

breznak · 2019-07-03T11:11:09Z

src/htm/algorithms/Connections.cpp

+  stream << "    Synapses pruned (" << (Real) self.prunedSyns_ / self.numSynapses() 
+	 << "%) Segments pruned (" << (Real) self.prunedSegs_ / self.numSegments() << "%)" << std::endl;
+  stream << "    Buffer for destroyed synapses: " << self.destroyedSynapses_.size() << " \t buffer for destr. segments: "
+	 << self.destroyedSegments_.size() << std::endl; 


can you help me make "not completely artificial" test where synapses & segments are heavily pruned?
I'd like to:

validate the code is actually (enough) used

verify the destroyed* buffers don't grow too much, cap them if needed.

This is what I get on Hotgym -> not really used at all.

TM Connections: Inputs (4653) ~> Outputs (16384) via Segments (32626) Segments on Cell Min/Mean/Max 0 / 1.99133 / 40 Potential Synapses on Segment Min/Mean/Max 10 / 10.5655 / 49 Connected Synapses on Segment Min/Mean/Max 0 / 0.356985 / 34 Synapses Dead (0%) Saturated (0.0110586%) Synapses pruned (0.0708685%) Segments pruned (0%) Buffer for destroyed synapses: 0 buffer for destr. segments: 0

You should be able to make it prune synapses by changing the data set. For example train it on sine waves and then move to saw tooth waves.

I think it's normal and good that it does not prune very many synapses / segments.

Segments should really only be pruned when the HTM reaches capacity.

You should be able to make it prune synapses by changing the data set. For example train it on sine waves and then move to saw tooth waves.

would be nice to have some test on "switching contexts", where we observe some more synapse pruning shortly after the change.

src/htm/algorithms/Connections.cpp

check even if maxSegmentPerCell == 1, otherwise we could create > 1 segments on such cell.

and hopefully more readable. Deterministic by adding rule for case xx[a] == xx[b]: then return a < b

that is if num synapses left < connectedThreshold_

src/htm/algorithms/TemporalMemory.cpp

breznak

Thank you @ctrl-z-9000-times for the explanation, I've added that to the block of code. Hope now this can be ready

breznak · 2019-07-10T22:14:11Z

src/htm/algorithms/TemporalMemory.cpp

@@ -351,16 +291,16 @@ burstColumn(vector<CellIdx> &activeCells,
  const CellIdx winnerCell =
      (bestMatchingSegment != columnMatchingSegmentsEnd)
          ? connections.cellForSegment(*bestMatchingSegment)
-          : getLeastUsedCell(rng, column, connections, cellsPerColumn);
+          : getLeastUsedCell(rng, column, connections, cellsPerColumn); //TODO replace (with random?) this is extremely costly, removing makes TM 6x faster!


getLeastUsedCells is very important on Hotgym, but I guess it just means that models is still not tuned properly. As TM should go the cellForSegment path more ofthen if learned.

breznak

This revision adds mostly cosmetic code cleanups & lots for improved doc,comments.
@ctrl-z-9000-times please one more review when you have time

breznak · 2019-07-12T07:05:15Z

src/htm/algorithms/Connections.hpp

+   *  SP & TM. 
+   */
+//!  const UInt32& iteration = iteration_; //FIXME cannot construct iteration like this?
+  UInt32 iteration() const { return iteration_; }


know how could I write this to use the const ref?

@ctrl-z-9000-times do you know why the commented above does not work?

Not exactly, but C++ is really picky about the const & trick. It stops the compiler from auto-generating certain boilerplate methods. I think I've encountered stuff like this before, but I don't remember how I fixed it.

breznak · 2019-07-12T07:06:01Z

src/htm/algorithms/Connections.hpp

@@ -539,7 +596,7 @@ class Connections : public Serializable
   */
  size_t numCells() const { return cells_.size(); }

-  Permanence getConnectedThreshold() const { return connectedThreshold_; }
+  constexpr Permanence getConnectedThreshold() const { return connectedThreshold_; }


nit, let's try to use constexpr more where possible

breznak · 2019-07-12T07:06:36Z

src/htm/algorithms/TemporalMemory.cpp

+  // because otherwise a strong input would be sampled many times and grow many synapses.
+  // That would give such input a stronger connection. 
+  // Synapses are supposed to have binary effects (0 or 1) but duplicate synapses give 
+  // them (synapses 0/1) varying levels of strength.
  for (const Synapse& synapse : connections.synapsesForSegment(segment)) {


improved doc

…into tm_conn

breznak · 2019-07-14T00:44:30Z

src/htm/algorithms/Connections.hpp


  // Extra bookkeeping for faster computing of segment activity.
  std::unordered_map<CellIdx, std::vector<Synapse>> potentialSynapsesForPresynapticCell_;
  std::unordered_map<CellIdx, std::vector<Synapse>> connectedSynapsesForPresynapticCell_;
  std::map<CellIdx, std::vector<Segment>> potentialSegmentsForPresynapticCell_;
  std::map<CellIdx, std::vector<Segment>> connectedSegmentsForPresynapticCell_;

-  std::vector<Segment> segmentOrdinals_;
-  std::vector<Synapse> synapseOrdinals_;


removed the ordinals vectors, this is now stored in Sement/SynapseData with no significant overhead, so just the code is clearer.

ctrl-z-9000-times

This looks ok to me, assuming you can fix what ever is making the unit tests fail.

Off Topic: I think that the HTM would work just as well if we never allowed for destroying segments (untested). Instead of destroying segments just remove all of the synapses. The use-case for destroying segments is when the TM is full and you want to add a new thing to it. In this situation, the destroyed segment will immediately recreated with different synapses. This would allow us to get rid of the segmentOrdinals and segment.id

use find to check the segment exists on the cell. Issue was in comparing const iteratior vs segments.end()

breznak

@ctrl-z-9000-times this should fix the test cought by Debug CI.

breznak · 2019-07-18T17:46:54Z

src/htm/algorithms/Connections.cpp

-                       });
-
-  NTA_ASSERT(segmentOnCell != cellData.segments.end());
+  const auto segmentOnCell = std::find(cellData.segments.cbegin(), cellData.segments.cend(), segment);


the error was dumb, comparing const iterator above with .segments.end() here.
Replaced with a simpler find that does not require extra sorting.

ctrl-z-9000-times

find is simpler and slower, but this method should not be called very often and I hope it eventually goes away altogether, so I'll approve it.

breznak · 2019-07-18T21:45:14Z

Thank you for reviewing, David!
Yes, I hope #574 will follow

breznak added 15 commits July 1, 2019 22:17

destroySynapse: const arg

103e8fc

TM: fix when custom minPermanence

b6a103a

use minPermanence as "zero", also avoid hard-coded crop for permanence, is done in connections.

COnnections: adaptSegment: deduplicate code for timeseries

cc476a9

merge the common code in if-else branches

Connections: adaptSegment fix hardcoded minPermanence

21c315c

was 0.0f, now minPermanence

TM:adaptSegment make similar to connections'

cdcf5df

make the code same as in connections

TM:adaptSegment uses connections', WIP

dd45567

call connections.adaptSegment instead of reimplementing the logic

Connections:adaptSegment allow to prune zero synapses

34401b4

this is functionality used in TM

TM: adaptSegment use SDR for prevActiveCells to match Connections

8607710

this matches signiture in Connections::adaptSegment and avoids need for conversions

Connections:adaptSegment remove empty segments

f6f029e

segments with no synapses (can be removed when pruneZeroSynapses is set ON) are pruned too by calling `destroySegment`.

SegmentData has simple default constructor

14871c9

Connections:createSegment can check for maxSegmentsPerCell

a25040c

can set limit on max segments on cell, if reached, least recently used segments will be pruned to make space. Used by TM.

TM: move lastUsedIterationForSegment to SegmentData.lastUsed

9648ae0

this field is used for createSegment which optionally (maxSegmentsPerCell > 0) removes excessive segments (least used first). Only used by TM.

TM uses Connections' createSegment

e20c267

Connections: add const + fix tests

24b2c36

comment

abb8f96

breznak added ready TM code code enhancement, optimization, cleanup..programmer stuff labels Jul 3, 2019

breznak requested review from dkeeney and ctrl-z-9000-times July 3, 2019 09:34

breznak self-assigned this Jul 3, 2019

breznak commented Jul 3, 2019

View reviewed changes

breznak added 3 commits July 3, 2019 12:52

Hotgym: print connections stats for SP, TM

cbf0d38

Connections: print stats on usage of pruned Segs, Syns

dac487a

and their reuse in destroyed buffers Maybe the buffers are not useful?

Hotgym: add Metrics for SDRs from SP, TM, Input

47eb418

breznak commented Jul 3, 2019

View reviewed changes

breznak commented Jul 5, 2019

View reviewed changes

src/htm/algorithms/Connections.cpp Outdated Show resolved Hide resolved

breznak added 3 commits July 5, 2019 08:15

Connection:createSegment fix check for maxSegmentsPerCell

cccc3e5

check even if maxSegmentPerCell == 1, otherwise we could create > 1 segments on such cell.

COnnections:createSegment make sort deterministic

8a1e871

Connections: make sort statements deterministic

e8fb520

and hopefully more readable. Deterministic by adding rule for case xx[a] == xx[b]: then return a < b

breznak added 2 commits July 10, 2019 19:38

COnnections: prune segment if it never can connect again

b5498f3

that is if num synapses left < connectedThreshold_

TM code comments

6e07f06

breznak requested a review from ctrl-z-9000-times July 10, 2019 22:07

TM:getLeastUsedCells improve doc

184ad2c

ctrl-z-9000-times reviewed Jul 11, 2019

View reviewed changes

src/htm/algorithms/TemporalMemory.cpp Outdated Show resolved Hide resolved

TM: add explanation for synapses only on new, unsynapsed segments

36ed0ca

breznak commented Jul 12, 2019

View reviewed changes

Merge branch 'master_community' into tm_conn

be38786

breznak commented Jul 12, 2019

View reviewed changes

breznak requested a review from ctrl-z-9000-times July 12, 2019 10:53

breznak mentioned this pull request Jul 12, 2019

SP compute() const, thread safe #560

Draft

4 tasks

breznak and others added 4 commits July 13, 2019 09:32

Merge branch 'master' into tm_conn

d20c9e8

Merge branch 'master_community' into tm_conn

2f16fb3

Connections: some more const

5fe3697

Merge branch 'tm_conn' of https://github.com/htm-community/nupic.core …

425f12c

…into tm_conn

breznak commented Jul 14, 2019

View reviewed changes

breznak added 3 commits July 14, 2019 10:25

update determ output results

d3186ed

Conn: synapseOrdinals_ moved to SynapseData

fc38d89

Conn: move segment ordinals (id) to SegmentData

5b368d8

breznak force-pushed the tm_conn branch from a41266e to 5b368d8 Compare July 14, 2019 08:39

ctrl-z-9000-times previously approved these changes Jul 17, 2019

View reviewed changes

breznak mentioned this pull request Jul 18, 2019

Consider removal of destroySegment #574

Open

breznak added 2 commits July 18, 2019 15:17

Merge branch 'master_community' into tm_conn

065b467

fixup Connections:destroySegment use find

24983cf

use find to check the segment exists on the cell. Issue was in comparing const iteratior vs segments.end()

breznak dismissed ctrl-z-9000-times’s stale review via 24983cf July 18, 2019 17:36

breznak commented Jul 18, 2019

View reviewed changes

ctrl-z-9000-times approved these changes Jul 18, 2019

View reviewed changes

breznak merged commit 6475fba into master Jul 18, 2019

breznak deleted the tm_conn branch July 18, 2019 21:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TM Connections integration #537

TM Connections integration #537

breznak commented Jul 3, 2019 •

edited

Loading

breznak left a comment

breznak Jul 3, 2019

breznak Jul 3, 2019

ctrl-z-9000-times Jul 5, 2019

breznak Jul 3, 2019

breznak Jul 3, 2019

breznak Jul 10, 2019

ctrl-z-9000-times Jul 12, 2019

breznak left a comment

breznak Jul 3, 2019

ctrl-z-9000-times Jul 10, 2019

breznak Jul 10, 2019

breznak Jul 10, 2019

breznak left a comment

breznak Jul 10, 2019

breznak left a comment

breznak Jul 12, 2019

breznak Jul 18, 2019

ctrl-z-9000-times Jul 18, 2019

breznak Jul 12, 2019

breznak Jul 12, 2019

breznak Jul 14, 2019

ctrl-z-9000-times left a comment •

edited

Loading

breznak left a comment

breznak Jul 18, 2019

ctrl-z-9000-times left a comment

breznak commented Jul 18, 2019

TM Connections integration #537

TM Connections integration #537

Conversation

breznak commented Jul 3, 2019 • edited Loading

breznak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

breznak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

breznak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

breznak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ctrl-z-9000-times left a comment • edited Loading

Choose a reason for hiding this comment

breznak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ctrl-z-9000-times left a comment

Choose a reason for hiding this comment

breznak commented Jul 18, 2019

breznak commented Jul 3, 2019 •

edited

Loading

ctrl-z-9000-times left a comment •

edited

Loading