HIT housekeeping data class #342

sdhoyt · 2024-02-15T18:14:58Z

Change Summary

Overview

Added a housekeeping data class for decommutation to L1A.

New Dependencies

None

New Files

housekeeping.py
- new data class for HIT housekeeping L1A data
hit_base.py
- base data class for all of the HIT L1A data classes.

Deleted Files

None

Updated Files

hit.rst
- added new files to docs
Updated the XTCE files based on new Telem definitions

Testing

Added test_houskeeping.py to unit test the housekeeping data. This unit test includes validation for CCSDS decommutation.

greglucas · 2024-02-15T18:35:01Z

imap_processing/hit/l0/data_classes/housekeeping.py

+        leak_list = list()
+        leak_bits = bitstring.Bits(bin=self.LEAK_I_RAW)
+        # Each Leak field is 10 bits long
+        leak_bit_length = 10
+        # There are 64 leak fields
+        num_leak_fields = 64
+        # The leak fields appear in the packet in ascending order, so to append
+        # the leak fields in the correct order, the binary will be parsed 
+        # from right to left.
+        for leak_idx in range(leak_bit_length * num_leak_fields, 0, -leak_bit_length):
+            leak_list.append(leak_bits[leak_idx - leak_bit_length:leak_idx].uint)
+        self.LEAK_I = np.array(leak_list)


You can initialize the shape of the array right away and then fill in the values and avoid bitstring as well I think.

Suggested change

leak_list = list()

leak_bits = bitstring.Bits(bin=self.LEAK_I_RAW)

# Each Leak field is 10 bits long

leak_bit_length = 10

# There are 64 leak fields

num_leak_fields = 64

# The leak fields appear in the packet in ascending order, so to append

# the leak fields in the correct order, the binary will be parsed

# from right to left.

for leak_idx in range(leak_bit_length * num_leak_fields, 0, -leak_bit_length):

leak_list.append(leak_bits[leak_idx - leak_bit_length:leak_idx].uint)

self.LEAK_I = np.array(leak_list)

# Each Leak field is 10 bits long

leak_bit_length = 10

# There are 64 leak fields

num_leak_fields = 64

self.LEAK_I = np.empty(num_leak_fields, dtype=np.uint16)

# The leak fields appear in the packet in ascending order, so to append

# the leak fields in the correct order, the binary will be parsed

# from right to left.

for i, leak_idx in enumerate(range(leak_bit_length * num_leak_fields, 0, -leak_bit_length)):

self.LEAK_I[i] = int(self.LEAK_I_RAW[leak_idx - leak_bit_length:leak_idx], 2)

# or something like this to iterate through and then just fill backwards:

# self.LEAK_I[-i] = int(self.LEAK_I_RAW[i * leak_bit_length:(i + 1) * leak_bit_length], 2)

Also, are the bits supposed to be read in reverse as well (little-endian vs big-endian matter). Or are they just stored in reverse?

They're just stored in reverse. There's 64 leak fields in the packet and they're ordered as leak_64, leak_63, .... so I just need to reverse the field order when I make the array

Are we trying to move away from the bitstring library entirely?

IMO we should. Let bitstring be a detail for space_packet_parser, but not bring it into imap_processing if we can avoid it. I found it to be really slow. If we need to do complicated bit manipulations then I don't see much of an issue, but everything I've seen so far is not too bad to handle ourselves. Note that bitstring has already made this a string for you, so you're sort of going back to bitstring then rather than using the original bits.

greglucas · 2024-02-15T18:36:14Z

imap_processing/hit/l0/utils/hit_base.py

+            if "SPARE" in key:
+                continue


I wonder if there is a way to get XTCE / space packet parser to ignore a field like this?

Good question. I'll check the docs

greglucas · 2024-02-15T18:36:51Z

imap_processing/hit/l0/utils/hit_base.py

+            )
+            if "SPARE" in key:
+                continue
+            if key not in attributes and "SPARE" not in key:


You already checked for it above I think

Suggested change

if key not in attributes and "SPARE" not in key:

if key not in attributes:

forgot to remove that part. thanks!

greglucas · 2024-02-15T18:43:42Z

imap_processing/tests/hit/test_housekeeping.py

+    # this test. These issues were discovered / confirmed with the HIT Ops
+    # engineer who delivered the data.


What joy :) Thanks for your patience in finding these! And especially documenting this.

greglucas · 2024-02-15T18:44:48Z

imap_processing/tests/hit/test_housekeeping.py

+    packets = decom.decom_packets(test_file.resolve(), xtce_file.resolve())
+    del packets[-1]


Suggested change

packets = decom.decom_packets(test_file.resolve(), xtce_file.resolve())

del packets[-1]

packets = decom.decom_packets(test_file.resolve(), xtce_file.resolve())[:-1]

greglucas · 2024-02-15T18:45:57Z

imap_processing/tests/hit/test_housekeeping.py

+    del packets[-1]
+    pkt_idx = 0
+
+    for packet in packets:


I'd remove the pkt_idx tracking yourself and let enumerate capture it.

Suggested change

for packet in packets:

for pkt_idx, packet in enumerate(packets):

greglucas · 2024-02-15T18:46:54Z

tools/xtce_generation/xtce_generator_hit.py

+    """HIT XTCE generator"""
+    instrument_name = "hit"
+    current_directory = Path(__file__).parent
+    module_path = f"{current_directory}/../../imap_processing"


import imap_module_path from the main package?

sdhoyt · 2024-02-15T22:00:56Z

I'm getting a "Too many statements" ruff error (PLR0915) because of the length of my test_houskeeping.py unit test function. Is this something we think can be ignored or should I break this test up? It seems weird to spread testing class attributes across multiple functions.

greglucas · 2024-02-15T22:06:50Z

I'm getting a "Too many statements" ruff error (PLR0915) because of the length of my test_houskeeping.py unit test function. Is this something we think can be ignored or should I break this test up? It seems weird to spread testing class attributes across multiple functions.

Yes, you can ignore it IMO. But, if you're looking to reduce boilerplate you have to write you could do something like this for many of those assertions:

# Turn this
assert hk.LAST_BAD_CMD == validation_data["LAST_BAD_CMD"][pkt_idx]
# into:
keys = ["LAST_BAD_CMD", ...]
for key in keys:
    assert getattr(hk, key) == validation_data[key][pkt_idx]

greglucas · 2024-02-16T14:38:06Z

docs/source/code-documentation/hit.rst

-    l0.hit_l1a_decom
+    l0.data_classes
+    l0.utils
+    l0.hit_l1a_decom.py


I think the extra .py might be causing the doc build failure.

Suggested change

l0.hit_l1a_decom.py

l0.hit_l1a_decom

greglucas · 2024-02-16T14:42:05Z

imap_processing/tests/hit/test_housekeeping.py

+                # Need to create an array out of the separate leak columns in the
+                # validation data
+                assert (
+                    hk.LEAK_I
+                    == np.array(validation_data.loc[pkt_idx, leak_columns].tolist())
+                ).all()


Definitely a nit, so feel free to ignore, but I would pull this out and test it individually before the loop since this is only one odd-ball. Also, numpy has nice array testing utilities, which will also print out which entries are failing if you want to start using those for the array tests too.

Suggested change

# Need to create an array out of the separate leak columns in the

# validation data

assert (

hk.LEAK_I

== np.array(validation_data.loc[pkt_idx, leak_columns].tolist())

).all()

# Need to create an array out of the separate leak columns in the

# validation data

np.testing.assert_array_equal(hk.LEAK_I, validation_data.loc[pkt_idx, leak_columns])

That's cool, I didn't realize numpy had the testing module

greglucas · 2024-02-16T14:43:46Z

imap_processing/hit/packet_definitions/P_HIT_IALRT.xml

Do you want to keep the I-ALiRT definition in the repo? @laspsandoval might want to reference it and keep it in, although I think she is creating a new one manually from these as reference so this might not be explicitly needed.

Yeah, I can remove the packet definition. I forgot to ignore it when I did one of the xtce generations.

greglucas

Looks good to me. Just need to fix the doc build.

maxinelasp

Looks good!

maxinelasp · 2024-02-19T16:38:28Z

imap_processing/hit/l0/utils/hit_base.py

+
+
+@dataclass
+class HITBase:


Nice, I like this

maxinelasp · 2024-02-19T16:39:27Z

imap_processing/hit/packet_definitions/P_HIT_AUT.xml

@@ -50,11 +47,11 @@
 			<xtce:Parameter name="PKT_LEN" parameterTypeRef="UINT16">
 				<xtce:LongDescription>CCSDS Packet Length (number of bytes after Packet length minus 1)</xtce:LongDescription>
 			</xtce:Parameter>
-			<xtce:Parameter name="SHCOARSE" parameterTypeRef="UINT32">
-				<xtce:ShortDescription>CCSDS Packet Sec Header</xtce:ShortDescription>
+			<xtce:Parameter name="SC_TICK" parameterTypeRef="UINT32">


I thought SHCOARSE or MET was required - is it missing here because it's a housekeeping packet?

No you're correct. HIT is calling it SC_TICK in their Telem definition and I guess I forgot to change it back to SHCOARSE when I got an updated definition sheet and re-generated the XTCE. I'm going to remove all the HIT XTCE that isn't housekeeping.

maxinelasp · 2024-02-19T16:39:55Z

imap_processing/hit/packet_definitions/P_HIT_HSKP.xml

@@ -63,7 +71,7 @@
 				<xtce:LongDescription>CCSDS Packet Length (number of bytes after Packet length minus 1)</xtce:LongDescription>
 			</xtce:Parameter>
 			<xtce:Parameter name="SHCOARSE" parameterTypeRef="UINT32">


But here, it's still named SHCOARSE? Why?

Yeah, I did remember to change the updated sheet to SHCOARSE for housekeeping since this is the one I was working on for this PR. Going to remove all non-hk xtce for this PR

maxinelasp · 2024-02-19T16:41:15Z

imap_processing/tests/hit/test_housekeeping.py

+    )
+    xtce_file = imap_module_directory / "hit/packet_definitions/P_HIT_HSKP.xml"
+
+    validation_data = pd.read_csv(validation_file)


Nice, I like this method of validation

* added housekeeping data class * added docstrings * removed unneeded import * updated variable names * fixed line lengths * fixed line lengths * comment updates * doc fix attempt * updated hk unit test * comment fixes * comment fix * removed all hit non-hk xtce

sdhoyt added 4 commits February 15, 2024 08:26

added housekeeping data class

68d01db

added docstrings

527644c

removed unneeded import

7e83ea6

updated variable names

cef5d2a

sdhoyt requested a review from a team February 15, 2024 18:14

sdhoyt self-assigned this Feb 15, 2024

sdhoyt requested review from bourque, greglucas, tech3371, bryan-harter, laspsandoval, bmcclellan-cu and maxinelasp and removed request for a team February 15, 2024 18:14

sdhoyt added 3 commits February 15, 2024 11:20

fixed line lengths

7f2a157

Merge branch 'dev' into hit-housekeeping

e34bf05

fixed line lengths

4fe51e8

greglucas reviewed Feb 15, 2024

View reviewed changes

sdhoyt added 2 commits February 15, 2024 14:55

comment updates

ee5562b

Merge branch 'dev' into hit-housekeeping

531cee7

doc fix attempt

18f4922

updated hk unit test

481a766

greglucas reviewed Feb 16, 2024

View reviewed changes

greglucas approved these changes Feb 16, 2024

View reviewed changes

sdhoyt added 2 commits February 16, 2024 11:22

comment fixes

4b82522

comment fix

92b003e

maxinelasp approved these changes Feb 19, 2024

View reviewed changes

removed all hit non-hk xtce

c88494a

sdhoyt merged commit 4195b8f into IMAP-Science-Operations-Center:dev Feb 19, 2024
17 checks passed

sdhoyt deleted the hit-housekeeping branch February 19, 2024 22:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HIT housekeeping data class #342

HIT housekeeping data class #342

sdhoyt commented Feb 15, 2024

greglucas Feb 15, 2024 •

edited

Loading

sdhoyt Feb 15, 2024

sdhoyt Feb 15, 2024

greglucas Feb 15, 2024

greglucas Feb 15, 2024

sdhoyt Feb 15, 2024

greglucas Feb 15, 2024

sdhoyt Feb 15, 2024

greglucas Feb 15, 2024

greglucas Feb 15, 2024

greglucas Feb 15, 2024

greglucas Feb 15, 2024

sdhoyt commented Feb 15, 2024

greglucas commented Feb 15, 2024 •

edited

Loading

greglucas Feb 16, 2024

greglucas Feb 16, 2024

sdhoyt Feb 16, 2024

greglucas Feb 16, 2024

sdhoyt Feb 16, 2024

greglucas left a comment

maxinelasp left a comment

maxinelasp Feb 19, 2024

maxinelasp Feb 19, 2024

sdhoyt Feb 19, 2024

maxinelasp Feb 19, 2024

sdhoyt Feb 19, 2024 •

edited

Loading

maxinelasp Feb 19, 2024

	if key not in attributes and "SPARE" not in key:
	if key not in attributes:

		# this test. These issues were discovered / confirmed with the HIT Ops
		# engineer who delivered the data.

		packets = decom.decom_packets(test_file.resolve(), xtce_file.resolve())
		del packets[-1]

	for packet in packets:
	for pkt_idx, packet in enumerate(packets):

HIT housekeeping data class #342

HIT housekeeping data class #342

Conversation

sdhoyt commented Feb 15, 2024

Change Summary

Overview

New Dependencies

New Files

Deleted Files

Updated Files

Testing

greglucas Feb 15, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sdhoyt commented Feb 15, 2024

greglucas commented Feb 15, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

greglucas left a comment

Choose a reason for hiding this comment

maxinelasp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sdhoyt Feb 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

greglucas Feb 15, 2024 •

edited

Loading

greglucas commented Feb 15, 2024 •

edited

Loading

sdhoyt Feb 19, 2024 •

edited

Loading