Idex release two updates - #3399
Conversation
…dicate instrument status (science, pulser, noise capture). Added flag to indicate if the event was an actual dust event. Resolved ion grid velocity and mass estimates to use the QI/Qt ratio and Qt respectively. Added logic to NaN fields based on flag states.
There was a problem hiding this comment.
Pull request overview
Adds IDEX release-two event classification and saturation-aware processing across L1A–L2B.
Changes:
- Adds event and waveform saturation flags through L1A–L2A.
- Updates velocity, mass, and L2B count selection logic.
- Adds CDF metadata, tests, and a local processing script.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
run_local_idex_chain.sh |
Adds local L1A–L2A processing workflow. |
imap_processing/idex/idex_event_flags.py |
Implements event and saturation classification. |
imap_processing/idex/idex_l1a.py |
Generates event flags. |
imap_processing/idex/idex_l1b.py |
Propagates flags to L1B. |
imap_processing/idex/idex_l2a.py |
Adds saturation-aware estimates and masking. |
imap_processing/idex/idex_l2b.py |
Filters events and selects target estimates. |
imap_processing/tests/idex/test_idex_event_flags.py |
Tests flag classification. |
imap_processing/tests/idex/test_idex_l0.py |
Updates expected variable count. |
imap_processing/tests/idex/test_idex_l1b.py |
Tests L1B flag propagation. |
imap_processing/tests/idex/test_idex_l2a.py |
Tests masking and Ion Grid estimates. |
imap_processing/tests/idex/test_idex_l2b.py |
Tests filtering and target selection. |
imap_processing/cdf/config/imap_idex_l1a_variable_attrs.yaml |
Defines L1A flag metadata. |
imap_processing/cdf/config/imap_idex_l1b_variable_attrs.yaml |
Defines L1B flag metadata. |
imap_processing/cdf/config/imap_idex_l2a_variable_attrs.yaml |
Defines L2A flag metadata. |
imap_processing/cdf/config/imap_idex_l2b_variable_attrs.yaml |
Corrects charge units to pC. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| VAR_TYPE: data | ||
|
|
||
| # <=== Instrument Setting Attributes ===> | ||
| science_event_flag: |
There was a problem hiding this comment.
Have you validated that these new attrs are ISTP compliant?
There was a problem hiding this comment.
Alex is going to use SKTeditor to check
There was a problem hiding this comment.
Best I can tell, these values are compliant. The webpage checker doesn't seem to like a fill-value of 255 but it's what we use across other variables so I think we leave it as is.
There was a problem hiding this comment.
What was the error? Can you attach a screen shot? 255 may be a valid fill val for other variables but not these if its not configured properly.
There was a problem hiding this comment.
ISTP checker now returning compliance for the flags! Thanks for catching those issues.
There was a problem hiding this comment.
excellent. Super appreciate you checking.
lacoak21
left a comment
There was a problem hiding this comment.
OK I have reviewed part of this and will continue when comments are addressed. In the future this PR could have been broken down into maybe 4 or 5 PRs. We generally try to make PRs as small as possible.
| saturation_flag = f"{waveform_name}_saturation_flag" | ||
| if saturation_flag not in l2a_dataset: | ||
| message = f"Required L2A saturation flag is missing: {saturation_flag}" | ||
| logger.error(message) |
There was a problem hiding this comment.
I think you can just choose one. Either log a message and continue with the code and do something OR raise an error.
lacoak21
left a comment
There was a problem hiding this comment.
LGTM! Once we know these are ISTP compliant I think this is good to go.
| CATDESC: Boolean event classification flag. | ||
| DEPEND_0: epoch | ||
| DICT_KEY: SPASE>Support>SupportQuantity:QualityFlag | ||
| FILLVAL: 0 |
There was a problem hiding this comment.
Ah this is super important @aldo9253 I would definitely fix this
| right_bracketed = ( | ||
| right < corrected.size - 1 | ||
| and np.isfinite(corrected[right - 1]) | ||
| and corrected[right - 1] >= half_height | ||
| and np.isfinite(corrected[right]) | ||
| and corrected[right] < half_height | ||
| ) |
|
[cid:2fbfdab1-f6a5-4470-85da-a8b21965989e]
________________________________
From: Luisa Coakley ***@***.***>
Sent: Tuesday, August 25, 2026 1:27 PM
To: IMAP-Science-Operations-Center/imap_processing ***@***.***>
Cc: aldo9253 ***@***.***>; Mention ***@***.***>
Subject: Re: [IMAP-Science-Operations-Center/imap_processing] Idex release two updates (PR #3399)
[External email - use caution]
@lacoak21 commented on this pull request.
________________________________
In imap_processing/cdf/config/imap_idex_l1b_variable_attrs.yaml<#3399 (comment)>:
@@ -56,6 +64,66 @@ spice_base: &spice_base
VAR_TYPE: data
# <=== Instrument Setting Attributes ===>
+science_event_flag:
What was the error? Can you attach a screen shot? 255 may be a valid fill val for other variables but not these if its not configured properly.
—
Reply to this email directly, view it on GitHub<#3399?email_source=notifications&email_token=AJRWSSWVHFP3AMEJNZCU6L35LXSC3A5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTKMBSGMYTANBXGQ32M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#discussion_r3856512975>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AJRWSSQUEAECSY6LAYOBZL35LXSC3AVCNFSNUABFKJSXA33TNF2G64TZHM3DKNBWG44TQMJYHNEXG43VMU5TKMJZGU3DGOJXGY4KC5QC>.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS<https://github.com/notifications/mobile/ios/AJRWSSU2B3GVY5AOE5MFBYT5LXSC3A5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTKMBSGMYTANBXGQ32M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJKTGN5XXIZLSL5UW64Y> and Android<https://github.com/notifications/mobile/android/AJRWSSSMQ6ZWF4YCYOTI3DL5LXSC3A5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTKMBSGMYTANBXGQ32M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLTGN5XXIZLSL5QW4ZDSN5UWI>. Download it today!
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
Can you try uploading it again? |
|
Just emailed it to you. |
| VALIDMIN: 0 | ||
| VAR_TYPE: support_data | ||
|
|
||
| event_flag_base: &event_flag_base |
There was a problem hiding this comment.
Oh I see. There are a couple of issues here.
I would not use "trigger_base" as the base because those are for variables that are not quality flags. So I would remove that part.
Second, make sure the array is actually np.unit8 e.g. data.astype(np.uint8)
It thinks the data is np.uint64 as implied by the warning.
|
Okay, hopefully this commit addressed it all. |
Nice looks great. Another thing I thought of. Have you produced a CDF with this code locally? |
|
Yes, for every version. Here are the most recent versions!
…________________________________
From: Luisa Coakley ***@***.***>
Sent: Tuesday, August 25, 2026 2:29 PM
To: IMAP-Science-Operations-Center/imap_processing ***@***.***>
Cc: aldo9253 ***@***.***>; Mention ***@***.***>
Subject: Re: [IMAP-Science-Operations-Center/imap_processing] Idex release two updates (PR #3399)
[External email - use caution]
[https://avatars.githubusercontent.com/u/48064300?s=20&v=4]lacoak21 left a comment (IMAP-Science-Operations-Center/imap_processing#3399)<#3399 (comment)>
Okay, hopefully this commit addressed it all.
Nice looks great.
Another thing I thought of. Have you produced a CDF with this code locally?
—
Reply to this email directly, view it on GitHub<#3399?email_source=notifications&email_token=AJRWSSREBGU7POQ5KN2F3FL5LXZJTA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKNBRGYZTIMRWHAZKM4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#issuecomment-5416342682>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AJRWSSUTXQZ5DHRY3QFHHVT5LXZJTAVCNFSNUABFKJSXA33TNF2G64TZHM3DKNBWG44TQMJYHNEXG43VMU5TKMJZGU3DGOJXGY4KC5QC>.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS<https://github.com/notifications/mobile/ios/AJRWSSXQICM3RU24URSB5QL5LXZJTA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKNBRGYZTIMRWHAZKM4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJKTGN5XXIZLSL5UW64Y> and Android<https://github.com/notifications/mobile/android/AJRWSSXPVCPEESG42AVHWJ35LXZJTA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKNBRGYZTIMRWHAZKM4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLTGN5XXIZLSL5QW4ZDSN5UWI>. Download it today!
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
|
I have some l2b changes here that will conflict with your 10-day refactor. I'm going to revert the ones here so that we have a clean merge. We will tackle all l2b related updates in seperate PRs more cleanly. |
f469be9
into
IMAP-Science-Operations-Center:dev
Change Summary
closes #3336
Overview
Adds IDEX event classification and waveform saturation flags through L1A–L2A, with
saturation- and Science-mode-aware velocity and mass estimates.
File changes
Testing
Relevant IDEX tests, formatting, linting, type checks, and pre-commit checks pass.
The local 20260719 processing chain completed successfully through L2A.