Skip to content
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

MNT: space_packet_parser update #1007

Merged
merged 31 commits into from
Oct 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
51c9d82
update space_packet_parser
tech3371 Oct 8, 2024
3fceeba
tons of updates
tech3371 Oct 8, 2024
8dd16f9
utils function
tech3371 Oct 8, 2024
9e4ca1c
update to new release
tech3371 Oct 9, 2024
5085029
fixes for dtype issue with binary data type and other fixes
tech3371 Oct 9, 2024
b43f2b3
update to glows
tech3371 Oct 9, 2024
c3e9c4d
IDEX minor changes
tech3371 Oct 9, 2024
bd3dcc9
hit and lo fixes
tech3371 Oct 9, 2024
f11f4e7
hi updates
tech3371 Oct 9, 2024
e7cda0c
ultra fixes
tech3371 Oct 9, 2024
a3a5676
codice fixes
tech3371 Oct 9, 2024
8a7c9c7
idex fixes
tech3371 Oct 9, 2024
6ded7f3
fixing glows and mag
maxinelasp Oct 9, 2024
f9115f7
skipping Lo's tests
tech3371 Oct 9, 2024
47742f2
undid spell checks
tech3371 Oct 9, 2024
40cefa5
ultra fixes
tech3371 Oct 9, 2024
5509ad1
rebase fixes
tech3371 Oct 11, 2024
b2f5e8c
more library updates
tech3371 Oct 11, 2024
d2715af
feedback changes
tech3371 Oct 11, 2024
9be6e9a
ultra fixes
tech3371 Oct 11, 2024
65ddad6
removed `.user_data` where appropriate.
tech3371 Oct 11, 2024
a598d72
GLOWS test fixes
tech3371 Oct 11, 2024
666e9b4
CoDICE type fix
tech3371 Oct 11, 2024
3b83380
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 11, 2024
137e506
fixing CoDICE pre-commit test
tech3371 Oct 11, 2024
1066ee3
Merge branch 'spp_update' of github.com:tech3371/imap_processing into…
tech3371 Oct 11, 2024
2082ea5
refactor codes that converts bytes into a function in utils.py
tech3371 Oct 14, 2024
3cb1995
undo CoDICE change to fix mypy error
tech3371 Oct 14, 2024
a20bcab
pre-commit fix
tech3371 Oct 15, 2024
43f6980
corrected convert_to_binary_string function
tech3371 Oct 15, 2024
4894653
final walk through updates
tech3371 Oct 15, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions imap_processing/ccsds/ccsds_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,7 @@ class CcsdsData:
def __init__(self, packet_header: dict):
attributes = [field.name for field in fields(self)]

for key, item in packet_header.items():
value = (
item.derived_value if item.derived_value is not None else item.raw_value
)
for key, value in packet_header.items():
if key in attributes:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we still need to get value.raw_value here... won't value be of type packets.IntParameter instead of int?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understood the original if statement, it seems like we want to get derived value if it's available or else get raw value. Now with new changes, we get derived value if it's available or else we get raw value. so getting the value will do what the if statement was doing before, if I understood correctly. That's how I understood at least when Gavin explain in the Slack thread. should we double check to be safe?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you could just print out the type and validate that it's a python type, that's good enough for me

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

packets.IntParameter is a subclass of int

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in this case, are we good with current change?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, if it's a different type, that's going to mess with some code, right? Some of my code depends on the types being accurate because of MyPy

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess, if you make the change and all the tests pass, then it's probably fine... but I'm just worried about changing all the types out from underneath everyone like this, even if they're basically the same as the original types. But perhaps that is just Python offending my delicate, type-loving sensibilities again

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, then I think you can change the types to IntParameter now. But note that raw_value is not the same as derived_value which was being used before if it was present.

setattr(self, key, value)
else:
Expand Down
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@laspsandoval After reverting some of " changes, I came across this error similar to what we saw couple days ago.

E                   ValueError: Unable to convert data to CDF format, data object cannot be of type string.

I did make changes to this file again since it seems like you did want your data in derived value. Please do revisit this in future PR if that's not what you want. It seems to involve more changes and I am worried I will mess something.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now I understand why it failed last time. It needed support_data type instead of metadata type since that's what it was. That's why I made that change in this PR.

Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,11 @@ string_base_attrs:
CATDESC: string metadata
FIELDNAM: string_metadata
FORMAT: A80
VAR_TYPE: metadata
LABLAXIS: "none"
VAR_TYPE: support_data
DISPLAY_TYPE: no_plot
DEPEND_0: epoch
UNITS: " "

packet_data_attrs:
CATDESC: packet data
Expand Down
2 changes: 2 additions & 0 deletions imap_processing/codice/codice_l1a.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
from imap_processing.codice.codice_l0 import decom_packets
from imap_processing.codice.decompress import decompress
from imap_processing.codice.utils import CODICEAPID
from imap_processing.utils import convert_to_binary_string

logger = logging.getLogger(__name__)
logger.setLevel(logging.INFO)
Expand Down Expand Up @@ -541,6 +542,7 @@ def process_codice_l1a(file_path: Path, data_version: str) -> xr.Dataset:
elif apid in constants.APIDS_FOR_SCIENCE_PROCESSING:
# Extract the data
science_values = packet_dataset.data.data[0]
science_values = convert_to_binary_string(science_values)

# Get the four "main" parameters for processing
table_id, plan_id, plan_step, view_id = get_params(packet_dataset)
Expand Down
38 changes: 0 additions & 38 deletions imap_processing/codice/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@

from enum import IntEnum

import space_packet_parser


class CODICEAPID(IntEnum):
"""Create ENUM for CoDICE APIDs."""
Expand Down Expand Up @@ -57,39 +55,3 @@ class CoDICECompression(IntEnum):
LOSSLESS = 3
LOSSY_A_LOSSLESS = 4
LOSSY_B_LOSSLESS = 5


def add_metadata_to_array(packet: space_packet_parser, metadata_arrays: dict) -> dict:
"""
Add metadata to the metadata_arrays.

Parameters
----------
packet : space_packet_parser.parser.Packet
CODICE data packet.
metadata_arrays : dict
Metadata arrays.

Returns
-------
metadata_arrays : dict
Updated metadata arrays with values.
"""
ignore_list = [
"SPARE_1",
"SPARE_2",
"SPARE_3",
"SPARE_4",
"SPARE_5",
"SPARE_6",
"CHECKSUM",
]

for key, value in packet.header.items():
metadata_arrays.setdefault(key, []).append(value.raw_value)

for key, value in packet.data.items():
if key not in ignore_list:
metadata_arrays.setdefault(key, []).append(value.raw_value)

return metadata_arrays
7 changes: 3 additions & 4 deletions imap_processing/decom.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from pathlib import Path
from typing import Union

from space_packet_parser import parser, xtcedef
from space_packet_parser import definitions


def decom_packets(
Expand All @@ -32,9 +32,8 @@ def decom_packets(
list
List of all the unpacked data.
"""
packet_definition = xtcedef.XtcePacketDefinition(xtce_packet_definition)
packet_parser = parser.PacketParser(packet_definition)
packet_definition = definitions.XtcePacketDefinition(xtce_packet_definition)

with open(packet_file, "rb") as binary_data:
packet_generator = packet_parser.generator(binary_data)
packet_generator = packet_definition.packet_generator(binary_data)
return list(packet_generator)
23 changes: 6 additions & 17 deletions imap_processing/glows/l0/decom_glows.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from enum import Enum
from pathlib import Path

from space_packet_parser import parser, xtcedef
from space_packet_parser import definitions

from imap_processing import imap_module_directory
from imap_processing.ccsds.ccsds_data import CcsdsData
Expand Down Expand Up @@ -49,39 +49,28 @@ def decom_packets(
f"{imap_module_directory}/glows/packet_definitions/GLX_COMBINED.xml"
)

packet_definition = xtcedef.XtcePacketDefinition(xtce_document)
glows_parser = parser.PacketParser(packet_definition)
packet_definition = definitions.XtcePacketDefinition(xtce_document)

histdata = []
dedata = []

filename = packet_file_path.name

with open(packet_file_path, "rb") as binary_data:
glows_packets = glows_parser.generator(binary_data)
glows_packets = packet_definition.packet_generator(binary_data)

for packet in glows_packets:
apid = packet.header["PKT_APID"].derived_value
apid = packet["PKT_APID"]
# Do something with the packet data
if apid == GlowsParams.HIST_APID.value:
values = [
item.derived_value
if item.derived_value is not None
else item.raw_value
for item in packet.data.values()
]
values = [item.raw_value for item in packet.user_data.values()]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before this was getting the derived value if it is not None. The if item.derived_value is not None else item.raw_value is taken care of for you by space_packet_parser now.

I'm pretty sure this is the equivalent to before now:

Suggested change
values = [item.raw_value for item in packet.user_data.values()]
values = list(packet.user_data.values())

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@maxinelasp what do you think here? I thought same as Greg but you helped me debug some of GLOWS' error and I saw that you had to get raw_value. Is that right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, when I printed out the types here it was all IntParameter (etc) instead of int.

Copy link
Collaborator

@greglucas greglucas Oct 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

class IntParameter(int):

IntParameter is a subclass of int so if you type it as int IntParameter should be acceptable as well? Alternatively, could you type it as IntParameter where this is needed?

item.raw_value is very different from item.

item.derived_value from v4.x is the same as item in v5.x

hist_l0 = HistogramL0(
__version__, filename, CcsdsData(packet.header), *values
)
histdata.append(hist_l0)

if apid == GlowsParams.DE_APID.value:
values = [
item.derived_value
if item.derived_value is not None
else item.raw_value
for item in packet.data.values()
]
values = [item.raw_value for item in packet.user_data.values()]

de_l0 = DirectEventL0(
__version__, filename, CcsdsData(packet.header), *values
Expand Down
12 changes: 1 addition & 11 deletions imap_processing/glows/l0/glows_l0_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,17 +114,7 @@ class HistogramL0(GlowsL0):
ELAVG: int
ELVAR: int
EVENTS: int
HISTOGRAM_DATA: bytearray

def __post_init__(self) -> None:
"""Convert HISTOGRAM_DATA attribute from string to bytearray if needed."""
if isinstance(self.HISTOGRAM_DATA, str):
# Convert string output from space_packet_parser to bytearray
self.HISTOGRAM_DATA = bytearray(
int(self.HISTOGRAM_DATA, 2).to_bytes(
len(self.HISTOGRAM_DATA) // 8, "big"
)
)
HISTOGRAM_DATA: bytes


@dataclass
Expand Down
6 changes: 4 additions & 2 deletions imap_processing/hi/l1a/histogram.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import xarray as xr

from imap_processing.cdf.imap_cdf_manager import ImapCdfAttributes
from imap_processing.utils import convert_to_binary_string

# define the names of the 24 counter arrays
# contained in the histogram packet
Expand Down Expand Up @@ -59,10 +60,11 @@ def create_dataset(input_ds: xr.Dataset) -> xr.Dataset:
# TODO: Look into avoiding the for-loops below
# It seems like we could try to reshape the arrays and do some numpy
# broadcasting rather than for-loops directly here
for i_epoch, counters_binary_data in enumerate(input_ds["counters"].data):
for i_epoch, counters_bytes_data in enumerate(input_ds["counters"].data):
binary_str_val = convert_to_binary_string(counters_bytes_data)
# unpack 24 arrays of 90 12-bit unsigned integers
counter_ints = [
int(counters_binary_data[i * 12 : (i + 1) * 12], 2) for i in range(90 * 24)
int(binary_str_val[i * 12 : (i + 1) * 12], 2) for i in range(90 * 24)
]
# populate the dataset with the unpacked integers
for i_counter, counter in enumerate(
Expand Down
4 changes: 3 additions & 1 deletion imap_processing/hi/l1a/science_direct_event.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

from imap_processing.cdf.imap_cdf_manager import ImapCdfAttributes
from imap_processing.spice.time import met_to_j2000ns
from imap_processing.utils import convert_to_binary_string

# TODO: read LOOKED_UP_DURATION_OF_TICK from
# instrument status summary later. This value
Expand Down Expand Up @@ -328,8 +329,9 @@ def science_direct_event(packets_data: xr.Dataset) -> xr.Dataset:
# end of the list. This way, I don't need to flatten
# the list later.
for i, data in enumerate(packets_data["de_tof"].data):
binary_str_val = convert_to_binary_string(data)
# break binary stream data into unit of 48-bits
event_48bits_list = break_into_bits_size(data)
event_48bits_list = break_into_bits_size(binary_str_val)
# parse 48-bits into meaningful data such as metaevent or direct event
de_data_list.extend([parse_direct_event(event) for event in event_48bits_list])
# add packet time to packet_met_time
Expand Down
2 changes: 1 addition & 1 deletion imap_processing/hit/l0/data_classes/housekeeping.py
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ class Housekeeping(HITBase):

def __init__(
self,
packet: space_packet_parser.parser.Packet,
packet: space_packet_parser.packets.CCSDSPacket,
software_version: str,
packet_file_name: str,
):
Expand Down
6 changes: 5 additions & 1 deletion imap_processing/hit/l0/decom_hit.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
import numpy as np
import xarray as xr

from imap_processing.utils import convert_to_binary_string

# TODO: Consider moving global values into a config file

# Structure to hold binary details for a
Expand Down Expand Up @@ -357,7 +359,9 @@ def assemble_science_frames(sci_dataset: xr.Dataset) -> xr.Dataset:
# Convert sequence flags and counters to NumPy arrays for vectorized operations
seq_flgs = sci_dataset.seq_flgs.values
seq_ctrs = sci_dataset.src_seq_ctr.values
science_data = sci_dataset.science_data.values
science_data = [
convert_to_binary_string(data) for data in sci_dataset.science_data.values
]
epoch_data = sci_dataset.epoch.values

# Number of packets in the file
Expand Down
6 changes: 3 additions & 3 deletions imap_processing/hit/l0/utils/hit_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,19 +31,19 @@ class HITBase:
packet_file_name: str
ccsds_header: CcsdsData

def parse_data(self, packet: space_packet_parser.parser.Packet) -> None:
def parse_data(self, packet: space_packet_parser.packets.CCSDSPacket) -> None:
"""
Parse Lo L0 packet data.

Parameters
----------
packet : space_packet_parser.parser.Packet
packet : space_packet_parser.packets.CCSDSPacket
A single Lo L0 packet from space packet parser.
"""
attributes = [field.name for field in fields(self)]

# For each item in packet, assign it to the matching attribute in the class.
for key, item in packet.data.items():
for key, item in packet.user_data.items():
value = (
item.derived_value if item.derived_value is not None else item.raw_value
)
Expand Down
Loading
Loading