Connections: replace destroyMinPermSynapses() with plausible decay

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

@@ -144,8 +144,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/examples/hello/HelloSPTP.cpp

@@ -213,12 +213,12 @@ EPOCHS = 2; // make test faster in Debug
 
       SDR goldTM({COLS});
       const SDR_sparse_t deterministicTM{
-        72, 85, 102, 114, 122, 126, 287, 308, 337, 339, 542, 920, 939, 952, 1268, 1507, 1508, 1518, 1546, 1547, 1626, 1627, 1633, 1668, 1727, 1804, 1805, 1827, 1832, 1844, 1859, 1862, 1918, 1920, 1924, 1931, 1933, 1945, 1961, 1965, 1966, 1968, 1970, 1973, 1975, 1976, 1977, 1979, 1986, 1987, 1991, 1992, 1996, 1998, 2002, 2006, 2008, 2012, 2042, 2045
+        36, 85, 118, 263, 287, 303, 308, 322, 336, 337, 339, 370, 432, 1115, 1147, 1214, 1508, 1512, 1518, 1523, 1626, 1668, 1691, 1694, 1729, 1781, 1797, 1798, 1803, 1827, 1832, 1844, 1858, 1859, 1860, 1861, 1862, 1917, 1929, 1936, 1939, 1941, 1943, 1947, 1950, 1953, 1956, 1958, 1964, 1965, 1967, 1971, 1973, 1976, 1984, 1985, 1987, 1994, 1996, 1997, 1998, 1999, 2002,2006, 2012, 2027, 2040, 2042
       };
       goldTM.setSparse(deterministicTM);
 
-      const float goldAn    = 0.637255f; //Note: this value is for a (randomly picked) datapoint, it does not have to improve (decrease) with better algorithms
-      const float goldAnAvg = 0.40804f; // ...the averaged value, on the other hand, should improve/decrease. 
+      const float goldAn    = 0.745098f; //Note: this value is for a (randomly picked) datapoint, it does not have to improve (decrease) with better algorithms
+      const float goldAnAvg = 0.408207f; // ...the averaged value, on the other hand, should improve/decrease. 
 
 #ifdef _ARCH_DETERMINISTIC
       if(e+1 == 5000) {

src/htm/algorithms/Connections.cpp

@@ -654,40 +654,6 @@ void Connections::bumpSegment(const Segment segment, const Permanence delta) {
 }
 
 
-void Connections::destroyMinPermanenceSynapses(
-                              const Segment segment, 
-			      const size_t nDestroy,
-                              const vector<CellIdx> &excludeCells)
-{
-  // Don't destroy any cells that are in excludeCells.
-  vector<Synapse> destroyCandidates;
-  for( Synapse synapse : synapsesForSegment(segment)) {
-    const CellIdx presynapticCell = dataForSynapse(synapse).presynapticCell;
-
-    if( not std::binary_search(excludeCells.cbegin(), excludeCells.cend(), presynapticCell)) {
-      destroyCandidates.push_back(synapse);
-    }
-  }
-
-  const auto comparePermanences = [&](const Synapse A, const Synapse B) {
-    const Permanence A_perm = dataForSynapse(A).permanence;
-    const Permanence B_perm = dataForSynapse(B).permanence;
-    if( A_perm == B_perm ) {
-      return A < B;
-    }
-    else {
-      return A_perm < B_perm;
-    }
-  };
-  std::sort(destroyCandidates.begin(), destroyCandidates.end(), comparePermanences);
-
-  const size_t destroy = std::min( nDestroy, destroyCandidates.size() );
-  for(size_t i = 0; i < destroy; i++) {
-    destroySynapse( destroyCandidates[i] );
-  }
-}
-
-
 namespace htm {
 /**
  * print statistics in human readable form

src/htm/algorithms/Connections.hpp

@@ -616,18 +616,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, 
-		                    const size_t nDestroy,
-                                    const SDR_sparse_t &excludeCells = {});
 
   /**
    * Print diagnostic info

src/htm/algorithms/TemporalMemory.cpp

@@ -141,19 +141,16 @@ void TemporalMemory::initialize(
   reset();
 }
 
-CellIdx TemporalMemory::getLeastUsedCell_(const CellIdx column) {
+CellIdx TemporalMemory::getLeastUsedCell_(const CellIdx column) const {
   if(cellsPerColumn_ == 1) return column;
 
   vector<CellIdx> cells = cellsForColumn(column);
-
-  //TODO: decide if we need to choose randomly from the "least used" cells, or if 1st is fine. 
-  //In that case the line below is not needed, and this method can become const, deterministic results in tests need to be updated
-  //un/comment line below: 
-  rng_.shuffle(cells.begin(), cells.end()); //as min_element selects 1st minimal element, and we want to randomly choose 1 from the minimals.
+  //Note: from the found "least used cells" (if there are more), choose just 1st, not randomly  
+  // or un/comment line below: 
+  //rng_.shuffle(cells.begin(), cells.end()); //as min_element selects 1st minimal element, and we want to randomly choose 1 from the minimals.
 
   const auto compareByNumSegments = [&](const CellIdx a, const CellIdx b) {
-    if(connections.numSegments(a) == connections.numSegments(b)) 
-      return a < b; //TODO rm? 
+    if(connections.numSegments(a) == connections.numSegments(b)) return a < b; //TODO rm? 
     else return connections.numSegments(a) < connections.numSegments(b);
   };
   return *std::min_element(cells.begin(), cells.end(), compareByNumSegments);
@@ -168,24 +165,16 @@ void TemporalMemory::growSynapses_(
   vector<CellIdx> candidates(prevWinnerCells.begin(), prevWinnerCells.end());
   NTA_ASSERT(std::is_sorted(candidates.begin(), candidates.end()));
 
-  //figure the number of new synapses to grow
-  const size_t nActual = std::min(static_cast<size_t>(nDesiredNewSynapses), candidates.size());
-  // ..Check if we're going to surpass the maximum number of synapses.
-  Int overrun = static_cast<Int>(connections.numSynapses(segment) + nActual - maxSynapsesPerSegment_);
-  if (overrun > 0) {
-    connections_.destroyMinPermanenceSynapses(segment, static_cast<size_t>(overrun), prevWinnerCells);
-  }
-  // ..Recalculate in case we weren't able to destroy as many synapses as needed.
-  const size_t nActualWithMax = std::min(nActual, static_cast<size_t>(maxSynapsesPerSegment_) - connections.numSynapses(segment));
+  const size_t nActual = std::min(static_cast<size_t>(nDesiredNewSynapses) + (size_t)connections.numSynapses(segment), (size_t)maxSynapsesPerSegment_); //even with the new additions, synapses fit to segment's limit
 
   // Pick nActual cells randomly.
-  rng_.shuffle(candidates.begin(), candidates.end());
-  const size_t nDesired = connections.numSynapses(segment) + nActualWithMax; //num synapses on seg after this function (+-), see #COND
+  rng_.shuffle(candidates.begin(), candidates.end()); 
   for (const auto syn : candidates) {
     // #COND: this loop finishes two folds: a) we ran out of candidates (above), b) we grew the desired number of new synapses (below)
-    if(connections.numSynapses(segment) == nDesired) break;
+    if(connections.numSynapses(segment) == nActual) break; //this break is also used because conn.createSynapse() can "exit early" if a syn already exists, this IF handles that case too.
     connections_.createSynapse(segment, syn, initialPermanence_); //TODO createSynapse consider creating a vector of new synapses at once?
   }
+  NTA_ASSERT(connections.numSynapses(segment) == nActual);
 }
 
 

src/htm/algorithms/TemporalMemory.hpp

@@ -660,7 +660,7 @@ class TemporalMemory : public Serializable
 		     const SynapseIdx nDesiredNewSynapses,
 		     const vector<CellIdx> &prevWinnerCells);
 
-  CellIdx getLeastUsedCell_(const CellIdx column);
+  CellIdx getLeastUsedCell_(const CellIdx column) const;
 
   void calculateAnomalyScore_(const SDR &activeColumns);
 

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.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.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.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};