CCSDSPy / ccsdspy

I/O interface and utilities for CCSDS binary spacecraft data in Python. Library used in flight missions at NASA, NOAA, and SWRI
https://ccsdspy.org
BSD 3-Clause "New" or "Revised" License
78 stars 19 forks source link

Odd numbers of packets in a file cannot be loaded when bit_length of array item is 12 #78

Closed scandey closed 1 year ago

scandey commented 1 year ago

There is some very strange behavior going on when a Packet Array field has 12 bits and the file being opened has an odd number of packets in it.

Ends up with the following value error. All but the last field seem to be executing, but it gets stuck on the last one. (not sure that the earlier ones are actually correct though...)

Traceback (most recent call last):
  File "/Users/scott/x/x/fake_twelvebit_data.py", line 29, in <module>
    pkt.load('fake_twelve_single.ccsds')
  File "/Users/scott/x/x/.venv/lib/python3.11/site-packages/ccsdspy/packet_types.py", line 169, in load
    packet_arrays = _load(
                    ^^^^^^
  File "/Users/scott/x/x/.venv/lib/python3.11/site-packages/ccsdspy/packet_types.py", line 622, in _load
    field_arrays = _decode_fixed_length(file_bytes, fields)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/scott/x/x/.venv/lib/python3.11/site-packages/ccsdspy/decode.py", line 207, in _decode_fixed_length
    arr[i :: meta.nbytes_final] = file_bytes[
    ~~~^^^^^^^^^^^^^^^^^^^^^^^^
ValueError: could not broadcast input array from shape (0,) into shape (1,)

Unfortunately this is beyond my debugging prowess, I really couldn't follow the logic of that section of code to find the (presumed) off-by-one error. I did generate a quick script to replicate the issue and a similar script with 8 bit values instead of 12 that does not replicate the issue. Both scripts can be run directly or as faux-notebooks in VSCode. (EDIT: Behavior will replicate with either 1.1.1 or main)

Example script for twelve bit array

# %%
import ccsdspy
from ccsdspy import PacketField, PacketArray
import io

# %%
pkt = ccsdspy.FixedLength([
    PacketArray(name='twelve', data_type='int', bit_length=12, array_shape=(8, 1), array_order='C')
])
# %% One packet does not work
fakepkt = io.BytesIO(b"\x00\x01\xC0\x00\x00\x0B\x00\x00\x01\x00\x20\x03\x00\x40\x05\x00\x60\x07")
print(f'Fake twelve single file has {fakepkt.getbuffer().nbytes} bytes')
with open('fake_twelve_single.ccsds', 'wb') as file:
    file.write(fakepkt.getbuffer())

# %% Two packets works fine
fakepkts_even = io.BytesIO(b"\x00\x01\xC0\x01\x00\x0B\x00\x00\x01\x00\x20\x03\x00\x40\x05\x00\x60\x07\x00\x01\xC0\x02\x00\x0B\x00\x00\x01\x00\x20\x03\x00\x40\x05\x00\x60\x07")
print(f'Fake twelve even file has {fakepkts_even.getbuffer().nbytes} bytes')
with open('fake_twelve_even.ccsds', 'wb') as file:
    file.write(fakepkts_even.getbuffer())

# %% Three packets does not work
fakepkts_odd = io.BytesIO(b"\x00\x01\xC0\x01\x00\x0B\x00\x00\x01\x00\x20\x03\x00\x40\x05\x00\x60\x07\x00\x01\xC0\x02\x00\x0B\x00\x00\x01\x00\x20\x03\x00\x40\x05\x00\x60\x07\x00\x01\xC0\x03\x00\x0B\x00\x00\x01\x00\x20\x03\x00\x40\x05\x00\x60\x07")
print(f'Fake twelve odd file has {fakepkts_odd.getbuffer().nbytes} bytes')
with open('fake_twelve_odd.ccsds', 'wb') as file:
    file.write(fakepkts_odd.getbuffer())
# %%
print(f'Attempting to load just one packet from file')
pkt.load('fake_twelve_single.ccsds')
# %%
print(f'Attempting to load two packets from file')
pkt.load('fake_twelve_even.ccsds')
# %%
print(f'Attempting to load three packets from file')
pkt.load('fake_twelve_odd.ccsds')

Equivalent working code for 8 bit array

# %%
import ccsdspy
from ccsdspy import PacketField, PacketArray
import io

# %%
pkt = ccsdspy.FixedLength([
    PacketArray(name='eight', data_type='int', bit_length=8, array_shape=(8, 1), array_order='C')
])
# %% One packet does not work
fakepkt = io.BytesIO(b"\x00\x02\xC0\x00\x00\x07\x00\x01\x02\x03\x04\x05\x06\x07")
print(f'Fake eight single file has {fakepkt.getbuffer().nbytes} bytes')
with open('fake_eight_single.ccsds', 'wb') as file:
    file.write(fakepkt.getbuffer())

# %% Two packets works fine
fakepkts_even = io.BytesIO(b"\x00\x02\xC0\x00\x00\x07\x00\x01\x02\x03\x04\x05\x06\x07\x00\x02\xC0\x01\x00\x07\x00\x01\x02\x03\x04\x05\x06\x07")
print(f'Fake eight even file has {fakepkts_even.getbuffer().nbytes} bytes')
with open('fake_eight_even.ccsds', 'wb') as file:
    file.write(fakepkts_even.getbuffer())

# %% Three packets does not work
fakepkts_odd = io.BytesIO(b"\x00\x02\xC0\x00\x00\x07\x00\x01\x02\x03\x04\x05\x06\x07\x00\x02\xC0\x01\x00\x07\x00\x01\x02\x03\x04\x05\x06\x07\x00\x02\xC0\x02\x00\x07\x00\x01\x02\x03\x04\x05\x06\x07")
print(f'Fake eight odd file has {fakepkts_odd.getbuffer().nbytes} bytes')
with open('fake_eight_odd.ccsds', 'wb') as file:
    file.write(fakepkts_odd.getbuffer())
# %%
print(f'Attempting to load just one packet from file')
pkt.load('fake_eight_single.ccsds')
# %%
print(f'Attempting to load two packets from file')
pkt.load('fake_eight_even.ccsds')
# %%
print(f'Attempting to load three packets from file')
pkt.load('fake_eight_odd.ccsds')
ddasilva commented 1 year ago

Thanks @scandey, I'll take a look and get back to you

ddasilva commented 1 year ago

Hi @scandey,

I appreciate you reporting this. It looks like this is a bug with FixedLength, but not the VariableLength class (the VariableLength uses a separate decoder). When I changed FixedLength to VariableLength in your example code, it runs cleanly.

I imagine you have an private use case that is driving this bug report... would you mind trying your use case with the VariableLength class? If it works, then we will at least know that we have a workaround as to not hold up your development.

ddasilva commented 1 year ago

Hi @scandey,

I also implemented a fix for this bug on this branch, and added a regression test. Could you give that a try as well? https://github.com/CCSDSPy/ccsdspy/tree/gh-issue-78-fix

scandey commented 1 year ago

Thank you! I can load the file with the VariableLength workaround or FixedLength (on the bugfix branch), though the signed integers for either seem to be a little broken. I'm getting a scale of 15x for negative integers... The 12-bit signed integer 0x800 returns -30720. Perhaps a new issue is in order for that though?