Skip to content

Lo L1C - Automatic Goodtimes#2896

Open
sdhoyt wants to merge 4 commits intoIMAP-Science-Operations-Center:devfrom
sdhoyt:lo-goodtimes
Open

Lo L1C - Automatic Goodtimes#2896
sdhoyt wants to merge 4 commits intoIMAP-Science-Operations-Center:devfrom
sdhoyt:lo-goodtimes

Conversation

@sdhoyt
Copy link
Copy Markdown
Contributor

@sdhoyt sdhoyt commented Mar 31, 2026

Change Summary

Overview

This code integrates Lo's python code for automatic goodtimes into the processing code.

I'm running into a mypy issue because of the number of if statements in a function. I'd like to push fixing that to another ticket a little later down the road to prioritize the remaining work I have for the 3 month maps.

I compared goodtimes values to the results of Lo's script. There was divergence, up to a few hundred seconds. If this needs to be more accurate, I'd also propose pushing this to a later change.

Closes #2841

@sdhoyt sdhoyt added this to the April 2026 milestone Mar 31, 2026
@sdhoyt sdhoyt self-assigned this Mar 31, 2026
@sdhoyt sdhoyt added Ins: Lo Related to the IMAP-Lo instrument Level: L1 Level 1 processing labels Mar 31, 2026
@sdhoyt sdhoyt added this to IMAP Mar 31, 2026
Copy link
Copy Markdown
Collaborator

@ahotasu ahotasu 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 to me--all of my comments are for long-term polish and aren't needed prior to merge.

epochs[row_count] = l1b_histrates["epoch"][max_row_count].values.item()
goodtimes[row_count, :] = [begin, end]
epochs[row_count] = l1b_histrates["epoch"][max_row_count - 1]
goodtimes[row_count, :] = [int(begin - 620), int(end + 320)]
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Same comment as above.

Copy link
Copy Markdown
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.

A few suggestions for improvements. Looks good other than these minor things.

if hasattr(shcoarse, "values"):
shcoarse = shcoarse.values
shcoarse = np.asarray(shcoarse, dtype=np.float64)
# shcoarse = epochs_ttj2000 / 1e9 # Convert from ns to s MET
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.

Suggested change
# shcoarse = epochs_ttj2000 / 1e9 # Convert from ns to s MET

# We use current SPICE kernels, so we should NOT add that offset
shcoarse = ttj2000ns_to_met(epochs_ttj2000)
# Convert to plain numpy array for easier indexing
if hasattr(shcoarse, "values"):
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.

Interesting. Is this because ttj2000ns_to_met was returning an xr.DataArray? I didn't know it would do that. You could just pass in l1b_histrates["epoch"].values instead.


h_intensity = np.sum(l1b_histrates["h_counts"][:, 0:7, 20:50], axis=(1, 2))
o_intensity = np.sum(l1b_histrates["o_counts"][:, 0:7, 20:50], axis=(1, 2))
epochs_ttj2000 = l1b_histrates["epoch"][:]
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.

What is the [:] doing here?

epochs = l1b_histrates["epoch"].values
epochs = xr.DataArray(epochs, dims=["epoch"])
goodtimes = xr.DataArray(np.zeros((max_row_count, 2), dtype=np.int64))
h_background_rate = xr.DataArray(np.zeros((max_row_count, 7), dtype=np.float32))
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.

Lots of hard coded sevens in this section of code. Do you have a global variable somewhere that defines the number of ESA steps that could be used instead? Otherwise, maybe use a module scoped global?

# Note: The reference script adds +9 seconds because they use an
# "older time kernel (pre 2012)"
# We use current SPICE kernels, so we should NOT add that offset
shcoarse = ttj2000ns_to_met(epochs_ttj2000)
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.

shcoarse is typically reserved for integer spacecraft clock coarse ticks. I suggest renaming to "met" or similar.

Comment on lines +2638 to +2646
h_background_rate[row_count, :] = [
h_bg_rate,
h_bg_rate,
h_bg_rate,
h_bg_rate,
h_bg_rate,
h_bg_rate,
h_bg_rate,
]
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.

I think all these repititions of values could be done like:

Suggested change
h_background_rate[row_count, :] = [
h_bg_rate,
h_bg_rate,
h_bg_rate,
h_bg_rate,
h_bg_rate,
h_bg_rate,
h_bg_rate,
]
h_background_rate[row_count, :] = np.full(num_esa_steps, h_bg_rate)

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: L1 Level 1 processing

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Lo L1C GoodTimes Processing

3 participants