-
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
Implement packet_file_to_datasets function for CoDICE #804
Implement packet_file_to_datasets function for CoDICE #804
Conversation
… individual integer values
…d "hard coded" into the configuration dictionary; Added spin sector config variable
…cessing pipeline a bit more readable; added proper numbers for positions/energies/spin_sectors
…arrays by positions, spin_sectors, and energies
…ing into codice-packet-file-to-dataset
…processing into codice-packet-file-to-dataset
( | ||
1, | ||
self.num_positions, | ||
self.num_spin_sectors, | ||
self.num_energy_steps, | ||
) |
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.
This change is to avoid 'unexpected argument' warnings from np.reshape()
@@ -309,49 +315,49 @@ def unpack_science_data(self, science_values: str) -> None: | |||
science_values : str | |||
A string of binary data representing the science values of the data. | |||
""" | |||
self.compression_algorithm = constants.LO_COMPRESSION_ID_LOOKUP[self.view_id] | |||
compression_algorithm = constants.LO_COMPRESSION_ID_LOOKUP[self.view_id] |
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.
When I originally wrote this, I thought that I might be using the compression_algorithm
in various places in the module, but turns out I only need it here, so no need to make it a class attribute.
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.
Overall, I think this looks great! A few minor comments is all.
packet : xarray.Dataset | ||
The packet to process. |
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.
full_dataset ?
I think this is the entire packet file you are passing in and not an individual packet.
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.
No I think this is just an individual packet? Here is how it is called:
packets = packet_file_to_datasets(file_path, xtce_packet_definition)
for apid in packets:
packet = packets[apid]
logger.info(f"\nProcessing {CODICEAPID(apid).name} packet")
if apid == CODICEAPID.COD_NHK:
dataset = create_hskp_dataset(packet, data_version)
elif apid in [CODICEAPID.COD_LO_PHA, CODICEAPID.COD_HI_PHA]:
dataset = create_event_dataset(apid, packet, data_version)
imap_processing/codice/codice_l1a.py
Outdated
epoch = xr.DataArray( | ||
met_to_j2000ns( | ||
metadata_arrays["SHCOARSE"], | ||
packet.shcoarse.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.
This should already be taken care of in the "epoch" variable. (I wonder if we will run into issues with you here and the different reference_epoch...
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.
Thanks, good catch. So if I am understanding correctly, this can be changed from:
epoch = xr.DataArray(
met_to_j2000ns(
packet.shcoarse.data,
reference_epoch=np.datetime64("2010-01-01T00:01:06.184", "ns"),
),
name="epoch",
dims=["epoch"],
attrs=cdf_attrs.get_variable_attributes("epoch"),
)
to:
epoch = xr.DataArray(
packet.epoch,
name="epoch",
dims=["epoch"],
attrs=cdf_attrs.get_variable_attributes("epoch"),
)
imap_processing/codice/codice_l1a.py
Outdated
plan_id = packet.data["PLAN_ID"].raw_value | ||
plan_step = packet.data["PLAN_STEP"].raw_value | ||
view_id = packet.data["VIEW_ID"].raw_value | ||
table_id = packet.table_id.data[0] |
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.
Are you able to remove any of the [0]
indexes now and keep the entire array as you go through? I think we'd want each epoch associated with its own table_id and not just the first one.
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.
These four parameters are grabbed from a single APID/packet, so I think these will always be a single value. However, I can avoid the [0]
by using int(packet.table_id.data)
@@ -35,7 +35,7 @@ | |||
(1, 128), # lo-pha | |||
] | |||
EXPECTED_ARRAY_SIZES = [ | |||
123, # hskp | |||
129, # hskp |
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.
Interesting, you got a few more packets now?
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.
Your comment here made me realize that the test for this is poorly named, because these values are actually the expected number of variables in the CDF (i.e. len(dataset)
), not the data array sizes. I will update this in a new commit.
Change Summary
Overview
This PR implements the new
utils.packet_file_to_datasets()
function in the CoDICE pipelines.Updated Files
imap_processing/codice/codice_l0.py
constants.py
since it is used in multiple modules; implementedpacket_file_to_datasets
functionimap_processing/codice/codice_l1a.py
packet_file_to_datasets
function and made a few other cleanup changesimap_processing/codice/constants.py
imap_processing/tests/codice/test_codice_l0.py
andimap_processing/tests/codice/test_codice_l1a.py
Closes #771