Back to the main page.
Bug 2409 - change of syntax in bitcmp
| Status | CLOSED FIXED |
| Reported | 2013-12-03 18:39:00 +0100 |
| Modified | 2015-02-11 10:40:12 +0100 |
| Product: | FieldTrip |
| Component: | fileio |
| Version: | unspecified |
| Hardware: | All |
| Operating System: | All |
| Importance: | P3 normal |
| Assigned to: | Robert Oostenveld |
| URL: | |
| Tags: | |
| Depends on: | 2415 |
| Blocks: | |
| See also: |
Guillaume - 2013-12-03 18:39:01 +0100
A warning is displayed with recent MATLAB versions when reading Biosemi data about a change of syntax in bitcmp: Warning: BITCMP(A,N) will not accept integer valued input N in a future release. Use BITCMP(A,ASSUMEDTYPE) instead. > In ft_read_event at 299 > You still can use the bitcmp function on integers of arbitrary bit length. For > example, bitcmp(x,N) and bitand(bitcmp(x),2^N-1) yield the same result.
Robert Oostenveld - 2013-12-05 14:45:18 +0100
matlab2012b on my desktop does not give the warning, but does state in the help: C = bitcmp(A,N) will not accept N in a future release. Use bitcmp(A,ASSUMEDTYPE) instead. matlab2013a also does not give a warning. We don't have 2013b anywhere yet (I will ask for it), I presume that is what you are using. On my OS X desktop I cannot install 2013a or later, as that requires OSX 10.7 or newer and I have 10.6.8.
Guillaume - 2013-12-05 14:49:49 +0100
(In reply to comment #1) Yes, sorry for being imprecise. The warning appears with R2013b.
Robert Oostenveld - 2013-12-05 16:29:40 +0100
I have now spent >30 minutes looking at the bit handling, and don't understand what is going on. The 24 bits in the file are converted to double precision in read_24bit.c by this b1 = (0x000000FF & ((int32_t)buf[indx ])); b2 = (0x000000FF & ((int32_t)buf[indx+1])); b3 = (0x000000FF & ((int32_t)buf[indx+2])); dat_p[count] = ((int32_t) ((b3 << 24) | (b2 << 16) | (b1 << 8)))/256; this first shifts them all the way to the left, and then 8 bits to the right again. The consequence of bit 24 being on (which signifies the sign bit, although that normally would be bit 32) is that all bits from 32 up to 25 turn on. fid = fopen('test.bin', 'wb') fwrite(fid, uint8(0)) fwrite(fid, uint8(255)) fclose(fid) bitget(int32(read_24bit('test.bin', 0, 1)), 32:-1:1) ans = Columns 1 through 8 1 1 1 1 1 1 1 1 Columns 9 through 16 1 1 1 1 1 1 1 1 Columns 17 through 24 0 0 0 0 0 0 0 0 Columns 25 through 32 0 0 0 0 0 0 0 0 Somehow this is compounded with the little endianness. If I comment out the section in ft_read_event around line 300 % find indices of negative numbers bit24i = find(sdata < 0); % make number positive and preserve bits 0-22 (counting from 0) sdata(bit24i) = bitcmp(abs(sdata(bit24i))-1,24); % re-insert the sign bit on its original location, i.e. bit 23 (counting from 0) sdata(bit24i) = sdata(bit24i)+(2^(24-1)); % typecast the data to ensure that the status channel is represented in 32 bits sdata = uint32(sdata); and replace it by % convert to integer representation and only preserve the lowest 24 bits sdata = bitand(int32(sdata), 2^24-1); then it all just seems to work fine.
Robert Oostenveld - 2013-12-05 16:49:25 +0100
I tested the new code sdata = bitand(int32(sdata), 2^24-1); on all 7 test BDF datasets that I happen to have, and the old and new give exactly the same result. So I'll switch to that. mac001> svn commit Sending fileio/ft_read_event.m Transmitting file data . Committed revision 8976.
Guillaume - 2014-06-19 15:57:55 +0200
This is now an error in R2014b: >> Error using bitcmp BITCMP(A,N) does not accept integer valued input N. Use BITCMP(A,ASSUMEDTYPE) instead. Error in ft_read_event (line 308) sdata(bit24i) = bitcmp(abs(sdata(bit24i))-1,24);
Vladimir Litvak - 2014-09-16 13:15:29 +0200
Hi Robert, Could you please finish fixing this bug? It affects our test dataset and our regression testing fails because of it so it'd be nice to get it fixed before the long awaited release.
Robert Oostenveld - 2014-09-16 14:24:14 +0200
The code to address this was already there, there was just an if true % old code else % new code end around it. I removed the old code altogether and made a test script that comprises two bdf files that I have. There is no warning any more in MATLAB 2014a. mac011> svn commit fileio/ft_read_event.m test/test_bug2415.m Sending fileio/ft_read_event.m Adding test/test_bug2415.m Transmitting file data .. Committed revision 9806.
Robert Oostenveld - 2014-09-16 14:25:53 +0200
(In reply to Robert Oostenveld from comment #7) the test script also runs fine under matlab2012b
Guillaume - 2014-09-19 13:22:42 +0200
The new version of the code works fine with MATLAB R2012b onwards but now returns an error with R2012a an earlier versions.
Vladimir Litvak - 2014-09-22 14:47:38 +0200
(In reply to Guillaume from comment #9) I checked in Guillaume's fix using try...catch. Please check and improve if you want.
Robert Oostenveld - 2014-09-22 17:02:43 +0200
(In reply to Vladimir Litvak from comment #10) thanks. I changed try-catch to matlabversion. roboos@mentat001> svn commit fileio/ Sending fileio/ft_read_event.m Adding fileio/private/matlabversion.m Transmitting file data . Committed revision 9824.