-
Notifications
You must be signed in to change notification settings - Fork 16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use byte_count to determine size of CoDICE data arrays #1171
Use byte_count to determine size of CoDICE data arrays #1171
Conversation
…sing byte_count to determine length of data array
) | ||
decompressed_values = [] | ||
for packet_data, byte_count in zip( | ||
science_values, self.dataset.byte_count.data |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was looking at what this byte_count
data does/mean and I saw this description from the XTCE
NUMBER OF BYTES IN THE DATA ARRAY. IF COMPRESSED, THIS VALUE REPRESENTS THE LENGTH OF THE COMPRESSED DATA.
Looks like this byte_count
is used by many CoDICE Hi and Lo packets. It says 'if compressed'. Then I looked at Lo and Hi's compression id lookup dictionary. It seems like all id maps to some compression form. Will there be a case when the data will not be compressed?
Also, I just noticed that in this line, you were using parameters from first packet for remaining all science packets. I think that may be remains from your previous implementation. Will that need to change now since packet_data
from science_values
may be using different view_id, plan_id, and so on.
Sorry for long comments. These comments are not for this PR but something I wanted to point out in case we missed it before.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will there be a case when the data will not be compressed?
I haven't come across any data product yet that is expected to have no compression (algorithm=0) (see screenshot from the SCI_LUT spreadsheet). I think this is one of those knobs that CoDICE wants to be able to turn to theoretically, but in practice I am not sure it would ever happen. I will ask Joey about this though.
Also, I just noticed that in this line, you were using parameters from first packet for remaining all science packets. I think that may be remains from your previous implementation. Will that need to change now since packet_data from science_values may be using different view_id, plan_id, and so on.
I am a little confused here. Were you referring to this part of the code?
table_id = int(dataset.table_id.data[0]) |
If it is the former, I think it is safe to use the first packet because we expect each packet for a particular APID to have the same view_id, plan_step, etc., but I will also double check that with Joey.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool. Since this is outside of this PR scope, I will approve it. We can address this in future PR if needed.
This PR fixes a bug in how the CoDICE data arrays were being processed. Some packet data contains a buffer at the end of the data array that needed to be stripped out before the decompression algorithm is called. The pipeline now properly uses the
byte_count
field to determine the which bytes are included in the data arrays. With this fix, more packets can now be processed, which changed some of the unit test values.