Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unexpected: AssertionError: 'inter_sample_shift' is not a property! #3628

Open
brian-arnold opened this issue Jan 18, 2025 · 6 comments · May be fixed by #3629
Open

Unexpected: AssertionError: 'inter_sample_shift' is not a property! #3628

brian-arnold opened this issue Jan 18, 2025 · 6 comments · May be fixed by #3629

Comments

@brian-arnold
Copy link

brian-arnold commented Jan 18, 2025

Hello!
Thank you for producing and maintaining such a fabulous piece of software!

Briefly, when I run Kilosort4 via runsorter.run_sorter_local, it complains that

AssertionError: 'inter_sample_shift' is not a property!

even though, on the line immediately preceding this run_sorter command, the following passes:

assert 'inter_sample_shift' in rec.get_property_keys().

I created a minimal example below to reproduce this behavior. The last section contains the error, and everything above is setup. For context, I originally encountered this assert error while preprocessing and motion correcting using spikeinterface, then passing this processed recording to Kilosort4 while skipping preprocessing and motion correction. In the example below, I run Kilosort4 on unprocessed data (which works) just to demonstrate the unexpected behavior.

Thanks for any advice!!

`

Load config file

config_file = '/mnt/lab/users/barnold/spike_sorting/package/spike_sort_config.yaml'
with open(config_file, 'r') as f:
config = yaml.safe_load(f)

Configure probe, specifying geometry of channels and other kwargs

positions = []
for i in range(config['probe']['n_row']):
# add channels by row, 2 channels per row
positions.append([0, i * config['probe']['pitch_row']])
positions.append([config['probe']['pitch_col'], i * config['probe']['pitch_row']])
positions = np.array(positions)

probe = Probe(ndim=config['probe']['n_dim'], si_units=config['probe']['units'])

probe.set_contacts(positions=positions,
shapes=config['probe']['shape'],
shape_params={'width': config['probe']['chan_width']})

probe.set_device_channel_indices(np.arange(config['probe']['n_chan']-1))

probe_kwargs = {'sampling_frequency': 30000.0561519867,
'num_channels': 385,
'dtype': 'int16',
'gain_to_uV': 2.3438,
'offset_to_uV': 0}

Load NP recording, assign probe

NP_file = '/mnt/mscratch1/Tarzan/2024-12-12_12-42-50/2024-12-12_12-43-01/NPElectrophysiology_2_2.bin'
rec = extractors.read_binary(file_paths = NP_file, **probe_kwargs)
rec = rec.select_channels([i for i in range(config['probe']['n_chan']-1)])
rec = rec.set_probe(probe)

set sample_shifts

sample_shifts = extractors.neuropixels_utils.get_neuropixels_sample_shifts(num_channels=rec.get_num_channels(), num_channels_per_adc=12)
rec.set_property('inter_sample_shift', sample_shifts)

take a small sample for testing

start_time_secs, duration_secs = 0,10
rec = rec.frame_slice(start_frame=int(start_time_secs * rec.sampling_frequency), end_frame=int((start_time_secs + duration_secs) * rec.sampling_frequency))

Unexpected AssertionError

this works like a charm

sorting = runsorter.run_sorter_local(sorter_name="kilosort4",
recording=rec,
folder=config['out_dir'],
**config['kilosort'])

do standard preprocessing using spikeinterface's preprocessing module

rec = preprocessing.bandpass_filter(recording=rec,
freq_min=config['ap_freqband']['lower'],
freq_max=config['ap_freqband']['upper'])

bad_channel_ids, channel_labels = preprocessing.detect_bad_channels(rec) # for this step, recording assumed to be filtered
rec = rec.remove_channels(remove_channel_ids=bad_channel_ids)
rec = preprocessing.phase_shift(recording=rec)
rec = preprocessing.common_reference(recording=rec, **config['common_reference'])

this passes

assert 'inter_sample_shift' in rec.get_property_keys()

this does NOT work and fails because of the assert error that just passed in previous line

sorting = runsorter.run_sorter_local(sorter_name="kilosort4",
recording=rec,
folder=config['out_dir'],
**config['kilosort']) `

This is the error message from the last command:

SpikeSortingError: Spike sorting error trace: ... File "/usr/local/lib/python3.9/dist-packages/spikeinterface/preprocessing/phase_shift.py", line 46, in __init__ assert "inter_sample_shift" in recording.get_property_keys(), "'inter_sample_shift' is not a property!" AssertionError: 'inter_sample_shift' is not a property!

Brian

@zm711
Copy link
Collaborator

zm711 commented Jan 18, 2025

Hey Brian, quick additional questions to help us:

version of spikeinterface
version of kilosort4

@brian-arnold
Copy link
Author

brian-arnold commented Jan 19, 2025

Ah, apologies for not including in the first message.

import spikeinterface as s
print(s.__version__)

showed 0.101.2

import kilosort as k
print(k.__version__)

showed 4.0.22

Thanks!

@zm711
Copy link
Collaborator

zm711 commented Jan 19, 2025

Thanks Brian.

One quick thing. We recently discovered that we don't officially support Kilosort 4.0.2x yet (they update so fast that we need to double check our wrapper). I don't think that should be the problem here though.

Do you have more of the error trace (it should be in the json file). It might be that we are not propagating a property appropriately, but more trace would be better to help find where this is happening.

@brian-arnold
Copy link
Author

Hi Zach,
Thanks for your reply! I pasted the json below. I also reran with the oldest version of Kilosort4 supported by spikeinterface (4.0.16) and encountered the same assertion error.

Interestingly, I ran the same code I posted above, but under the section I named do standard preprocessing using spikeinterface's preprocessing module, if I comment out the following line of code:

rec = preprocessing.phase_shift(recording=rec)

the run_sorter_local runs successfully afterwards. But, regardless of what I do, oddly the following assert

assert 'inter_sample_shift' in rec.get_property_keys()

passes right before running run_sorter_local in which it errors.

{
"sorter_name": "kilosort4",
"sorter_version": "4.0.16",
"datetime": "2025-01-20T05:20:12.109052",
"runtime_trace": [],
"error": true,
"error_trace": [
"Traceback (most recent call last):",
" File "/usr/local/lib/python3.9/dist-packages/spikeinterface/sorters/basesorter.py", line 261, in run_from_folder",
" SorterClass._run_from_folder(sorter_output_folder, sorter_params, verbose)",
" File "/usr/local/lib/python3.9/dist-packages/spikeinterface/sorters/external/kilosort4.py", line 233, in _run_from_folder",
" recording = cls.load_recording_from_folder(sorter_output_folder.parent, with_warnings=False)",
" File "/usr/local/lib/python3.9/dist-packages/spikeinterface/sorters/basesorter.py", line 212, in load_recording_from_folder",
" recording = load_extractor(json_file, base_folder=output_folder)",
" File "/usr/local/lib/python3.9/dist-packages/spikeinterface/core/base.py", line 1196, in load_extractor",
" return BaseExtractor.load(file_or_folder_or_dict, base_folder=base_folder)",
" File "/usr/local/lib/python3.9/dist-packages/spikeinterface/core/base.py", line 783, in load",
" extractor = BaseExtractor.from_dict(d, base_folder=base_folder)",
" File "/usr/local/lib/python3.9/dist-packages/spikeinterface/core/base.py", line 529, in from_dict",
" extractor = _load_extractor_from_dict(dictionary)",
" File "/usr/local/lib/python3.9/dist-packages/spikeinterface/core/base.py", line 1116, in _load_extractor_from_dict",
" new_kwargs[name] = _load_extractor_from_dict(value)",
" File "/usr/local/lib/python3.9/dist-packages/spikeinterface/core/base.py", line 1135, in _load_extractor_from_dict",
" extractor = extractor_class(**new_kwargs)",
" File "/usr/local/lib/python3.9/dist-packages/spikeinterface/preprocessing/phase_shift.py", line 46, in init",
" assert "inter_sample_shift" in recording.get_property_keys(), "'inter_sample_shift' is not a property!"",
" AssertionError: 'inter_sample_shift' is not a property!"
],
"run_time": null

@alejoe91
Copy link
Member

alejoe91 commented Jan 20, 2025

Hi guys,

The problem is that by default properties in memory are not dumped to JSON. This results in the fact that when the run sorter reloads the recording, inter_sample_shift is not found.
I'll make a PR to expose the parameter and change the behavior.

@alejoe91 alejoe91 linked a pull request Jan 20, 2025 that will close this issue
@brian-arnold
Copy link
Author

Hello! I just wanted to say that I saw there was a pull request initiated and that I'd be happy to help if possible!

On this, I'm also currently using dump_to_json to save preprocessed data for downstream steps to load instead of repeating preprocessing (because these downstream steps may get run at a different time on a different node). I saw this in one of your nice tutorials here. Is this not recommended with the current issue?

Thanks!
Brian

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants