Connections remove destroying LRU segments

bindings/py/cpp_src/bindings/algorithms/py_Connections.cpp

@@ -138,8 +138,6 @@ R"(Returns pair of:
 
     py_Connections.def("bumpSegment", &Connections::bumpSegment);
 
-    py_Connections.def("destroyMinPermanenceSynapses", &Connections::destroyMinPermanenceSynapses);
-
     py_Connections.def("numCells", &Connections::numCells);
 
     py_Connections.def("numSegments",

src/htm/algorithms/Connections.cpp

@@ -73,25 +73,14 @@ Segment Connections::createSegment(const CellIdx cell,
 
   //limit number of segmets per cell. If exceeded, remove the least recently used ones.
   NTA_ASSERT(maxSegmentsPerCell > 0);
-  while (numSegments(cell) >= maxSegmentsPerCell) {
-    const auto& destroyCandidates = segmentsForCell(cell);
-    const auto compareSegmentsByLRU = [&](const Segment a, const Segment b) {
-	if(dataForSegment(a).lastUsed == dataForSegment(b).lastUsed) {
-	  return a < b; //needed for deterministic sort
-        } 
-	else return dataForSegment(a).lastUsed < dataForSegment(b).lastUsed; //sort segments by access time
-      };
-    const auto leastRecentlyUsedSegment = std::min_element(destroyCandidates.cbegin(), 
-        destroyCandidates.cend(), compareSegmentsByLRU);
-
-    destroySegment(*leastRecentlyUsedSegment);
-  }
+  NTA_CHECK(numSegments(cell) <= maxSegmentsPerCell) << "Segment exceeded max allowed limit of segments on cell " << maxSegmentsPerCell;
 
   //proceed to create a new segment
   NTA_CHECK(segments_.size() < std::numeric_limits<Segment>::max()) << "Add segment failed: Range of Segment (data-type) insufficinet size."
 	    << (size_t)segments_.size() << " < " << (size_t)std::numeric_limits<Segment>::max();
+
   const Segment segment = static_cast<Segment>(segments_.size());
-  const SegmentData& segmentData = SegmentData(cell, iteration_, nextSegmentOrdinal_++);
+  const SegmentData& segmentData = SegmentData(cell, iteration_);
   segments_.push_back(segmentData);
 
   CellData &cellData = cells_[cell];
@@ -436,7 +425,7 @@ void Connections::adaptSegment(const Segment segment,
   }
 
   const auto& synapses = synapsesForSegment(segment);
-  for( size_t i = 0; i <  synapses.size(); i++) {
+  for( size_t i = 0; i <  synapses.size(); i++) { //TODO use presynaptic cells vector here
       const auto synapse = synapses[i];
       const SynapseData &synapseData = dataForSynapse(synapse);
 

src/htm/algorithms/Connections.hpp

@@ -95,12 +95,11 @@ struct SynapseData: public Serializable {
  * The cell that this segment is on.
  */
 struct SegmentData {
-  SegmentData(const CellIdx cell, Segment id, UInt32 lastUsed = 0) : cell(cell), numConnected(0), lastUsed(lastUsed), id(id) {} //default constructor
+  SegmentData(const CellIdx cell, Segment id) : cell(cell), numConnected(0), id(id) {} //default constructor
 
   std::vector<Synapse> synapses;
   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`
   Segment id; 
 };
 
@@ -234,8 +233,7 @@ class Connections : public Serializable
    * @param cell Cell to create segment on.
    *
    * @param maxSegmetsPerCell Optional. Enforce limit on maximum number of segments that can be
-   * created on a Cell. If the limit is exceeded, call `destroySegment` to remove least used segments 
-   * (ordered by LRU `SegmentData.lastUsed`). Default value is numeric_limits::max() of the data-type, 
+   * created on a Cell. Default value is numeric_limits::max() of the data-type, 
    * so effectively disabled. 
    *
    * @retval Unique ID of the created segment `seg`. Use `dataForSegment(seg)` to obtain the segment's data. 
@@ -418,6 +416,8 @@ class Connections : public Serializable
    */
   std::vector<Synapse> synapsesForPresynapticCell(const CellIdx presynapticCell) const;
 
+  void destroyMinPermanenceSynapses(const Segment segment, Int nDestroy, const std::vector<CellIdx> &excludeCells); //TODO removed in other PR
+
   /**
    * For use with time-series datasets.
    */
@@ -525,17 +525,6 @@ class Connections : public Serializable
    */
   void bumpSegment(const Segment segment, const Permanence delta);
 
-  /**
-   * Destroy the synapses with the lowest permanence values.  This method is
-   * useful for making room for more synapses on a segment which is already
-   * full.
-   *
-   * @param segment - Index of segment in Connections, to be modified.
-   * @param nDestroy - Must be greater than or equal to zero!
-   * @param excludeCells - Presynaptic cells which will NOT have any synapses destroyed.
-   */
-  void destroyMinPermanenceSynapses(const Segment segment, Int nDestroy,
-                                    const SDR_sparse_t &excludeCells = {});
 
   /**
    * Print diagnostic info

src/htm/algorithms/TemporalMemory.cpp

@@ -442,12 +442,6 @@ void TemporalMemory::activateDendrites(const bool learn,
   }
   const auto compareSegments = [&](const Segment a, const Segment b) { return connections.compareSegments(a, b); };
   std::sort( activeSegments_.begin(), activeSegments_.end(), compareSegments); //SDR requires sorted when constructed from activeSegments_
-  // Update segment bookkeeping.
-  if (learn) {
-    for (const auto segment : activeSegments_) {
-      connections.dataForSegment(segment).lastUsed = connections.iteration(); //TODO the destroySegments based on LRU is expensive. Better random? or "energy" based on sum permanences?
-    }
-  }
 
   // Matching segments, potential synapses.
   matchingSegments_.clear();

src/test/unit/algorithms/TemporalMemoryTest.cpp

@@ -794,8 +794,8 @@ TEST(TemporalMemoryTest, RecycleWeakestSynapseToMakeRoomForNewSynapse) {
       /*connectedPermanence*/ 0.50f,
       /*minThreshold*/ 1,
       /*maxNewSynapseCount*/ 3,
-      /*permanenceIncrement*/ 0.02f,
-      /*permanenceDecrement*/ 0.02f,
+      /*permanenceIncrement*/ 0.06f,
+      /*permanenceDecrement*/ 0.05f,
       /*predictedSegmentDecrement*/ 0.0f,
       /*seed*/ 42,
       /*maxSegmentsPerCell*/ 255,
@@ -810,33 +810,36 @@ TEST(TemporalMemoryTest, RecycleWeakestSynapseToMakeRoomForNewSynapse) {
 
   Segment matchingSegment = tm.createSegment(4);
 
-  // Create a weak synapse. Make sure it's not so weak that
-  // permanenceDecrement destroys it.
+  // A: Create a weak synapse. This synapse should be destroyed in the experiment. 
+  // Make sure it's not so weak that permanenceDecrement destroys it on a single 
+  // step. (0.11 < 3x0.05 => in 3 steps it should be gone)
   tm.connections.createSynapse(matchingSegment, 0, 0.11f);
 
-  // Create a synapse that will match.
+  // B: Create a synapse that will match. ('1' is in the active columns, 0 above is not)
   tm.connections.createSynapse(matchingSegment, 1, 0.20f);
 
-  // Create a synapse with a high permanence.
+  // C: Create a synapse with a high permanence. (31 is also not in the active cols, 
+  // but here permanence is so high that it would not be removed)
   tm.connections.createSynapse(matchingSegment, 31, 0.6f);
 
-  // Activate a synapse on the segment, making it "matching".
+  for(int i=0; i< 3; i++) {
+  // Activate a synapse on the segment, making it "matching". (B matches, as it's presyn cell '1'
+  // is in active cols).
   tm.compute(previousActiveColumns);
-
   ASSERT_EQ(prevWinnerCells, tm.getWinnerCells());
-
   // Now mark the segment as "correct" by activating its cell.
   tm.compute(activeColumns);
+  }
 
-  // There should now be 3 synapses, and none of them should be to cell 0.
+  // There should now be 3 synapses, and none of them should be to cell '0' (A).
   const vector<Synapse> &synapses =
       tm.connections.synapsesForSegment(matchingSegment);
-  ASSERT_EQ(4ul, synapses.size());
 
   std::set<CellIdx> presynapticCells;
   for (Synapse synapse : synapses) {
-    presynapticCells.insert(
-        tm.connections.dataForSynapse(synapse).presynapticCell);
+    const auto presyn = tm.connections.dataForSynapse(synapse).presynapticCell;
+    EXPECT_TRUE(presyn != 0) << "Permanence to cell '0' in case A should have been deleted.";
+    presynapticCells.insert(presyn);
   }
 
   std::set<CellIdx> expected = {1, 2, 3, 31};