Abolish SegmentOverlap PR Discussion



I’m continuing the discussion here so as not to clutter or confuse discussions surrounding the primary purpose of this PR.


That key function is equivalent to checking a.cell < b.cell and then a.idx < b.idx as the secondary check. Andrew converted this check into a single scalar check because Python magically makes sorting faster when you return an integer from your key function. It uses the maxSegmentsPerCell because that guarantees correct key ordering – I thought this was a pretty clever trick on Andrew’s part. (This code still exists after this PR, it’s just in the TM’s activateDendrites method)

I see, so the segments are ordered according to cell index, and if ever two segments are on the same cell then the secondary index (which is local to the cell) is used to further order them amongst those belonging to the same cell. Multiplying the indexes times the maxSegmentsPerCell, has the effect of spreading out those segments which are on contiguous cells so that the first segment belonging to the next cell will have a sort order one greater than the index of the max segment of the previous cell! That is pretty clever! I just wish it had been more apparent to onlookers (namely me).

It sounds like the bug you’re describing is located in computeActivity. This has nothing to do with groupby2…

computeActivity()'s main purpose is to prepare the active and matching segments for the next TemporalMemory.compute() compute function, and the sorting at the end has a profound affect on the results of the computation in the TM’s compute method, so they’re not as “unrelated” as it might seem.

My Segments all have a global index, and from memory I observed the “flatIdx” to always correspond with my global index, which is why I made the comment that it should be used - especially when I observed that the segment.idx was always zero. I didn’t know that it was a local index.

Now that I understand what is happening (and now that I understand more about @amalta’s wizardry :wink:) , I don’t think there’s any bug related to either the Connections.computeActivity() -or- the groupby2 functionality. Thank you for the explanation.