Skip to content
This repository has been archived by the owner on Jun 6, 2023. It is now read-only.

Propagate raise_error to get_results #207

Merged
merged 4 commits into from
Mar 11, 2021
Merged

Conversation

alejoe91
Copy link
Member

Fixes https://github.com/SpikeInterface/spikesorters/issues/206

@eejd can you try that out? Note that you need the master version of spikeextractors and spiketoolkit

spikesorters/basesorter.py Outdated Show resolved Hide resolved
@alejoe91 alejoe91 merged commit 12e3571 into master Mar 11, 2021
@alejoe91 alejoe91 deleted the fix_raise_error_when_grouping branch March 12, 2021 20:32
@eejd
Copy link

eejd commented Mar 26, 2021

Sorry, was off this while handling something, I'm going to test but I wanted to know if the return_rescaled is also merged into master. I was working from return_rescaled.

@alejoe91
Copy link
Member Author

Yes into master and released :)

@eejd
Copy link

eejd commented Mar 26, 2021

Ok, not sure if this was a problem with the local work I was doing to sort the loading (based on return_rescaled, but doing a compare with (your) upstream repo I'm not seeing a collision. I'm running the script that was working with that branch when last run. I'm getting this error:

Exception ignored in: <function BaseExtractor.__del__ at 0x7f7a612eb790>
Traceback (most recent call last):
  File "repos/spikeextractors/spikeextractors/baseextractor.py", line 36, in __del__
    for memmap_obj in self._memmap_files:
AttributeError: 'OpenEphysRecordingExtractor' object has no attribute '_memmap_files'
Traceback (most recent call last):
  File "repos/pipeline/Python/EphysTest.py", line 57, in <module>
    recording_oe = se.OpenEphysRecordingExtractor(session_folder + session_open_ephys, dtype='int16')
TypeError: __init__() got an unexpected keyword argument 'dtype'

I'll dig into this more now, but if you see something obvious, let me know.

@alejoe91
Copy link
Member Author

Yep the openephys extractor doesn't have a style arg anymore as this is handled internally by the return_scaled

@eejd
Copy link

eejd commented Mar 27, 2021

Ok, I removed it and ran through. Seemed good, until I realised I'd been still running a version of pyopenephys forked from an old release for reasons that had been necessary but I can't reconstruct. So, mixed results after further testing. First, when running with my pyopenephys 1.0.7 forked branch for hacking the 'int16' problem, I got:

repos/spikesorters/spikesorters/basesorter.py:249: UserWarning: Sorting output 7 could not be loaded
  warnings.warn(f"Sorting output {i} could not be loaded")
Elapsed time:  1156.4461076259613

Realising that I was getting a deprecation warning, I switched to a clean 1.1.3 pyopenephys and encountered:

Loading Open-Ephys: reading settings...
Decoding data from  openephys  format
[34, 36, 38, 40, 17, 19, 21, 23, 9, 11, 13, 15, 1, 3, 5, 7, 2, 4, 6, 8, 10, 12, 14, 16, 18, 20, 22, 24, 26, 28, 30, 32, 42, 44, 46, 48, 50, 52, 54, 56, 58, 60, 62, 57, 59, 61, 63, 49, 51, 53, 55, 41, 43, 45, 47, 33, 35, 37, 39, 25, 27, 29, 31]
['gain', 'group', 'location', 'offset']
KiloSort2 (Grouped)
Traceback (most recent call last):
  File "repos/pipeline/Python/EphysTest.py", line 126, in <module>
    sorting_tetrodes = ss.run_sorter('kilosort2',
  File "repos/spikesorters/spikesorters/sorterlist.py", line 92, in run_sorter
    sorter.set_params(**params)
  File "repos/spikesorters/spikesorters/basesorter.py", line 118, in set_params
    self._dump_params()
  File "repos/spikesorters/spikesorters/basesorter.py", line 126, in _dump_params
    json.dump(_check_json(params), f, indent=4)
  File "repos/spikeextractors/spikeextractors/baseextractor.py", line 602, in _check_json
    d[k] = _check_json(v)
  File "repos/spikeextractors/spikeextractors/baseextractor.py", line 602, in _check_json
    d[k] = _check_json(v)
  File "repos/spikeextractors/spikeextractors/baseextractor.py", line 602, in _check_json
    d[k] = _check_json(v)
  [Previous line repeated 3 more times]
  File "repos/spikeextractors/spikeextractors/baseextractor.py", line 614, in _check_json
    if len(v) > 0:
TypeError: len() of unsized object

So walking backwards, pyopenephys 1.1.2 produced:

Loading Open-Ephys: reading settings...
Decoding data from  openephys  format
Traceback (most recent call last):
  File "repos/pipeline/Python/EphysTest.py", line 60, in <module>
    recording = se.CacheRecordingExtractor(recording_oe, save_path=path.expanduser('~/Workspaces/Research/5HTTD/local/'))
  File "repos/spikeextractors/spikeextractors/cacheextractors.py", line 35, in __init__
    BinDatRecordingExtractor.__init__(self, self._tmp_file, numchan=recording.get_num_channels(),
  File "repos/spikeextractors/spikeextractors/extractors/bindatrecordingextractor/bindatrecordingextractor.py", line 63, in __init__
    self._sampling_frequency = float(sampling_frequency)
TypeError: float() argument must be a string or a number, not 'NoneType'

and with pyopenephys 1.1.0:

Loading Open-Ephys: reading settings...
Decoding data from  openephys  format
Traceback (most recent call last):
  File "repos/pipeline/Python/EphysTest.py", line 58, in <module>
    recording_oe = se.OpenEphysRecordingExtractor(session_folder + session_open_ephys)
  File "repos/spikeextractors/spikeextractors/extractors/openephysextractors/openephysextractors.py", line 42, in __init__
    self._set_analogsignal(self._recording.analog_signals[0])
  File "repos/spikeextractors/spikeextractors/extractors/openephysextractors/openephysextractors.py", line 52, in _set_analogsignal
    self.set_channel_gains(gains=self._analogsignals.gain)
AttributeError: 'AnalogSignal' object has no attribute 'gain'

Finally, reverting to a clean pyopenephys 1.0.7, it worked again.

@alejoe91
Copy link
Member Author

alejoe91 commented Mar 27, 2021

@eejd ok it seems to have a problem in dumping the OpenEphys extractor.

If recording is the variable that you pass to the sorter, what's the output of recording.dump_to_dict()? (using latest spikeinterface + pyopenephys versions)

@eejd
Copy link

eejd commented Mar 29, 2021

So I think the three problems were each different (i.e. that occurred at a different part of the processing pipeline). With 1.1.3, the problem is at the call to the sorter (KiloSort2 in this case). Note the final cause of the exception:

File "repos/spikeextractors/spikeextractors/baseextractor.py", line 614, in _check_json
    if len(v) > 0:
TypeError: len() of unsized object

For the errors <= 1.1.3, the problems are occurring before the call to the sorter. With 1.1.2 it is in the call to the CacheRecordingExtractor with a save location. If I dump_to_dict using the_dict = recording_oe.dump_to_dict() there, it succeeds but recording = se.CacheRecordingExtractor(recording_oe, save_path= is the problem:

Traceback (most recent call last):
  File "/home/dep/Workspaces/Research/5HTTD/repos/pipeline/Python/EphysTest.py", line 61, in <module>
    recording = se.CacheRecordingExtractor(recording_oe, save_path=path.expanduser('~/Workspaces/Research/5HTTD/local/'))
  File "/home/dep/Workspaces/Research/5HTTD/repos/spikeextractors/spikeextractors/cacheextractors.py", line 35, in __init__
    BinDatRecordingExtractor.__init__(self, self._tmp_file, numchan=recording.get_num_channels(),
  File "repos/spikeextractors/spikeextractors/extractors/bindatrecordingextractor/bindatrecordingextractor.py", line 63, in __init__
    self._sampling_frequency = float(sampling_frequency)
TypeError: float() argument must be a string or a number, not 'NoneType'

1.1.0 fails at recording_oe = se.OpenEphysRecordingExtractor(session_folder + session_open_ephys).

SI versions:

spikecomparison    0.3.2
spikeextractors    0.9.5
spikefeatures      0.1.2
spikemetrics       0.2.4
spikesorters       0.4.4
spiketoolkit       0.7.4
spikewidgets       0.5.3

@alejoe91
Copy link
Member Author

Thanks @eejd !

I think we should focus on solving the first problem. Could I ask you if you are willing to share this dataset with me? It would greatly help debugging it!

@alejoe91
Copy link
Member Author

Quick test: what does recording_oe.get_sampling_frequency() return?

@eejd
Copy link

eejd commented Mar 29, 2021

Thanks @eejd !

I think we should focus on solving the first problem. Could I ask you if you are willing to share this dataset with me? It would greatly help debugging it!

I think so, let me check with the collaborators. Also, if you want to do a quick audio/video call that's no problem. Also, I'll be working on this more continuously again, so I will try to look through the recent merges to understand the change.

@eejd
Copy link

eejd commented Mar 29, 2021

Quick test: what does recording_oe.get_sampling_frequency() return?

recording_oe.get_sampling_frequency()
array(30000.)

@eejd
Copy link

eejd commented Mar 31, 2021

@alejoe91 How should I prepare and send the data set?

@alejoe91
Copy link
Member Author

You can send me an email and share the data on gdrive with a script/notebook to reproduce the error :)

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

Successfully merging this pull request may close these issues.

Passing 'run_sorter_kwargs' with 'raise_error' throws Bad Parameter
2 participants