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

OpenEphysRawIO unequal signal lengths after clipping #1336

Closed
weiglszonja opened this issue Oct 16, 2023 · 4 comments
Closed

OpenEphysRawIO unequal signal lengths after clipping #1336

weiglszonja opened this issue Oct 16, 2023 · 4 comments
Labels

Comments

@weiglszonja
Copy link
Contributor

weiglszonja commented Oct 16, 2023

Raised in catalystneuro/tye-lab-to-nwb#40, we have ephys data in OpenEphys legacy format that could not be parsed with OpenEphysRawIO.

The traceback is:

Continuous files do not have aligned timestamps; clipping to make them aligned.
Traceback (most recent call last):
  File "/Applications/PyCharm.app/Contents/plugins/python/helpers/pydev/pydevd.py", line 1500, in _exec
    pydev_imports.execfile(file, globals, locals)  # execute the script
  File "/Applications/PyCharm.app/Contents/plugins/python/helpers/pydev/_pydev_imps/_pydev_execfile.py", line 18, in execfile
    exec(compile(contents+"\n", file, 'exec'), glob, loc)
  File "/Users/weian/Library/Mobile Documents/com~apple~CloudDocs/catalystneuro/tye-lab-to-nwb/src/tye_lab_to_nwb/neurotensin_valence/try_openephys.py", line 8, in <module>
    interface = OpenEphysRecordingInterface(
  File "/Users/weian/anaconda3/envs/neuropixels39/lib/python3.9/site-packages/neuroconv/datainterfaces/ecephys/openephys/openephysdatainterface.py", line 45, in __new__
    return OpenEphysLegacyRecordingInterface(
  File "/Users/weian/anaconda3/envs/neuropixels39/lib/python3.9/site-packages/neuroconv/datainterfaces/ecephys/openephys/openephyslegacydatainterface.py", line 56, in __init__
    available_streams = self.get_stream_names(
  File "/Users/weian/anaconda3/envs/neuropixels39/lib/python3.9/site-packages/neuroconv/datainterfaces/ecephys/openephys/openephyslegacydatainterface.py", line 17, in get_stream_names
    stream_names, _ = OpenEphysLegacyRecordingExtractor.get_streams(
  File "/Users/weian/anaconda3/envs/neuropixels39/lib/python3.9/site-packages/spikeinterface/extractors/neoextractors/neobaseextractor.py", line 71, in get_streams
    neo_reader = cls.get_neo_io_reader(cls.NeoRawIOClass, **neo_kwargs)
  File "/Users/weian/anaconda3/envs/neuropixels39/lib/python3.9/site-packages/spikeinterface/extractors/neoextractors/neobaseextractor.py", line 64, in get_neo_io_reader
    neo_reader.parse_header()
  File "/Users/weian/anaconda3/envs/neuropixels39/lib/python3.9/site-packages/neo/rawio/baserawio.py", line 178, in parse_header
    self._parse_header()
  File "/Users/weian/anaconda3/envs/neuropixels39/lib/python3.9/site-packages/neo/rawio/openephysrawio.py", line 164, in _parse_header
    assert all(all_sigs_length[0] == e for e in all_sigs_length),\
AssertionError: Not all signals have the same length

@samuelgarcia I checked and the timestamps have equal length, its the signals that don't have equal length (they can be shorter and longer by 1024 'header_bytes').
@CodyCBakerPhD suggested a solution here to force equal signal length by clipping.

Environment:

  • OS: macOS
  • Python version: 3.9
  • Neo version: 0.13.0.dev0
  • NumPy version: 1.25.2
@weiglszonja
Copy link
Contributor Author

weiglszonja commented Oct 24, 2023

We were discussing this with @samuelgarcia and this is a bug because of using ignore_timestamps_errors=True which does not guarantee that the data has no gaps.

The ignore_timestamps_errors=True should only be used for loading the data and debug which channels are affected by the gaps, but OpenEphysRawIO does not guarantee that the data is aligned when used with ignore_timestamps_errors=True.

@samuelgarcia
Copy link
Contributor

Yes.
The was when using ignore_timestamps_errors=True, some channels do not have the same gaps (missing block number). And so when cropping the first or last block at the border, the channel finish with diffrent data size.

In short, this option ignore_timestamps_errors is quite a bit dangerous unless are lossing the same block (which is possible).
The actual signal fetcher rely on data block continuity.
The only good solution would be to rewrite this reader with discontinuity in mind. It is a bit tedious and would make the reader even slower. Lets see how large is the user demand on this.
A simpler solution would be to check that if they are gaps per channel, then we ensure that the gaps are the same across channels. (this was the not case for the file that triggerd the bug)

I will try to improve the doc about this option.

@zm711
Copy link
Contributor

zm711 commented Oct 25, 2023

@samuelgarcia just as an fyi and to link to an actual use case.
This has already been used by at least one spikeinterface user SpikeInterface/spikeinterface#2107.

@zm711
Copy link
Contributor

zm711 commented Feb 2, 2024

Fixed by #1387.

We can open a new issue if there is a new problem with the IO.

@zm711 zm711 closed this as completed Feb 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants