uwmadison-chm / bioread

Utilities to work with files from BIOPAC's AcqKnowlege software
MIT License
65 stars 23 forks source link

Channels truncated on Windows #3

Closed dgfitch closed 8 years ago

dgfitch commented 9 years ago

When I convert using acq2mat on Windows, I get different lengths per channel. [It works fine on whatever version of bioread guero is running.]

acq_info .\AudioTest_task0001.acq
File revision: 108
Sample time: 1.0
Compressed: False
Number of channels: 11
EBR:
        Units: microV
        Number of samples: 580231
        Frequency divider: 1
Zygo:
        Units: microV
        Number of samples: 580231
        Frequency divider: 1
<SNIP>
[spoiler alert, they are all 580231]

But in matlab after I run acq2mat:

>> cd /scratch/fitch/startle_delay/converted_on_windows
>> biopac = load('AudioTest_task0001.mat')
>> lengths = arrayfun(@(c) length(biopac.channels{c}.data), 1:11)
580231    580230    580229    580228    580227    580226    580225    580224    580223    580222    580221

The first channel is the correct length. The rest are losing a sample each time. So it looks like bioread might need to get hacked up a bit for this newer format revision, or maybe nobody has been crazy/silly enough to run it on Windows?

Bleh! Not a big deal because I can just convert on Unix, but thought I should file a bug so it's visible out there in interwebspace. If you think it's important, I can try to fix this - let me know.

njvack commented 9 years ago

Hm. I don't know why this should affect windows differently than unix. I think the whole read method actually wants a rewrite.

Anyhow, I'll look into this...

On Thursday, January 29, 2015, Dan Fitch notifications@github.com wrote:

When I convert using acq2mat on Windows, I get different lengths per channel. [It works fine on whatever version of bioread guero is running.]

acq_info .\AudioTest_task0001.acq File revision: 108 Sample time: 1.0 Compressed: False Number of channels: 11 EBR: Units: microV Number of samples: 580231 Frequency divider: 1 Zygo: Units: microV Number of samples: 580231 Frequency divider: 1

[spoiler alert, they are all 580231] But in matlab after I run acq2mat: > > cd /scratch/fitch/startle_delay/converted_on_windows > > biopac = load('AudioTest_task0001.mat') > > lengths = arrayfun(@(c) length(biopac.channels{c}.data), 1:11) > > 580231 580230 580229 580228 580227 580226 580225 580224 580223 580222 580221 The first channel is the correct length. The rest are losing a sample each time. So it looks like bioread might need to get hacked up a bit for this newer format revision, or maybe nobody has been crazy/silly enough to run it on Windows? Bleh! Not a big deal because I can just convert on Unix, but thought I should file a bug so it's visible out there in interwebspace. If you think it's important, I can try to fix this - let me know. — Reply to this email directly or view it on GitHub https://github.com/njvack/bioread/issues/3.
njvack commented 9 years ago

Huh. This is very odd, actually -- the length of the channel is determined in the header of the file; we aren't doing any crazy tricks to figure it out. And certainly nothing that should be varying from platform to platform.

You can do this on a Windows box to test:

import bioread
f = open("AudioTest_task0001.acq", "r")
r = bioread.AcqReader(f)
r.read()
counts = [ch.point_count for ch in r.channel_headers]

... that'll at least confirm that reading headers is working right in Windows.

AcqReader.__read_data_uncompressed() still needs to die in a fire.

dgfitch commented 9 years ago

I'll go back now and see if I can duplicate the problem on windows; more likely it was just me being silly.

On Wed, Apr 1, 2015 at 2:04 PM, Nate Vack notifications@github.com wrote:

Huh. This is very odd, actually -- the length of the channel is determined in the header of the file; we aren't doing any crazy tricks to figure it out. And certainly nothing that should be varying from platform to platform.

You can do this on a Windows box to test:

import bioread f = open("AudioTest_task0001.acq", "r") r = bioread.AcqReader(f) r.read() counts = [ch.point_count for ch in r.channel_headers]

... that'll at least confirm that reading headers is working right in Windows.

AcqReader.__read_data_uncompressed() still needs to die in a fire.

— Reply to this email directly or view it on GitHub https://github.com/njvack/bioread/issues/3#issuecomment-88596613.

njvack commented 9 years ago

If you were using a Matlab-based converter, that might have a whole other raft of problems...

On Wed, Apr 1, 2015 at 2:06 PM Dan Fitch notifications@github.com wrote:

I'll go back now and see if I can duplicate the problem on windows; more likely it was just me being silly.

On Wed, Apr 1, 2015 at 2:04 PM, Nate Vack notifications@github.com wrote:

Huh. This is very odd, actually -- the length of the channel is determined in the header of the file; we aren't doing any crazy tricks to figure it out. And certainly nothing that should be varying from platform to platform.

You can do this on a Windows box to test:

import bioread f = open("AudioTest_task0001.acq", "r") r = bioread.AcqReader(f) r.read() counts = [ch.point_count for ch in r.channel_headers]

... that'll at least confirm that reading headers is working right in Windows.

AcqReader.__read_data_uncompressed() still needs to die in a fire.

— Reply to this email directly or view it on GitHub https://github.com/njvack/bioread/issues/3#issuecomment-88596613.

— Reply to this email directly or view it on GitHub https://github.com/njvack/bioread/issues/3#issuecomment-88596920.

dgfitch commented 9 years ago

OK, time for an actually helpful bug report.

Result from your sample code on unix:

[580231, 580231, 580231, 580231, 580231, 580231, 580231, 580231, 580231, 580231, 580231]

Result from your sample code on windows:

Traceback (most recent call last):
  File "test.py", line 16, in <module>
    r.read()
  File "build\bdist.win32\egg\bioread\readers.py", line 73, in read
  File "build\bdist.win32\egg\bioread\readers.py", line 158, in _read_data
  File "build\bdist.win32\egg\bioread\readers.py", line 206, in __read_data_uncompressed
  File "build\bdist.win32\egg\bioread\readers.py", line 224, in __copy_uncompressed_data
IndexError: index 154 is out of bounds for axis 1 with size 137

But oddly, if I do this:

r = bioread.read_file(thing)

print([ch.point_count for ch in r.channel_headers])
print([ch.point_count for ch in r.channels])

I get the expected stable values twice on Unix, but I get this on Windows:

[580231, 580231, 580231, 580231, 580231, 580231, 580231, 580231, 580231, 580231, 580231]
[580231, 580230, 580229, 580228, 580227, 580226, 580225, 580224, 580223, 580222, 580221]

I will investigate!

njvack commented 8 years ago

FWIW this is borked in Unix, too. Things may have worked properly in a previous Python.

Regardless, the fix will be a total rewrite of Reader.__read_uncompressed(), which is in progress.

njvack commented 8 years ago

Fixed in bb733d34a2ff482876fb8bb1b39e135f0014bce3 -- this will make it into the soon-upcoming 1.0 release.