Skip to content

Connections create synapse improvements #746

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.

Sign up for GitHub

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

Merged
breznak merged 4 commits into from
Nov 12, 2019
Merged

Connections create synapse improvements #746

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8b371dc
Connections.mapSegmentsToCells removed
breznak Nov 5, 2019
194363e
Connections: createSynapse() checks for duplicit presynaptic synapses
breznak Nov 5, 2019
06ca0c2
Connections: remove destroyedSynapse/Segments_ buffers
breznak Nov 6, 2019
183f30f
Merge branch 'master' into connections_createSynapse
dkeeney Nov 12, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/examples/hotgym/HelloSPTP.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
70 changes: 30 additions & 40 deletions src/htm/algorithms/Connections.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,7 @@ Connections::Connections(const CellIdx numCells,
void Connections::initialize(CellIdx numCells, Permanence connectedThreshold, bool timeseries) {
cells_ = vector<CellData>(numCells);
segments_.clear();
destroyedSegments_.clear();
synapses_.clear();
destroyedSynapses_.clear();
potentialSynapsesForPresynapticCell_.clear();
connectedSynapsesForPresynapticCell_.clear();
potentialSegmentsForPresynapticCell_.clear();
Expand Down Expand Up @@ -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();
Copy link
Member Author

@breznak breznak Nov 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replacing the destroyed_ buffers. Makes code less complicated and the performance is the same.

Copy link
Collaborator

@ctrl-z-9000-times ctrl-z-9000-times Nov 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this will never reuse a destroyed segment?

IMO: in a typical application, segments should never be destroyed, just created. If the HTM reaches full capacity and needs to make room for new information, then the HTM should remove synapses from a segment instead of removing the segment itself.

Copy link
Member Author

@breznak breznak Nov 7, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this will never reuse a destroyed segment?
IMO: in a typical application, segments should never be destroyed, just created.

yes. if you want, I can move this commit to a separete PR to discuss there? You're right we almost never remove segments/synapses (cca 0.X% on hotgym benchmark).

If the HTM reaches full capacity and needs to make room for new information, then the HTM should remove synapses from a segment instead of removing the segment itself.

if that is valid requirement, we should remove altogether the method destroySegment().

HTM reaches full capacity and needs to make room

when is this? When synapses on a segment are full, when synapses on all segments are maxed, ...?

should remove synapses from a segment instead of removing the segment itself.

then we'd need to limit this pruning to some reasonable value (keep more synapses than activation threshold), otherwise the seg can never be active again.

  • currently we do this the other way, that such segment dies off (which I think is more plausible than setting a limit on a number of syns that need be left).

destroyedSegments_.pop_back();
} else { //create a new segment
NTA_CHECK(segments_.size() < std::numeric_limits<Segment>::max()) << "Add segment failed: Range of Segment (data-type) insufficinet size."
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();
segment = static_cast<Segment>(segments_.size());
const SegmentData& segmentData = SegmentData(cell, iteration_, nextSegmentOrdinal_++);
segments_.push_back(segmentData);
}
const Segment segment = static_cast<Segment>(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
Expand All @@ -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
Copy link
Member Author

@breznak breznak Nov 6, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1/ moving code for avoiding duplicate presynaptic connections on the same segment from TM,

  • implementation is easier (&faster about 8% for TM)

2/ was a bug that it was checked only in TM, is applicable generally and should be part of Connections.

}
} //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<Synapse>::max()) << "Add synapse failed: Range of Synapse (data-type) insufficient size."
NTA_ASSERT(synapses_.size() < std::numeric_limits<Synapse>::max()) << "Add synapse failed: Range of Synapse (data-type) insufficient size."
<< synapses_.size() << " < " << (size_t)std::numeric_limits<Synapse>::max();
synapse = static_cast<Synapse>(synapses_.size());
synapses_.emplace_back(SynapseData());
}
const Synapse synapse = static_cast<Synapse>(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];
Expand Down Expand Up @@ -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<Segment> &segmentsOnCell = cells_[segmentData.cell].segments;
return (std::find(segmentsOnCell.cbegin(), segmentsOnCell.cend(), segment) !=
Expand Down Expand Up @@ -211,7 +218,7 @@ void Connections::destroySegment(const Segment segment) {
NTA_ASSERT(*segmentOnCell == segment);

cellData.segments.erase(segmentOnCell);
destroyedSegments_.push_back(segment);
destroyedSegments_++;
}


Expand Down Expand Up @@ -261,8 +268,7 @@ void Connections::destroySynapse(const Synapse synapse) {
NTA_ASSERT(*synapseOnSegment == synapse);

segmentData.synapses.erase(synapseOnSegment);

destroyedSynapses_.push_back(synapse);
destroyedSynapses_++;
}


Expand Down Expand Up @@ -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];
Expand Down Expand Up @@ -606,7 +599,6 @@ void Connections::destroyMinPermanenceSynapses(
const vector<CellIdx> &excludeCells)
{
NTA_ASSERT( nDestroy >= 0 );
if( nDestroy <= 0 ) return; // Nothing to do.

// Don't destroy any cells that are in excludeCells.
vector<Synapse> destroyCandidates;
Expand Down Expand Up @@ -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;
}
Expand Down
47 changes: 26 additions & 21 deletions src/htm/algorithms/Connections.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,13 +246,30 @@ class Connections : public Serializable
const SegmentIdx maxSegmentsPerCell = std::numeric_limits<SegmentIdx>::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,
Expand Down Expand Up @@ -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;
}

Expand All @@ -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,
Copy link
Member Author

@breznak breznak Nov 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed unused method, cellForSegment() provides the same.

const Segment *segments_end,
CellIdx *cells_begin) const;

/**
* Gets the segment that this synapse is on.
*
Expand Down Expand Up @@ -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.
Expand All @@ -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_;
}

/**
Expand Down Expand Up @@ -709,9 +714,9 @@ class Connections : public Serializable
private:
std::vector<CellData> cells_;
std::vector<SegmentData> segments_;
std::vector<Segment> destroyedSegments_;
Segment destroyedSegments_ = 0;
std::vector<SynapseData> synapses_;
std::vector<Synapse> destroyedSynapses_;
Synapse destroyedSynapses_ = 0; //number of destroyed synapses
Permanence connectedThreshold_; //TODO make const
UInt32 iteration_ = 0;

Expand Down
39 changes: 10 additions & 29 deletions src/htm/algorithms/TemporalMemory.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,45 +185,26 @@ void TemporalMemory::growSynapses_(
const Segment& segment,
const SynapseIdx nDesiredNewSynapses,
const vector<CellIdx> &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<CellIdx> 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<size_t>(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<Int>(connections.numSynapses(segment) + nActual - maxSynapsesPerSegment_);
if (overrun > 0) {
connections.destroyMinPermanenceSynapses(segment, static_cast<Int>(overrun), prevWinnerCells);
connections.destroyMinPermanenceSynapses(segment, static_cast<Int>(overrun), prevWinnerCells); //TODO move this functionality to Connections.createSynapse()
Copy link
Member Author

@breznak breznak Nov 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OT: for next PR

}

// 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<size_t>(maxSynapsesPerSegment_) - connections.numSynapses(segment));

// Pick nActual cells randomly.
for (const auto syn : rng_.sample(candidates, static_cast<UInt>(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?
}
}
Expand Down
28 changes: 21 additions & 7 deletions src/test/unit/algorithms/ConnectionsTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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!";
Copy link
Member Author

@breznak breznak Nov 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test the new "check dupes, skip" behavior of createSynapse.

}

/**
Expand Down Expand Up @@ -644,22 +658,22 @@ 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);
const Segment segment2 = connections.createSegment(42);
const Segment segment3 = connections.createSegment(43);

const vector<Segment> segments = {segment1, segment2, segment3, segment1};
vector<CellIdx> cells(segments.size());

connections.mapSegmentsToCells(
segments.data(), segments.data() + segments.size(), cells.data());

const vector<CellIdx> expected = {42, 42, 43, 42};
vector<CellIdx> cells;

for(auto seg : segments) {
cells.push_back(connections.cellForSegment(seg));
}
ASSERT_EQ(expected, cells);
}

Expand Down