Fix synapse replay matching with population#77
Conversation
Codecov Report❌ Patch coverage is
... and 119 files with indirect coverage changes 🚀 New features to boost your workflow:
|
AurelienJaquier
left a comment
AurelienJaquier
left a comment
There was a problem hiding this comment.
looks good, just have a couple of comments/questions
| spikes["t"].clip(lower=0., inplace=True) | ||
| spikes["t"] = spikes["t"].clip(lower=0.0) | ||
|
|
||
| # Group spikes by node_id and ensure spike times are sorted in ascending order. |
There was a problem hiding this comment.
You don't want to keep this comment?
There was a problem hiding this comment.
Yes, that's a good idea, I added it back and updated it.
| stimulus = DummyStimulus(spike_file) | ||
| Cell.add_synapse_replay(cell, stimulus, -20.0, "soma") | ||
|
|
||
| assert set(cell.connections.keys()) == {("vpm", 0)} |
There was a problem hiding this comment.
Quick question to be sure that I understand how this works correctly. Here, we have only vpm and we do not have s1 because the only stimulus we gave to add_synapse_replay comes from an h5 file that only has VPM as the population.
Is that correct?
There was a problem hiding this comment.
Yes exactly, the replay file only contains VPM (L939) spikes, so only the synapse coming from VPM should get connected. The S1 synapse has the same pre_gid but since its source population is different, it should not match. That’s why we only expect {("vpm", 0)} here.
AurelienJaquier
left a comment
AurelienJaquier
left a comment
There was a problem hiding this comment.
thanks for the explanations.
I approve this PR!
2 participants
Fix synapse replay matching by including population in CellId.
Previously, replay spikes were matched only by node_id which caused incorrect behavior with multiple populations.