search

NeuralEnsemble / python-neo

Neo is a package for representing electrophysiology data in Python, together with support for reading a wide range of neurophysiology file formats
http://neo.readthedocs.io/en/latest/
BSD 3-Clause "New" or "Revised" License
325 stars 248 forks source link

uint32 too small for opening raw ns5 #1528

Closed shaneallcroft-bg closed 2 months ago

shaneallcroft-bg commented 3 months ago

Hi, i haven't been so assiduous in combing documentation for relevant config stuff, so take with a grain of salt etc; def possible that this one is on me, but currently when opening a .ns5 file (NSx 3.0 file standard BR neurotech) a uint32 variable is used to represent file size when it probably should be a uint64(?) -- the ns5 in question is ~10 min recording which seems not unreasonably large. Let me know if there's any additional details I can provide. Thanks!

$ python read_ns5.py --ns5 ~/data/datafile003.ns5 

reading in /home/six/data/datafile003.ns5
Traceback (most recent call last):
  File "/home/six/today/read_ns5.py", line 28, in <module>
    main()
  File "/home/six/today/read_ns5.py", line 18, in main
    ns5_data_reader.parse_header()
  File "/usr/lib/python3.12/site-packages/neo-0.14.0.dev0-py3.12.egg/neo/rawio/baserawio.py", line 189, in parse_header
    self._parse_header()
  File "/usr/lib/python3.12/site-packages/neo-0.14.0.dev0-py3.12.egg/neo/rawio/blackrockrawio.py", line 330, in _parse_header
    self.__nsx_data_header[nsx_nb] = nsx_dataheader_reader(nsx_nb)
                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/site-packages/neo-0.14.0.dev0-py3.12.egg/neo/rawio/blackrockrawio.py", line 997, in __read_nsx_dataheader_variant_c
    npackets = int((filesize - offset) / np.dtype(ptp_dt).itemsize)
                    ~~~~~~~~~^~~~~~~~
OverflowError: Python integer 9437464460 out of bounds for uint32
zm711 commented 3 months ago

This seems like a fair change to make. I would need to read into the code a bit to see what this is. I know @cboulay has helped us with blackrock before. If they don't have time, I'll look at this soon.

zm711 commented 3 months ago

@h-mayorquin, I feel like you're an expert of overflow. Any chance just glancing at this you see what the likely problem is. I'm not super strong with all the name mangling that the RawIO uses. I'm trying to see what is actually flowing. filesize should be an int, but maybe I'm missing something.

h-mayorquin commented 3 months ago

That's a strange reputation to have. I think it is the header offset in bytes which is actually read as a struct. I linked a fix.

zm711 commented 3 months ago

Haha. I have never met someone who has had so much success removing instances of overflow.

thanks! @shaneallcroft-bg do you want to test the PR and see if that has fixed it.

shaneallcroft-bg commented 3 months ago

Whoa! Super fast turnaround, nice -- will test that PR tonight and get back to you all. Thank you!

shaneallcroft-bg commented 3 months ago

:+1: Yes confirmed fixed with https://github.com/NeuralEnsemble/python-neo/pull/1540! Thanks all!

zm711 commented 2 months ago

Fixed by #1540.