SigprocFile unpacking problems for small nchans and nbits<8
Test case for nchans=1, nbits=4 in sigproc.py results in frames being doubled in size, and the data duplicated. This is because the unpacking has no method for when a single byte of memory is split across two frames.
Been thinking about this. It's a symptom of something that will be a problem for the whole library, which is that specifying array strides in units of bytes means things break when a datatype has nbits < 8.
I think the best option may be to specify strides throughout the library in units of bits instead of bytes. This loses direct compatibility with Numpy (which uses byte strides), but does provide complete generality. An alternative would be to use units of elements (which is what HDF5 does), but this causes problems when using things like nbit=12 with padded arrays.
I'll try to assess how much effort it will be to switch to bit strides in the existing code.
This looks to me like a "there be dragons" situation. Could there be something like stride_in_bits=True flag or something, to maintain numpy compatibility for the most part?
- Danny
On Mon, Oct 24, 2016 at 11:36 AM Ben Barsdell [email protected] wrote:
Been thinking about this. It's a symptom of something that will be a problem for the whole library, which is that specifying array strides in units of bytes means things break when a datatype has nbits < 8.
I think the best option may be to specify strides throughout the library in units of bits instead of bytes. This loses direct compatibility with Numpy (which uses byte strides), but does provide complete generality. An alternative would be to use units of elements (which is what HDF5 does), but this causes problems when using things like nbit=12 with padded arrays.
I'll try to assess how much effort it will be to switch to bit strides in the existing code.
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/ledatelescope/bifrost/issues/1#issuecomment-255827019, or mute the thread https://github.com/notifications/unsubscribe-auth/AAriI6c-4mGOteD30mnfxcAVLKDIMnbWks5q3Pq1gaJpZM4Iy_HH .
I considered having a flag like that; it could help, but it may also make things complicated.
If we use nbit strides at all, we will have to handle conversion to numpy arrays specially anyway. The real problem is that we will have to change the ring buffer implementation to support nbit strides, as well as any function that can accept dtypes < 8 bits. Mixing bit and byte strides via a flag may actually make things more complicated, versus always using bits inside the library and then converting to bytes only at the last minute when we need to convert to numpy arrays.
I'm actually not keen to make the change to bits, and I think I'll probably put it off for a while, but if we want to support < 8bit data types I think it will be a necessary evil. It's unfortunate that radio astronomers are the only people who want to use <8bit data these days!
On 25 October 2016 at 11:14, Danny Price [email protected] wrote:
This looks to me like a "there be dragons" situation. Could there be something like stride_in_bits=True flag or something, to maintain numpy compatibility for the most part?
- Danny
On Mon, Oct 24, 2016 at 11:36 AM Ben Barsdell [email protected] wrote:
Been thinking about this. It's a symptom of something that will be a problem for the whole library, which is that specifying array strides in units of bytes means things break when a datatype has nbits < 8.
I think the best option may be to specify strides throughout the library in units of bits instead of bytes. This loses direct compatibility with Numpy (which uses byte strides), but does provide complete generality. An alternative would be to use units of elements (which is what HDF5 does), but this causes problems when using things like nbit=12 with padded arrays.
I'll try to assess how much effort it will be to switch to bit strides in the existing code.
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <https://github.com/ledatelescope/bifrost/issues/ 1#issuecomment-255827019>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAriI6c- 4mGOteD30mnfxcAVLKDIMnbWks5q3Pq1gaJpZM4Iy_HH> .
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/ledatelescope/bifrost/issues/1#issuecomment-256116740, or mute the thread https://github.com/notifications/unsubscribe-auth/ADy3WBhSdkmVRkJo9WIt8BpZ0J3evPsXks5q3kcNgaJpZM4Iy_HH .