diff --git a/src/examples/hotgym/HelloSPTP.cpp b/src/examples/hotgym/HelloSPTP.cpp index 9b7cc90b55..59582e6f1e 100644 --- a/src/examples/hotgym/HelloSPTP.cpp +++ b/src/examples/hotgym/HelloSPTP.cpp @@ -212,12 +212,12 @@ EPOCHS = 2; // make test faster in Debug SDR goldTM({COLS}); const SDR_sparse_t deterministicTM{ - 62, 77, 85, 322, 340, 432, 952, 1120, 1488, 1502, 1512, 1518, 1547, 1627, 1633, 1668, 1727, 1729, 1797, 1803, 1805, 1812, 1858, 1859, 1896, 1918, 1923, 1925, 1929, 1931, 1939, 1941, 1942, 1944, 1950, 1953, 1955, 1956, 1965, 1966, 1967, 1968, 1974, 1980, 1987, 1996, 2006, 2008, 2011, 2027, 2030, 2042, 2046 + 36, 62, 77, 85, 87, 90, 102, 113, 118, 126, 133, 155, 277, 322, 337, 339, 340, 352, 370, 432, 493, 1089, 1095, 1114, 1184, 1214, 1230, 1488, 1499, 1502, 1507, 1508, 1518, 1547, 1626, 1691, 1711, 1760, 1781, 1797, 1803, 1804, 1805, 1812, 1827, 1828, 1832, 1841, 1858, 1859, 1860, 1862, 1918, 1925, 1929, 1944, 1950, 1953, 1956, 1958, 1967, 1968, 1971, 1973, 1975, 1976, 1977, 1980, 1985, 1986, 1994, 1998, 1999, 2002, 2013, 2027, 2036, 2042, 2045 }; goldTM.setSparse(deterministicTM); - const float goldAn = 0.627451f; - const float goldAnAvg = 0.407265f; + const float goldAn = 0.77451f; //Note: this value is for a (randomly picked) datapoint, it does not have to improve (decrease) with better algorithms + const float goldAnAvg = 0.411894f; // ...the averaged value, on the other hand, should improve/decrease. #ifdef _ARCH_DETERMINISTIC if(EPOCHS == 5000) { diff --git a/src/htm/algorithms/Connections.cpp b/src/htm/algorithms/Connections.cpp index b7ac68c43d..0843383d1f 100644 --- a/src/htm/algorithms/Connections.cpp +++ b/src/htm/algorithms/Connections.cpp @@ -40,9 +40,7 @@ Connections::Connections(const CellIdx numCells, void Connections::initialize(CellIdx numCells, Permanence connectedThreshold, bool timeseries) { cells_ = vector(numCells); segments_.clear(); - destroyedSegments_.clear(); synapses_.clear(); - destroyedSynapses_.clear(); potentialSynapsesForPresynapticCell_.clear(); connectedSynapsesForPresynapticCell_.clear(); potentialSegmentsForPresynapticCell_.clear(); @@ -90,17 +88,11 @@ Segment Connections::createSegment(const CellIdx cell, } //proceed to create a new segment - Segment segment; - if (!destroyedSegments_.empty() ) { //reuse old, destroyed segs - segment = destroyedSegments_.back(); - destroyedSegments_.pop_back(); - } else { //create a new segment - NTA_CHECK(segments_.size() < std::numeric_limits::max()) << "Add segment failed: Range of Segment (data-type) insufficinet size." + NTA_CHECK(segments_.size() < std::numeric_limits::max()) << "Add segment failed: Range of Segment (data-type) insufficinet size." << (size_t)segments_.size() << " < " << (size_t)std::numeric_limits::max(); - segment = static_cast(segments_.size()); - const SegmentData& segmentData = SegmentData(cell, iteration_, nextSegmentOrdinal_++); - segments_.push_back(segmentData); - } + const Segment segment = static_cast(segments_.size()); + const SegmentData& segmentData = SegmentData(cell, iteration_, nextSegmentOrdinal_++); + segments_.push_back(segmentData); CellData &cellData = cells_[cell]; cellData.segments.push_back(segment); //assign the new segment to its mother-cell @@ -116,17 +108,31 @@ Segment Connections::createSegment(const CellIdx cell, Synapse Connections::createSynapse(Segment segment, CellIdx presynapticCell, Permanence permanence) { + + // Skip cells that are already synapsed on by this segment + // Biological motivation (?): + // There are structural constraints on the shapes of axons & synapses + // which prevent a large number duplicate of connections. + // + // It's important to prevent cells from growing duplicate synapses onto a segment, + // 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& syn : synapsesForSegment(segment)) { + const CellIdx existingPresynapticCell = dataForSynapse(syn).presynapticCell; //TODO 1; add way to get all presynaptic cells for segment (fast) + if (presynapticCell == existingPresynapticCell) { + return syn; //synapse (connecting to this presyn cell) already exists on the segment; don't create a new one, exit early and return the existing + } + } //else: the new synapse is not duplicit, so keep creating it. + + + // Get an index into the synapses_ list, for the new synapse to reside at. - Synapse synapse; - if (!destroyedSynapses_.empty() ) { - synapse = destroyedSynapses_.back(); - destroyedSynapses_.pop_back(); - } else { - NTA_ASSERT(synapses_.size() < std::numeric_limits::max()) << "Add synapse failed: Range of Synapse (data-type) insufficient size." + NTA_ASSERT(synapses_.size() < std::numeric_limits::max()) << "Add synapse failed: Range of Synapse (data-type) insufficient size." << synapses_.size() << " < " << (size_t)std::numeric_limits::max(); - synapse = static_cast(synapses_.size()); - synapses_.emplace_back(SynapseData()); - } + const Synapse synapse = static_cast(synapses_.size()); //TODO work on cache locality. Have all Synapse, SynapseData on Segment in continuous mem block ? + synapses_.emplace_back(SynapseData()); // Fill in the new synapse's data SynapseData &synapseData = synapses_[synapse]; @@ -154,6 +160,7 @@ Synapse Connections::createSynapse(Segment segment, } bool Connections::segmentExists_(const Segment segment) const { + NTA_CHECK(segment < segments_.size()); const SegmentData &segmentData = segments_[segment]; const vector &segmentsOnCell = cells_[segmentData.cell].segments; return (std::find(segmentsOnCell.cbegin(), segmentsOnCell.cend(), segment) != @@ -211,7 +218,7 @@ void Connections::destroySegment(const Segment segment) { NTA_ASSERT(*segmentOnCell == segment); cellData.segments.erase(segmentOnCell); - destroyedSegments_.push_back(segment); + destroyedSegments_++; } @@ -261,8 +268,7 @@ void Connections::destroySynapse(const Synapse synapse) { NTA_ASSERT(*synapseOnSegment == synapse); segmentData.synapses.erase(synapseOnSegment); - - destroyedSynapses_.push_back(synapse); + destroyedSynapses_++; } @@ -329,19 +335,6 @@ SegmentIdx Connections::idxOnCellForSegment(const Segment segment) const { } -void Connections::mapSegmentsToCells(const Segment *segments_begin, - const Segment *segments_end, - CellIdx *cells_begin) const { - CellIdx *out = cells_begin; - - for (auto segment = segments_begin; segment != segments_end; - ++segment, ++out) { - NTA_ASSERT(segmentExists_(*segment)); - *out = segments_[*segment].cell; - } -} - - bool Connections::compareSegments(const Segment a, const Segment b) const { const SegmentData &aData = segments_[a]; const SegmentData &bData = segments_[b]; @@ -606,7 +599,6 @@ void Connections::destroyMinPermanenceSynapses( const vector &excludeCells) { NTA_ASSERT( nDestroy >= 0 ); - if( nDestroy <= 0 ) return; // Nothing to do. // Don't destroy any cells that are in excludeCells. vector destroyCandidates; @@ -703,8 +695,6 @@ std::ostream& operator<< (std::ostream& stream, const Connections& self) << "%) Saturated (" << (Real) synapsesSaturated / self.numSynapses() << "%)" << std::endl; 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; return stream; } diff --git a/src/htm/algorithms/Connections.hpp b/src/htm/algorithms/Connections.hpp index 745bf93286..d8bb35e8a5 100644 --- a/src/htm/algorithms/Connections.hpp +++ b/src/htm/algorithms/Connections.hpp @@ -246,13 +246,30 @@ class Connections : public Serializable const SegmentIdx maxSegmentsPerCell = std::numeric_limits::max()); /** - * Creates a synapse on the specified segment. + * Creates a synapse on the specified segment that connects to the presynaptic cell. + * + * Note 1: If attemping to connect to an already synapsed presynaptic cell, we don't create + * a duplicit synapse, and just return early with the existing synapse. + * This has an effect that `connections.synapsesForSegment()` is not ensured to grow (by +1) + * after calling `createSynapse()` is the method conditionally skips. Users can query this by + * `connections.numSynapses(segment)`. + * + * Explanation: + * Biological motivation (?): + * There are structural constraints on the shapes of axons & synapses + * which prevent a large number duplicate of connections. + * + * It's important to prevent cells from growing duplicate synapses onto a segment, + * 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. * * @param segment Segment to create synapse on. * @param presynapticCell Cell to synapse on. * @param permanence Initial permanence of new synapse. * - * @reval Created synapse. + * @return Created synapse. //TODO consider changing to void, or explain what's returned */ Synapse createSynapse(const Segment segment, const CellIdx presynapticCell, @@ -312,6 +329,7 @@ class Connections : public Serializable * @retval Cell that this segment is on. */ CellIdx cellForSegment(const Segment segment) const { + NTA_ASSERT(segmentExists_(segment)); return segments_[segment].cell; } @@ -324,19 +342,6 @@ class Connections : public Serializable */ SegmentIdx idxOnCellForSegment(const Segment segment) const; - /** - * Get the cell for each provided segment. - * - * @param segments - * The segments to query - * - * @param cells - * Output array with the same length as 'segments' - */ - void mapSegmentsToCells(const Segment *segments_begin, - const Segment *segments_end, - CellIdx *cells_begin) const; - /** * Gets the segment that this synapse is on. * @@ -607,8 +612,8 @@ class Connections : public Serializable * @retval Number of segments. */ size_t numSegments() const { - NTA_ASSERT(segments_.size() >= destroyedSegments_.size()); - return segments_.size() - destroyedSegments_.size(); } + NTA_ASSERT(segments_.size() >= destroyedSegments_); + return segments_.size() - destroyedSegments_; } /** * Gets the number of segments on a cell. @@ -625,8 +630,8 @@ class Connections : public Serializable * @retval Number of synapses. */ size_t numSynapses() const { - NTA_ASSERT(synapses_.size() >= destroyedSynapses_.size()); - return synapses_.size() - destroyedSynapses_.size(); + NTA_ASSERT(synapses_.size() >= destroyedSynapses_); + return synapses_.size() - destroyedSynapses_; } /** @@ -709,9 +714,9 @@ class Connections : public Serializable private: std::vector cells_; std::vector segments_; - std::vector destroyedSegments_; + Segment destroyedSegments_ = 0; std::vector synapses_; - std::vector destroyedSynapses_; + Synapse destroyedSynapses_ = 0; //number of destroyed synapses Permanence connectedThreshold_; //TODO make const UInt32 iteration_ = 0; diff --git a/src/htm/algorithms/TemporalMemory.cpp b/src/htm/algorithms/TemporalMemory.cpp index 064eabb7e8..299133f00f 100644 --- a/src/htm/algorithms/TemporalMemory.cpp +++ b/src/htm/algorithms/TemporalMemory.cpp @@ -185,45 +185,26 @@ void TemporalMemory::growSynapses_( const Segment& segment, const SynapseIdx nDesiredNewSynapses, const vector &prevWinnerCells) { - // It's possible to optimize this, swapping candidates to the end as - // they're used. But this is awkward to mimic in other - // implementations, especially because it requires iterating over - // the existing synapses in a particular order. - + vector candidates(prevWinnerCells.begin(), prevWinnerCells.end()); NTA_ASSERT(std::is_sorted(candidates.begin(), candidates.end())); - // Skip cells that are already synapsed on by this segment - // Biological motivation (?): - // There are structural constraints on the shapes of axons & synapses - // which prevent a large number duplicate of connections. - // - // It's important to prevent cells from growing duplicate synapses onto a segment, - // 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)) { - const CellIdx presynapticCell = connections.dataForSynapse(synapse).presynapticCell; - const auto already = std::lower_bound(candidates.cbegin(), candidates.cend(), presynapticCell); - if (already != candidates.cend() && *already == presynapticCell) { - candidates.erase(already); - } - } - + //figure the number of new synapses to grow const size_t nActual = std::min(static_cast(nDesiredNewSynapses), candidates.size()); - - // Check if we're going to surpass the maximum number of synapses. + // ..Check if we're going to surpass the maximum number of synapses. Int overrun = static_cast(connections.numSynapses(segment) + nActual - maxSynapsesPerSegment_); if (overrun > 0) { - connections.destroyMinPermanenceSynapses(segment, static_cast(overrun), prevWinnerCells); + connections.destroyMinPermanenceSynapses(segment, static_cast(overrun), prevWinnerCells); //TODO move this functionality to Connections.createSynapse() } - - // Recalculate in case we weren't able to destroy as many synapses as needed. + // ..Recalculate in case we weren't able to destroy as many synapses as needed. const size_t nActualWithMax = std::min(nActual, static_cast(maxSynapsesPerSegment_) - connections.numSynapses(segment)); // Pick nActual cells randomly. - for (const auto syn : rng_.sample(candidates, static_cast(nActualWithMax))) { + rng_.shuffle(candidates.begin(), candidates.end()); + const size_t nDesired = connections.numSynapses(segment) + nActualWithMax; //num synapses on seg after this function (+-), see #COND + 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; connections.createSynapse(segment, syn, initialPermanence_); //TODO createSynapse consider creating a vector of new synapses at once? } } diff --git a/src/test/unit/algorithms/ConnectionsTest.cpp b/src/test/unit/algorithms/ConnectionsTest.cpp index 84ee2a240c..453449b20b 100644 --- a/src/test/unit/algorithms/ConnectionsTest.cpp +++ b/src/test/unit/algorithms/ConnectionsTest.cpp @@ -120,6 +120,20 @@ TEST(ConnectionsTest, testCreateSynapse) { SynapseData synapseData2 = connections.dataForSynapse(synapses[1]); ASSERT_EQ(synapseData2.presynapticCell, 150ul); ASSERT_NEAR((Permanence)0.48, synapseData2.permanence, htm::Epsilon); + //TODO add tests for failures +} + + +TEST(ConnectionsTest, testCreateSynapseAvoidDuplicitPresynapticConnections) { + Connections connections(1024); + UInt32 cell = 10; + Segment segment = connections.createSegment(cell); + + connections.createSynapse(segment, 50, 0.34f); + connections.createSynapse(segment, 51, 0.34f); + const size_t numSynapses = connections.synapsesForSegment(segment).size(); //created 2 synapses above + connections.createSynapse(segment, 50, 0.48f); //attempt to create already existing synapse (to presyn cell "50") -> skips as no duplication should happen + ASSERT_EQ(connections.synapsesForSegment(segment).size(), numSynapses) << "Duplicit synapses should not be created!"; } /** @@ -644,9 +658,9 @@ TEST(ConnectionsTest, testBumpSegment) { } /** - * Test the mapSegmentsToCells method. + * Test the mapping semgnets to cells by cellForSegment() method. */ -TEST(ConnectionsTest, testMapSegmentsToCells) { +TEST(ConnectionsTest, testCellForSegment) { Connections connections(1024); const Segment segment1 = connections.createSegment(42); @@ -654,12 +668,12 @@ TEST(ConnectionsTest, testMapSegmentsToCells) { const Segment segment3 = connections.createSegment(43); const vector segments = {segment1, segment2, segment3, segment1}; - vector cells(segments.size()); - - connections.mapSegmentsToCells( - segments.data(), segments.data() + segments.size(), cells.data()); - const vector expected = {42, 42, 43, 42}; + vector cells; + + for(auto seg : segments) { + cells.push_back(connections.cellForSegment(seg)); + } ASSERT_EQ(expected, cells); }