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

Lo DE Segmented Packets #1016

Merged
merged 26 commits into from
Oct 28, 2024
Merged

Conversation

sdhoyt
Copy link
Contributor

@sdhoyt sdhoyt commented Oct 11, 2024

Change Summary

Overview

This PR adds a function to handle Lo direct event segmented packets. This function will soon be added to Lo processing in combination with the event parsing to produce a L1A dataset for Lo direct events.

If there are too many bits of data needed to store Lo direct events for a single Aggregated Science Cycle into a single packet, the data will be segmented across multiple packets. The segments will be indicated by the seq_flag:

seq_flag 1 = start of segment
seq_flag 0 = one of the middle packet of a segment
seq_flag 2 = last packet of a segment
seq_flag 3 = unsegmented packet

The packets after the first will only contain the headers, shcoarse, binary data and checksum. They will not contain the count and passes fields.

Closes #1011

@sdhoyt sdhoyt added Ins: Lo Related to the IMAP-Lo instrument Level: L0 Level 0 processing Level: L1 Level 1 processing labels Oct 11, 2024
@sdhoyt sdhoyt added this to the Oct 2024 milestone Oct 11, 2024
@sdhoyt sdhoyt requested review from a team October 11, 2024 19:21
@sdhoyt sdhoyt self-assigned this Oct 11, 2024
@sdhoyt sdhoyt requested review from subagonsouth and removed request for a team October 11, 2024 19:21
@sdhoyt sdhoyt requested review from laspsandoval, bourque and tech3371 and removed request for a team October 11, 2024 19:21
Copy link
Contributor

@subagonsouth subagonsouth left a comment

Choose a reason for hiding this comment

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

This looks good to me. Nice clean, easy to read code. I'll approve with the assumption that you address the comment on the use of np.where().

# 1 = start of a group of segmented packet
# 2 = end of a group of segmented packets
# 3 = unsegmented packet
seg_starts = np.where((seq_flgs == 1) | (seq_flgs == 3))[0]
Copy link
Contributor

Choose a reason for hiding this comment

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

I nit-pick the use of np.where() a lot. This should really be np.nonzero(). See the Note at the top of the numpy documentation: https://numpy.org/doc/2.0/reference/generated/numpy.where.html

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah interesting. I wasn't aware of that. Thanks, I'll make that change

@sdhoyt sdhoyt requested a review from joycecj October 14, 2024 16:07
Copy link
Contributor

@tech3371 tech3371 left a comment

Choose a reason for hiding this comment

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

One minor comment. Otherwise, LGTM

seq_flgs = dataset.seq_flgs.values
seq_ctrs = dataset.src_seq_ctr.values

# Find the start and end of each segment of direct events
Copy link
Contributor

Choose a reason for hiding this comment

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

These short comments were very helpful!

imap_processing/tests/lo/test_lo_science.py Show resolved Hide resolved
@tech3371 tech3371 removed this from the Oct 2024 milestone Oct 18, 2024
@sdhoyt
Copy link
Contributor Author

sdhoyt commented Oct 23, 2024

I just made an update to this PR. I found out that padding is added to the binary data to make the packet divisible by 8, so I made a small change to help me identify where to look for padding when this is used in processing. The change adds a comma between segments when they're combined into a single binary string. I'm not crazy about that approach, but I should be able to use that with a find for the next comma or end of the string and determine which bits I need to throw way. I also thought about keeping the binary string separated in a list of lists, but that would require another for loop or passing around another variable to keep track of which segment I'm in. Adding the comma seems easier, but the binary string with commas isn't ideal. Hoping to get others' thoughts on this.

@tech3371
Copy link
Contributor

I just made an update to this PR. I found out that padding is added to the binary data to make the packet divisible by 8, so I made a small change to help me identify where to look for padding when this is used in processing. The change adds a comma between segments when they're combined into a single binary string. I'm not crazy about that approach, but I should be able to use that with a find for the next comma or end of the string and determine which bits I need to throw way. I also thought about keeping the binary string separated in a list of lists, but that would require another for loop or passing around another variable to keep track of which segment I'm in. Adding the comma seems easier, but the binary string with commas isn't ideal. Hoping to get others' thoughts on this.

I wish I have ideas for this. This is where Greg is great at :).

Copy link
Contributor

@tech3371 tech3371 left a comment

Choose a reason for hiding this comment

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

Still looks good to me. You can revisit this again in the future if you think of another solution to ,.

@sdhoyt sdhoyt merged commit ce33595 into IMAP-Science-Operations-Center:dev Oct 28, 2024
17 checks passed
@sdhoyt sdhoyt deleted the lo-seg-pkts branch October 28, 2024 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ins: Lo Related to the IMAP-Lo instrument Level: L0 Level 0 processing Level: L1 Level 1 processing
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Combine Lo segmented direct event data
3 participants