Are dmin and dminx send to kilosort4?

[ecobost]

Kilosort4 has two params for the vertical and horizontal spacing between contacts:

    'dmin': {
        'gui_name': 'dmin', 'type': float, 'min': 0, 'max': np.inf,
        'exclude': [0], 'default': None, 'step': 'spike detection',
        'description':
            """
            Vertical spacing of template centers used for spike detection,
            in microns. Determined automatically by default.
            """
    },

    'dminx': {
        'gui_name': 'dminx', 'type': float, 'min': 0, 'max': np.inf,
        'exclude': [0], 'default': 32, 'step': 'spike detection',
        'description':
            """
            Horizontal spacing of template centers used for spike detection,
            in microns. The default 32um should work well for Neuropixels 1 and
            Neuropixels 2 probes. For other probe geometries, try setting this 
            to the median lateral distance between contacts to start.
            """
    },

Does SpikeInterface provide these to kilosort when calling run_sorter()? Should it?

EDIT: It seems dmin will be computed as median(diff(unique(y_coords))) anyway while they intentionally disabled automatic calculation of dminx. I guess there is a reason for that so maybe it's ok that spikeinterface does not send dminx = median(diff(unique(x_coords))) .

[samuelgarcia]

Oups I was not aware of this.
@alejoe91 @chrishalcrow @JoeZiminski : any idea on this ?

[JoeZiminski]

Hey @ecobost good spot, I think it makes sense to expose these parameters on the SI side as people might want to change these for non-neuropixel probes

[ecobost]

@JoeZiminski they are exposed! I can call for instance:

ss.run_sorter('kilosort4', recording=nhp_neuropixel,
  ...
  # dmin=20, # vertical spacing between channels (not needed, default is already median(diff(unique(y_positions))) = 20)
  dminx=103, # horizontal spacing between channels (default: 32)
)

Question was whether they should automagically be filled in by Spikeinterface (like fs or n_chan_bin), but as I said in the EDIT above, it seems KS people decided against computing dminx automatically (don't know why) so it might not be worth computing it automatically here either. Up to the user to read the docs and set dminx correctly 🤷🏽
SI could maybe raise a warning if median(diff(unique(x_coords))) is very far from 32 but I don't feel strongly either way.

[JoeZiminski]

(edit: can ignore, they are propagated)

Hey @ecobost thanks for this, I think they are accepted as **sorter_kwargs but run_sorter but it doesn't look like they are actually used in the kilosort4 wrapper. Could you check by setting a breakpoint() inside kilosort4's sort function and see if the arguments are propagated?

I wonder if we should move towards wrapping only the kilosort4 entry function (mentioned briefly here #4387), that way we don't need to worry about this propagation to staying up to date with kilosort internal API. At least, we can check in this function if there are any unhandled parameters remaining, and throw an error? (assume indeed I've not made an oversight, and the arguments are not being propagated).

[ecobost]

It is definitely propragated cause I just tried dminx=sssjs'` and run_sorter failed with:

SpikeSortingError: Spike sorting error trace:
Traceback (most recent call last):
  File "/home/ecobost/Documents/California/spikeinterface/src/spikeinterface/sorters/basesorter.py", line 269", in run_from_folder
    SorterClass._run_from_folder(sorter_output_folder, sorter_params, verbose)
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/ecobost/Documents/California/spikeinterface/src/spikeinterface/sorters/external/kilosort4.py", line 418, in _run_from_folder
    st, tF, Wall0, clu0 = detect_spikes(**detect_spikes_kwargs)
                          ~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/ecobost/Documents/California/sptest/.venv/lib/python3.13/site-packages/kilosort/run_kilosort.py", line 787, in detect_spikes
    st0, tF, ops = spikedetect.run(
                   ~~~~~~~~~~~~~~~^
        ops, bfile, device=device, progress_bar=progress_bar,
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        clear_cache=clear_cache, verbose=verbose
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        )
        ^
  File "/home/ecobost/Documents/California/sptest/.venv/lib/python3.13/site-packages/kilosort/spikedetect.py", line 124, in template_centers
    nx = np.round((xmax - xmin) / (dminx/2)) + 1
                                   ~~~~~^~
TypeError: unsupported operand type(s) for /: 'str' and 'int'

though I don't understand how is that sent because the detect_spike_kwargs does not seem to have it:

spikeinterface/src/spikeinterface/sorters/external/kilosort4.py

Line 406 in 2bc9006

detect_spikes_kwargs = dict(

I just assumed everything shown in ss.get_default_sorter_params('kilosort4') (essentially anything in kilosort/params.py) was propagated, is that not true?

EDIT: I tested the ks params I use (highpass_cutoff, dmin, dminx, nearest_chans, max_channel_distance, duplicate_spike_ms and position_limit) and they are all propagetd fine.

[JoeZiminski]

Cool thanks @ecobost I was looking in the wrong place! They indeed passed through this settings dict. The default params of ks are copied here then overwritten if they are propagated. Apologies for the confusion!

Agree with your point, I would also err on leaving the default kilosort values rather than trying to compute them on the fly. But indeed if the probe has dimensions very different to Neuropixels it makes sense to throw a warning. I think often it's not clear to people that KS is very much designed for NP probes.