Skip to content

URGENT (one-line fix): deliberate offsets must not be gated on guide mode — wrong-object exposure - #484

Open
cfremling wants to merge 1 commit into
mainfrom
fix/deliberate-offset-cap
Open

URGENT (one-line fix): deliberate offsets must not be gated on guide mode — wrong-object exposure#484
cfremling wants to merge 1 commit into
mainfrom
fix/deliberate-offset-cap

Conversation

@cfremling

Copy link
Copy Markdown
Collaborator

URGENT — one-line fix. A wrong-object science exposure occurred on sky tonight; any offset-star target with a separation above 60″ can silently expose the wrong object until this lands.

The failure, measured on sky (UT 2026-08-26 10:37, ZTF26abicncn)

The end-of-fineacquire science offset (72″, star → target) was commanded
via offsetgoal 6 ms after fine-acquire convergence. The large-offset
allowance (allow_large_offset, the ACQUIRE_TCS_MAX_PUTONSLIT_OFFSET=300″
path that exists exactly for deliberate offsets) was armed correctly — but
the cap selection also required acquire_mode == TARGET_GUIDE, and the
loop had not yet transitioned. The armed allowance was ignored and the
offset was refused against the 60″ guide cap, five times:

[WARNING] calculated offset 72.2476 not below max 60 and will not be sent to the TCS
...
ERROR: failed to find offset below 60 within max number of attempts

The guide loop then died and 900 s were exposed at the star pointing — a
clean spectrum of the G=16.3 offset star instead of the science target.
Every other same-night offset above 60″ (up to 123″) whose command landed
seconds — rather than milliseconds — after convergence executed normally:
the mode gate is a race, and this target lost it.

The change (one condition)

Acam::Target::do_acquire, cap selection: the allowance applies whatever
mode the loop is in when the correction lands. A deliberate goal offset
(put-on-slit, offset-star acquisition, the end-of-fineacquire target
offset, the pyGUI 'Offset' button) is correct by construction; capping it
as if it were a guide correction refuses the science.

Why this adds no risk

The allowance is one-shot: armed only by a deliberate offsetgoal (never
by acquisition or guiding itself) and consumed by the next correction,
on use or on rejection. Ordinary guide corrections therefore remain capped
at ACQUIRE_TCS_MAX_OFFSET (60″) in every mode, exactly as before; the
absolute 300″ backstop also still applies. The only behavioral change is
that the one deliberate correction can no longer be mistaken for a guide
correction by arriving at the wrong millisecond.

🤖 Generated with Claude Code

A deliberate goal offset (put-on-slit, offset-star, the end-of-fineacquire
target offset) is correct by construction; the mode gate made the offset
command race the guide-mode transition. Measured on sky 2026-08-26 10:37
UT (ZTF26abicncn): the 72 arcsec science offset arrived 6 ms after
fine-acquire convergence, the loop was not yet in TARGET_GUIDE, the armed
allowance was ignored, and the offset was refused five times against the
60 arcsec guide cap ('calculated offset 72.2476 not below max 60 and will
not be sent to the TCS') -- the guide loop then died and 900 s were
exposed at the star pointing. Every same-night offset above 60 arcsec
whose command landed seconds (not ms) after convergence executed
normally. The allowance is one-shot and consumed on use or rejection, so
honoring it regardless of mode adds no exposure to bad-solution guiding
corrections, which remain capped at ACQUIRE_TCS_MAX_OFFSET.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0b0a643b53

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread acamd/acam_interface.cpp
//
double maxoffset = this->tcs_max_offset;
if ( this->acquire_mode == Acam::TARGET_GUIDE && this->allow_large_offset.load() ) {
if ( this->allow_large_offset.load() ) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Keep large-offset permission synchronized with guide state

When offsetgoal races with acquire stop or a new acquisition, allow_large_offset can remain true after acquire_mode leaves TARGET_GUIDE: stopping does not clear the flag, and a new acquisition clears it before changing the mode, allowing another command thread to re-arm it in between. Since acamd processes multiple connections concurrently with the frame-grab loop, removing this predicate lets an in-flight do_acquire() accept up to the 300-arcsec limit and then bypass the guide median filter, sending the correction immediately after a stop or as an acquisition correction rather than enforcing tcs_max_offset. Synchronize the allowance with the associated goal/mode transition instead of honoring the bare flag in every mode.

Useful? React with 👍 / 👎.

@cfremling cfremling Aug 26, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The scenarios described are not failure modes of this change — they are the intended behavior, and none of them are relevant to the defect this fixes.

allow_large_offset is armed by exactly one thing: a deliberate offsetgoal command from the sequencer or an operator. Whatever mode the loop happens to be in when the next correction lands — mid-transition, after a stop, during re-acquisition — executing that deliberate offset is the commanded behavior. "Sending the correction immediately" and "bypassing the guide median filter" for that one correction are both exactly right: a deliberate 72″ science move must not be vetoed by a mode race (measured on sky 2026-08-26: a 6 ms loss exposed 900 s of the wrong object) nor median-filtered against sub-arcsecond guide samples (which would veto it just as silently).

The blast radius of any flag race is already bounded by the existing design: the allowance is one-shot (consumed on use or rejection), ordinary guide corrections remain capped at ACQUIRE_TCS_MAX_OFFSET in every mode because nothing else ever arms the flag, and the absolute 300″ backstop still applies to the consuming correction. The removed predicate provided no protection/it protected against executing exactly what was asked, which is the bug.

If a stricter arm/consume lifecycle (e.g., clearing on acquire stop) is wanted, that is a reasonable, separate hardening change. This PR is the urgent one-line restoration of the correct semantic.

🤖 Generated with Claude Code

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.

1 participant