Connections: computeActivity() consider using only 3 arg version

[breznak]

removing the 2 arg version;

From #331 (comment)

This would require std::vector<SynapseIdx> &numActivePotentialSynapsesForSegment, be used as an optional param, with default value that skips its execution (unless given by user)

void computeActivity(std::vector &numActiveConnectedSynapsesForSegment,
std::vector &numActivePotentialSynapsesForSegment,
const std::vector &activePresynapticCells) const;
void computeActivity(std::vector &numActiveConnectedSynapsesForSegment, //TODO remove the 2 arg version, use only the 3 arg (with 1 being optional)
const std::vector &activePresynapticCells) const;

ctrl-z-9000-times 7 hours ago Member
No can do, the two different versions do different things. One computes less than have as much as the other, making it significantly faster. Removing it would likely double the spatial poolers runtime.

breznak 6 hours ago Author Member

One computes less than have as much as the other, making it significantly faster.

I'm aware, if we use {} the loop will skip, or some empheral value, as in TM(?)

[ctrl-z-9000-times]

if we use {} the loop will skip

That would work.

When the SP call connections::compute it passes an empty vector for numActivePotentialSynapsesForSegment and the compute method detects this and does not calculate it.

The only thing left to ask is, can we reorder the arguments so that it has a default argument?

// Current order:
    vector<UInt32> &numActiveConnectedSynapsesForSegment,
    vector<UInt32> &numActivePotentialSynapsesForSegment,
    const vector<CellIdx> &activePresynapticCells

// Proposed order:
    const vector<CellIdx> &activePresynapticCells,
    vector<UInt32> &numActiveConnectedSynapsesForSegment,
    vector<UInt32> &numActivePotentialSynapsesForSegment = {} // Default argument

[breznak]

The only thing left to ask is, can we reorder the arguments so that it has a default argument?

I'm for it 👍 it'll be an API breaking change, but not a really visible one:

• NetworkAPI & bindings: unaffected
  • even C++ TM,SP public users are OK
    • so only direct "experimentators with C++ Connections" would notice, and as C++ Conn is fairly new, I'm quite certain that's about the 3 of us.

I'm not sure why it's not part of the #331 , but from the name ...SynapsesForSegment the vector type should be SynapseIdx

// Proposed order2:
    const vector<CellIdx> &activePresynapticCells,
    vector<SynapseIdx> &numActiveConnectedSynapsesForSegment,
    vector<SynapseIdx> &numActivePotentialSynapsesForSegment = {} // Default argument

the advantage being CellIdx, SynapseIdx are different types, so for any existing users the change would result in compile-error, not a silent break in functionality if CellIdx (aka UInt32) is kept.