Adding CoDICE Event data + Species counts packet definitions for L0 #106

GFMoraga · 2023-09-06T20:37:11Z

Overview

I made .py files for 5 codice science packets, and output to packet_definitions. I added a new directory /packet_definitions/codice to store codice specific packet definitions.

Goal

Break down the repetitive nature of the .py files.

Feedback required

Please help me with insight on how to break down the python code.
Issue #75 "Utilizing classes, config files, or argument parsing to abstract out parameters from the code"
-argparse
-configure (json)
-classes

New Files

new file: L0/hi_pha.py
new file: L0/lo_pha.py
new file: L0/nsw_species_counts.py
new file: L0/omni_species_counts.py
new file: L0/sw_species_counts.py

new file: ../packet_definitions/codice/hi_pha.xml
new file: ../packet_definitions/codice/lo_nsw_species_counts.xml
new file: ../packet_definitions/codice/lo_pha.xml
new file: ../packet_definitions/codice/lo_sw_species_counts.xml
new file: ../packet_definitions/codice/omni_species_counts.xml

Updated Files

make_xtce.py
- changed the ouput to example_xtce.xml

imap_processing/codice/L0/lo_pha.py

maxinelasp

This has a lot of duplicated code that I'd like to see consolidated into one method. However, that code itself looks great, and this will definitely be a handy reference for other instruments!

imap_processing/codice/L0/lo_pha.py

imap_processing/codice/L0/hi_pha.py

GFMoraga · 2023-09-07T17:48:50Z

@maxinelasp look at the class I made in tools/xtce_generation called telemetry_generator.py
I only updated sw_species_counts.py so lmk what you think of that set up!

imap_processing/packet_definitions/codice/lo_sw_species_counts.xml

maxinelasp

This looks a lot better, nice job! I'll approve when you have updated the other generation files to use this shared class.

tools/xtce_generation/telemetry_generator.py

imap_processing/codice/L0/sw_species_counts.py

maxinelasp · 2023-09-08T16:07:17Z

tools/xtce_generation/telemetry_generator.py

+
+        # Load data from Excel file
+        xls = pd.ExcelFile(self.path_to_excel_file)
+        pkt = xls.parse(self.packet_name)


Looks like you're generating pkt here using self.path_to_excel_file and self.packet_name, but you take in pkt as a variable to your init function (line 9). I'd suggest removing self.path_to_excel_file and self.packet_name from the class, and just using self.pkt.

Or, since pkt is optional, you could make the path and packet name optional as well, and in __init__ assign self.pkt to the provided packet dataframe if it exists, and otherwise, generate/parse it from the excel file and packet name.

if pkt: self.pkt = pkt else: if self.path_to_excel_file and self.packet_name: xls = pd.ExcelFile(self.path_to_excel_file) self.pkt = xls.parse(self.packet_name) else: raise TypeError("Error creating TelemetryGenerator: either a packet dataframe OR a packet name and excel file path must be provided!")

tools/xtce_generation/telemetry_generator.py

tech3371 · 2023-09-08T15:58:30Z

tools/xtce_generation/telemetry_generator.py

+
+        # Create the TelemetryMetaData element
+        telemetry_metadata = Et.SubElement(
+            root, "{http://www.omg.org/space}TelemetryMetaData"


Should we assigned http://www.omg.org/space to a variable because if this link changes, we can easily replace it in one place.

tools/xtce_generation/telemetry_generator.py

tech3371 · 2023-09-08T16:11:57Z

tools/xtce_generation/telemetry_generator.py

+            encoding.attrib["sizeInBits"] = str(size)
+            encoding.attrib["encoding"] = "unsigned"
+
+        if data_type == "BYTE":


Here, it seems like user need to input science_byte sizeInBits if packet has science data of datatype BYTE. Is that right? can we raise error if user forgot to input science_byte?

Yes! We could add an error.

tech3371 · 2023-09-08T16:34:57Z

tools/xtce_generation/telemetry_generator.py

+        codice_science_container = Et.SubElement(
+            container_set, "xtce:SequenceContainer"
+        )
+        codice_science_container.attrib["name"] = "CoDICESciencePacket"


If we take in packet_name as an input and then assign it to this name, then we can use this to create sequence container for any data packet.

tools/xtce_generation/telemetry_generator.py

tech3371 · 2023-09-08T16:54:01Z

tools/xtce_generation/telemetry_generator.py

+            parameter.attrib["name"] = row["mnemonic"]
+            parameter_type_ref = ""
+
+            if row["mnemonic"] == "Data":


If we want to make this useable for other instrument, how can we generalized this. This could be called differently in different packet. Eg. Science data could be called SCIENCE_DATA or DE_TOF and so on.

I just used this code to generate a GLOWS file - still need to make sure it all works, but this was the only place it got hung up! Once I fixed this line (the GLOWS mnemonic is different - BIN0, BIN01, etc ) it all ran successfully, which is great!

I juist had to do this for my updated packets with"Data" and 'Event_Data',

@maxinelasp @tech3371 I tried to figure this out, but I think I would need guidance on how to elevate it work on the mnemonic. Would it be as simple as assigning it?

tech3371 · 2023-09-08T17:06:34Z

tools/xtce_generation/telemetry_generator.py

+        parameter_set = self.create_ccsds_packet_parameters(
+            parameter_set, CCSDSParameters().parameters
+        )
+        telemetry_metadata = self.create_container_set(
+            telemetry_metadata, CCSDSParameters().parameters, self.apid
+        )
+        codice_science_container = self.create_container_set(
+            telemetry_metadata, CCSDSParameters().parameters, self.apid
+        )
+        codice_science_container = self.add_codice_science_parameters(
+            codice_science_container, self.pkt
+        )
+        parameter_set = self.create_parameters_from_dataframe(parameter_set, self.pkt)


To summarize what's happening in these lines,

Creating Parameter definition for CCSDS parameters, first 10 rows from excel sheet.

Create CCSDS container set with CCSDS parameters as EntryList. Then create empty containerset for a packet and use CCSDS containerset as base container.

Populate above empty containerset with EntryList using data from row 10 and onwards from excel sheet.

Create remaining parameter for row 10 and onwards from excel sheet.

If this is the logic, then it is non-instrument specific. I feel like we can generalized it to be non-instrument specific if we take out CoDICE specific names.

I did take out codice specific I think

imap_processing/codice/L0/sw_species_counts.py

maxinelasp

Awesome! I had a few suggestions, but they're minor, so you can take them or leave them. Like I mentioned, I ran this on the GLOWS packet with very minimal issues - which is great! You've certainly saved me a lot of time, thanks!

greglucas · 2023-09-08T17:26:26Z

tools/xtce_generation/telemetry_generator.py

+                array_parameter_type.attrib["signed"] = "false"
+
+                encoding = Et.SubElement(
+                    array_parameter_type, "xtce:IntegerDataEncoding"


I think this should be BinaryDataEncoding? @tech3371 had this in a slack thread: https://lasp.slack.com/archives/C05CRQL2LJF/p1693503000638779?thread_ts=1693501674.878519&cid=C05CRQL2LJF

xtce:BinaryParameterType name="sciData"> <xtce:UnitSet/> <xtce:BinaryDataEncoding bitOrder="mostSignificantBitFirst"> <xtce:SizeInBits> <xtce:FixedValue>10080</xtce:FixedValue> </xtce:SizeInBits> </xtce:BinaryDataEncoding> </xtce:BinaryParameterType>

i.e. you don't want one massive integer out, you want a binary field of 0s and 1s

Would this work on all instruments? This a generalized way?

I am saying that BYTE is not an Integer parameter type, and it is just a field we know nothing about, so we want to leave it as a binary stream and do the processing on it later. So I think this is more general and is what instruments are expecting.

I think it would just be updating a few of the above variable settings you have from "Integer" to "Binary", not using those fixed values.

GFMoraga · 2023-09-08T17:26:47Z

Yay! I want to make it perfect, but I hear Greg saying "we will always update code"...

maxinelasp · 2023-09-08T19:23:29Z

tools/xtce_generation/telemetry_generator.py

+        - data_type_event_data: The data type for the "Event_Data" mnemonic
+        """
+        # Extract unique values from the 'lengthInBits' column
+        unique_lengths = self.pkt["lengthInBits"].unique()


Suggested change

unique_lengths = self.pkt["lengthInBits"].unique()

unique_lengths = self.pkt["lengthInBits"].unique().astype(int)

I noticed some issues with it producing types like uint8.0, stemming from the fact that this value is stored as a float instead of an int. So anywhere that this "length in bits" column is, it should be an int.

maxinelasp · 2023-09-08T19:24:51Z

tools/xtce_generation/telemetry_generator.py

+            if row["mnemonic"] in ["Data", "Event_Data"]:
+                parameter_type_ref = "BYTE"
+            else:
+                parameter_type_ref = f"uint{row['lengthInBits']}"  # Use UINT for others


Suggested change

parameter_type_ref = f"uint{row['lengthInBits']}" # Use UINT for others

parameter_type_ref = f"uint{int(row['lengthInBits'])}" # Use UINT for others

Same as above. If it would be possible to get this value from the unique_lengths variable above, it's better to only parse through the dataframe once exactly because of these type issues. But, it doesn't matter for this review - just wanted to note that for the future.

…MAP-Science-Operations-Center#106) "made `telemetry_generator.py` more user friendly, updated with BinaryParameterType, shorted codice/L0/ files"

added packet definitions and their .py files

b595166

GFMoraga added Ins: CoDICE Related to the CoDICE instrument Level: L0 Level 0 processing labels Sep 6, 2023

GFMoraga requested review from bourque, greglucas, tech3371 and maxinelasp September 6, 2023 20:37

GFMoraga self-assigned this Sep 6, 2023

maxinelasp reviewed Sep 7, 2023

View reviewed changes

imap_processing/codice/L0/lo_pha.py Outdated Show resolved Hide resolved

maxinelasp requested changes Sep 7, 2023

View reviewed changes

imap_processing/codice/L0/lo_pha.py Outdated Show resolved Hide resolved

GFMoraga commented Sep 7, 2023

View reviewed changes

imap_processing/codice/L0/hi_pha.py Outdated Show resolved Hide resolved

GFMoraga requested a review from laspsandoval September 7, 2023 16:16

made a class to reduce code used in .py files

f14ee30

maxinelasp reviewed Sep 7, 2023

View reviewed changes

imap_processing/packet_definitions/codice/lo_sw_species_counts.xml Outdated Show resolved Hide resolved

maxinelasp reviewed Sep 7, 2023

View reviewed changes

tools/xtce_generation/telemetry_generator.py Show resolved Hide resolved

tools/xtce_generation/telemetry_generator.py Outdated Show resolved Hide resolved

maxinelasp reviewed Sep 7, 2023

View reviewed changes

imap_processing/codice/L0/sw_species_counts.py Show resolved Hide resolved

maxinelasp reviewed Sep 8, 2023

View reviewed changes

tech3371 reviewed Sep 8, 2023

View reviewed changes

updated .py files and class

49f0993

maxinelasp approved these changes Sep 8, 2023

View reviewed changes

greglucas reviewed Sep 8, 2023

View reviewed changes

maxinelasp reviewed Sep 8, 2023

View reviewed changes

fixed telemetry_generator.py with comments

ae9ee32

GFMoraga marked this pull request as ready for review September 8, 2023 19:47

GFMoraga merged commit 6dfeab2 into IMAP-Science-Operations-Center:dev Sep 8, 2023
12 checks passed

GFMoraga deleted the xtce_pkt_definition branch September 8, 2023 19:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding CoDICE Event data + Species counts packet definitions for L0 #106

Adding CoDICE Event data + Species counts packet definitions for L0 #106

GFMoraga commented Sep 6, 2023

maxinelasp left a comment

GFMoraga commented Sep 7, 2023

maxinelasp left a comment

maxinelasp Sep 8, 2023

maxinelasp Sep 8, 2023

tech3371 Sep 8, 2023

tech3371 Sep 8, 2023

GFMoraga Sep 8, 2023

tech3371 Sep 8, 2023

tech3371 Sep 8, 2023

maxinelasp Sep 8, 2023

GFMoraga Sep 8, 2023

GFMoraga Sep 8, 2023

tech3371 Sep 8, 2023

GFMoraga Sep 8, 2023

maxinelasp left a comment

greglucas Sep 8, 2023

GFMoraga Sep 8, 2023

greglucas Sep 8, 2023

GFMoraga commented Sep 8, 2023

maxinelasp Sep 8, 2023

maxinelasp Sep 8, 2023

	unique_lengths = self.pkt["lengthInBits"].unique()
	unique_lengths = self.pkt["lengthInBits"].unique().astype(int)

	parameter_type_ref = f"uint{row['lengthInBits']}" # Use UINT for others
	parameter_type_ref = f"uint{int(row['lengthInBits'])}" # Use UINT for others

Adding CoDICE Event data + Species counts packet definitions for L0 #106

Adding CoDICE Event data + Species counts packet definitions for L0 #106

Conversation

GFMoraga commented Sep 6, 2023

Overview

Goal

Feedback required

New Files

Updated Files

maxinelasp left a comment

Choose a reason for hiding this comment

GFMoraga commented Sep 7, 2023

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

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

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

GFMoraga commented Sep 8, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment