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 - Create L1A Direct Event CDFs #1221

Merged
merged 4 commits into from
Dec 16, 2024

Conversation

sdhoyt
Copy link
Contributor

@sdhoyt sdhoyt commented Dec 13, 2024

Change Summary

Overview

Adds the DE processing code to the pipeline and adds L1A Direct Event Attributes to the processed dataset

Updated Files

  • imap_processing/cdf/config/imap_lo_l1a_variable_attrs.yaml
    • minor fixes to attribute fields
  • imap_processing/lo/l0/lo_science.py
    • removed bit_pos from attributes (used for tracking bit position of direct event data)
  • imap_processing/lo/l1a/lo_l1a.py
    • Added DE processing to file
    • Added DE attribute setting

Testing

Added check for L1A dataset logical source results for each product being created in the pipeline. Planning to add further attribute testing in the future

Closes #1013
Closes #1014

@sdhoyt sdhoyt added Ins: Lo Related to the IMAP-Lo instrument Level: L1 Level 1 processing CDF Related to CDF files labels Dec 13, 2024
@sdhoyt sdhoyt requested a review from a team December 13, 2024 19:45
@sdhoyt sdhoyt self-assigned this Dec 13, 2024
@sdhoyt sdhoyt requested review from bourque, subagonsouth, tech3371 and joycecj and removed request for a team December 13, 2024 19:45
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.

Changes in l1a makes sense to me. LGTM

@@ -127,6 +127,7 @@ direct_events:
CATDESC: Index of number of direct events for pointing
FIELDNAM: direct_events
LABLAXIS: DE
LABL_PTR_1: direct_events_label
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you want to move this upto line 50? It seems like you are inheriting de_default a lot and it has depend_1. If it has depend_1, it also needs labl_prt_1. I don't remember if ISTP check catches that or not but it will be something you will come across later.

@@ -146,7 +147,7 @@ esa_step:
VALIDMAX: 7
FORMAT: I1
CATDESC: Energy Step
DEPEND_1: esa_step
DEPEND_1: 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.

could this be extra now since de_default already has it? you may just need to remove it?

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.

LGTM

@sdhoyt sdhoyt merged commit 12b60b9 into IMAP-Science-Operations-Center:dev Dec 16, 2024
17 checks passed
@sdhoyt sdhoyt deleted the de-cdf branch December 16, 2024 22:58
nkerman pushed a commit to nkerman/imap_processing that referenced this pull request Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CDF Related to CDF files Ins: Lo Related to the IMAP-Lo instrument Level: L1 Level 1 processing
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Add Lo DE attributes to dataset Add lo DE processing to l1a pipeline
3 participants