[cindykrafft]

Fixes #1042.

What

swarmsplitter.check_CCG (since v4.1.5) contains

K , T = compute_CCG(st1, st2, nbins = nbins, tbin = tbin)
if len(st1) == 0 or len(st2 == 0) or T == 0:
    return False, False

st2 == 0 is a boolean array, so len(st2 == 0) equals len(st2) and is truthy for every non-empty spike train. The guard therefore fires on all real inputs and check_CCG always returns (False, False). Downstream, refractoriness() can never return 1, so the refractory-CCG veto in swarmsplitter.split ("never split a pair whose cross-correlogram is refractory") has been dead code in v4.1.5–v4.1.7. Splits are currently decided by modularity and bimodality alone, which allows oversplitting of units whose halves remain mutually refractory (bursty, amplitude-varying, or drifting units) — exactly the case the veto was designed to catch. v4.0–v4.1.4 applied the veto.

There is a second problem in the same line: the guard sits after the compute_CCG call, but compute_CCG raises ValueError on .max() of an empty array, so the empty-train case the guard tests for crashes before the guard is reached. Only the T == 0 clause could ever usefully trigger (presumably the motivation from the ZeroDivisionError reports, e.g. #1002).

Fix

• correct len(st2 == 0) to len(st2) == 0;
• move the empty-train check above the compute_CCG call, keeping the T == 0 check after it.

Verification (kilosort 4.1.7, CPU)

A single simulated refractory neuron (3 ms refractory period, ~10 Hz, 20 min) split randomly in half — the canonical oversplit the veto should block. Its cross-CCG metrics are R12 = 0.0000, Q12 = 0.0000 (cross_refractory = True under the v4.1.4 logic), yet:

before: check_CCG(st1, st2) -> (False, False);  refractoriness -> 0  (split allowed)
after:  check_CCG(st1, st2) -> (True,  True);   refractoriness -> 1  (never split)

Edge cases after the fix:

check_CCG([1.0, 2.0], [])   -> (False, False)   (previously: ValueError in compute_CCG)
check_CCG([1.0], [1.0])     -> (False, False)   (T == 0 path, unchanged)

Standalone reproduction script: https://github.com/cindykrafft/research-software-audit/blob/main/audits/kilosort/verify/kl1_swarmsplitter_gate.py

🤖 Generated with Claude Code

https://claude.ai/code/session_012d2sJAD5EUp4GqAStqoBX7

[kvnloo]

@cindykrafft thanks for tracking this down — I took a pass at independently exercising the fix and the downstream "refractoriness()" behavior.

I tested the current behavior, this PR's fix, and a few deliberately incomplete/broken variants across empty inputs, refractory spike trains, and non-refractory controls. The PR version passed all 12 cases; importantly, a variant where "check_CCG()" was fixed but its result was effectively ignored downstream failed the end-to-end regression.

One thing that might be useful before landing this is a small regression test covering both levels:

def test_check_ccg_empty_inputs():
assert check_CCG(np.array([]), np.array([1.0])) == (False, False)
assert check_CCG(np.array([1.0]), np.array([])) == (False, False)

def test_refractoriness_blocks_refractory_split():
# Two halves of the same simulated refractory spike train.
...
assert check_CCG(st1, st2)[1]
assert refractoriness(st1, st2) == 1

The second assertion seems especially valuable because it protects the actual behavioral contract — a refractory pair should veto the split — rather than only guarding the typo itself.

My verification was CPU-only and isolated to these Kilosort modules, not a full GPU sorting run.

Thanks again for finding this.

[cindykrafft]

Thanks for checking it independently, and for the test sketch. I pushed eabed49 with tests/test_swarmsplitter.py, which covers both levels you describe:

• test_check_ccg_empty_inputs: an empty first, second or only train returns (False, False) instead of reaching compute_CCG;
• test_check_ccg_zero_duration: the T == 0 path;
• test_refractoriness_blocks_refractory_split: two random halves of one simulated refractory unit (3 ms refractory period, ~10 Hz, 20 min) are reported cross_refractory and refractoriness() returns 1;
• test_refractoriness_allows_independent_units: two independent units are not vetoed, so the fix does not simply block every split.

With this branch all four pass (CPU torch); with the swarmsplitter.py of v4.1.5 through v4.1.7 the empty-input test raises ValueError inside compute_CCG and the veto test fails with refractoriness() == 0, so the tests catch both halves of the original problem. Like your run, this is CPU-only and does not include a full sorting pass.

[kvnloo]

@cindykrafft thanks for adding these, I independently re-ran your unchanged tests/test_swarmsplitter.py from eabed49.

All four tests pass on CPU. Replacing only swarmsplitter.py with the PR base version reproduces the two expected failures: empty inputs reach compute_CCG(), and the refractory-pair check returns false.

The independent-unit control passes too, which protects against accidentally vetoing every split. This addresses the regression coverage I was suggesting; thanks for keeping it focused.

Verification boundary: AI-assisted testing of complete source modules and your test file, checked against GitHub's blob hashes. I used a minimal package initializer rather than the full Kilosort package setup, so this does not validate package initialization, the full test suite, or a GPU sorting run.