Skip to content

Lo - L1B Sweep Table GoodTimes window fix #3021

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.

Sign up for GitHub

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

Open
sdhoyt wants to merge 6 commits into
base: dev
Choose a base branch
from
+2 −2
Open

Lo - L1B Sweep Table GoodTimes window fix #3021

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8d46264
changed goodtime start to goodtime end
sdhoyt Apr 16, 2026
d7a05ca
changed goodtime start to goodtime end
sdhoyt Apr 16, 2026
3cf4624
removed --no-update due to deprecation
sdhoyt Apr 16, 2026
2590907
added 3.12 req to precommit hook
sdhoyt Apr 16, 2026
e2e8f83
reverted pre-commit hook file
sdhoyt Apr 16, 2026
1863253
switched goodtimes check
sdhoyt Apr 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion imap_processing/lo/l1b/lo_l1b.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -498,7 +498,7 @@ def set_esa_mode(
# Get the sweep table rows that correspond to the time period of the pointing
pointing_sweep_df = sweep_df[
(sweep_df["GoodTime_start"] >= pointing_start_met)
& (sweep_df["GoodTime_start"] <= pointing_end_met)
Copy link
Copy Markdown
Contributor Author

@sdhoyt sdhoyt Apr 20, 2026

Choose a reason for hiding this comment

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

Fix is different from what the ticket says, but I think this is correct

Copy link
Copy Markdown
Collaborator

@ahotasu ahotasu Apr 20, 2026

Choose a reason for hiding this comment

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

Have you checked that this fix addresses the l1b DE failure for flight data on 20260407 (and other days with similar errors)?

The updated logic ensures that the sweep table entry ("GoodTime") starts at or after the pointing start time and also starts at or before the pointing end time, right? Or, from the opposite point of view, the pointing starts at or before the GoodTime start and the pointing ends at or after the GoodTime start, which seems odd to me--I think what is really intended here is to validate that the pointing is completely contained within the GoodTime (or the GoodTime encompasses the entire pointing), but I could very well be wrong.

Copy link
Copy Markdown
Contributor Author

@sdhoyt sdhoyt Apr 20, 2026

Choose a reason for hiding this comment

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

I'll deploy it and confirm. I was thinking about it the other way. I thought the Goodtimes windows should be contained within or match the pointing. But I might be wrong about that.

Copy link
Copy Markdown
Contributor Author

@sdhoyt sdhoyt Apr 20, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

I did some debugging and there is no row in the sweep table for where the goodtime start/end window is within a pointing for 20260407. I remember discussing this with Lo a while ago and they said the Goodtimes should never overlap with a repointing so at least at that time, the goodtimes window should exist within a pointing. Do you know if something has changed?

Then the job is failing because it's checking that there is only 1 ESA mode for that goodtime window

    if len(pointing_sweep_df["ESA_Mode"].unique()) == 1:
        # Update the ESA mode strings to be 0 for HiRes and 1 for HiThr
        sweep_df["esa_mode"] = sweep_df["ESA_Mode"].map({"HiRes": 0, "HiThr": 1})
        # Get the ESA mode for the pointing
        esa_mode = sweep_df["esa_mode"].values[0]
        # Repeat the ESA mode for each direct event in the pointing
        esa_mode_array: np.ndarray = np.repeat(esa_mode, len(l1b_science["epoch"]))
    else:
        raise ValueError("Multiple ESA modes found in sweep table for pointing.")

There are 0 ESA modes because it couldn't find a goodtime row but there's only an if ==1 or else, so it's getting lumped in with the too many ESA modes error.

So we need to add logic to handle the case of no goodtimes in the sweep table for a pointing, which I'm not sure would be an error or if Lo has another plan for handling that.

Copy link
Copy Markdown
Contributor Author

@sdhoyt sdhoyt Apr 24, 2026

Choose a reason for hiding this comment

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

I think this update is correct, but it doesn't fix the issue found in the pipeline which is explained above. I think it makes sense to merge this and I'll create a new ticket explaining the other issue.

Copy link
Copy Markdown
Collaborator

@ahotasu ahotasu Apr 24, 2026

Choose a reason for hiding this comment

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

I had to write down an example for myself:

time | event
----------------------
0    | Pointing_start
5    | GoodTime_start
7    | Pointing_end
x    | GoodTime_end

Per the current logic, x could be 6, 7, or 8+ (i.e. before, at, or after the pointing end. I think this is wrong for checking either GoodTime wholly within a pointing or a pointing wholly within a GoodTime.

I don't know which is correct--I haven't asked Lo and your recollection is probably correct. It could be that there is a code inconsistency AND the sweep table is incorrect.

Copy link
Copy Markdown
Contributor

@tmplummer tmplummer Apr 27, 2026

Choose a reason for hiding this comment

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

I agree with Sean. Merge this and write a new ticket for for the follow on.

Copy link
Copy Markdown
Collaborator

@ahotasu ahotasu Apr 27, 2026

Choose a reason for hiding this comment

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

This change was supposed to address the processing job failures, which it is not doing, so I don't understand why we would merge this...

Copy link
Copy Markdown
Contributor Author

@sdhoyt sdhoyt Apr 30, 2026

Choose a reason for hiding this comment

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

The string needs to be fixed, but it's not the cause of the failure. I just meant this string update is complete in this PR, so we can merge this fix now. I wrote another ticket to make clearer errors (#3080).

As far as the processing job failure, I'm not sure that it's an issue with the SDC. If there aren't pointing windows and sweep table goodtimes windows that are lining up, then that seems like an issue with the Lo sweep table ancillary file.

& (sweep_df["GoodTime_end"] <= pointing_end_met)
]

# Check that there is only one ESA mode in the sweep table for the pointing
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ build-backend = "poetry_dynamic_versioning.backend"
[tool.poetry]
name = "imap-processing"
# Gets updated dynamically by the poetry-dynamic-versioning plugin
version = "1.0.25.post6.dev0+882be304"
version = "0.0.0"
description = "IMAP Science Operations Center Processing"
authors = ["IMAP SDC Developers <imap-sdc@lists.lasp.colorado.edu>"]
readme = "README.md"
Expand Down
Loading