Skip to content

CoDICE: SCI-LUT day boundary refactor#3167

Merged
tech3371 merged 6 commits into
IMAP-Science-Operations-Center:devfrom
tech3371:#2697
May 12, 2026
Merged

CoDICE: SCI-LUT day boundary refactor#3167
tech3371 merged 6 commits into
IMAP-Science-Operations-Center:devfrom
tech3371:#2697

Conversation

@tech3371
Copy link
Copy Markdown
Contributor

@tech3371 tech3371 commented May 8, 2026

Change Summary

closes #2697 and #2453

Overview

Refactor to process for days when there are multiple table_id in CoDICE L1A SCI-LUT.

File changes

Testing

I sent locally processed hi-omni file from day 20260403 to Joey and we match data plot for this scope of work.

@tech3371 tech3371 changed the title #2697 CoDICE: SCI-LUT day boundary refactor May 8, 2026
@tech3371 tech3371 marked this pull request as ready for review May 8, 2026 21:31
@tech3371 tech3371 requested review from joeymukherjee and lacoak21 May 8, 2026 21:31
Copy link
Copy Markdown
Contributor

@lacoak21 lacoak21 left a comment

Choose a reason for hiding this comment

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

Nice refactor. I think there still may be some todos in imap_processing/codice/utils.py for example in calculate_acq_time_per_step. Which brings up that question for me. How are we treating acq_time_per_esa_step variable here? There may be the case where we get two arrays with different values depending on the sci lut.

)
for table_id in unique_table_ids
]
if len(processed) == 1:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do you need these two lines? I think if you pass in processed as a single element list concat will still work.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

good point!

@tech3371
Copy link
Copy Markdown
Contributor Author

Nice refactor. I think there still may be some todos in imap_processing/codice/utils.py for example in calculate_acq_time_per_step. Which brings up that question for me. How are we treating acq_time_per_esa_step variable here? There may be the case where we get two arrays with different values depending on the sci lut.

Good find, @lacoak21!

Looks like I didn't remove that TODO. Looks like I updated function to read in dynamic num_steps_data from SCI-LUT's data instead of hard coding but didn't remove the TODO. Now with this PR update, we should be we ok for that as well. Since we are passing in a specified table_id's SCI-LUT data into our previous function steps, we should still map to using correct num_steps_data for calculate_acq_time_per_step calculation. That's my understanding. I can ask Joey to match our acquisition times to be sure.

@lacoak21
Copy link
Copy Markdown
Contributor

Looks like I didn't remove that TODO. Looks like I updated function to read in dynamic num_steps_data from SCI-LUT's data instead of hard coding but didn't remove the TODO. Now with this PR update, we should be we ok for that as well. Since we are passing in a specified table_id's SCI-LUT data into our previous function steps, we should still map to using correct num_steps_data for calculate_acq_time_per_step calculation. That's my understanding. I can ask Joey to match our acquisition times to be sure.

Oh right! I forgot it was 2d!! Ok great I agree that this is resolved in your PR.

@lacoak21 lacoak21 self-requested a review May 11, 2026 15:34
Copy link
Copy Markdown
Contributor

@lacoak21 lacoak21 left a comment

Choose a reason for hiding this comment

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

Looks good!

@tech3371
Copy link
Copy Markdown
Contributor Author

@mstarkey2158 confirmed that calculate_acq_time_per_step is looking good for the file I sent. voltage_table comment from Michael may be outside of this scope of work.

@tech3371 tech3371 merged commit ad23c79 into IMAP-Science-Operations-Center:dev May 12, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CoDICE telemetry or SCI LUT boundary day handling

2 participants