Skip to content

Fix exposure count on repeat observation. #1400

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
wtgee wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Fix exposure count on repeat observation. #1400

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
48 changes: 33 additions & 15 deletions src/panoptes/pocs/scheduler/dispatch.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,22 +99,40 @@ def get_observation(self, time=None, show_all=False, constraints=None, read_file
top_obs_name, top_obs_score = best_obs[0]
self.logger.info(f"Best observation: {top_obs_name}\tScore: {top_obs_score:.02f}")

# Check new best against current_observation
if self.current_observation is not None and top_obs_name != self.current_observation.name:
self.logger.info(f"Checking if {self.current_observation} is still valid")
# Determine if we should keep the current observation
keep_current = False

# Favor the current observation if still available
end_of_next_set = time + self.current_observation.set_duration
if self.observation_available(self.current_observation, end_of_next_set):
# If current is better or equal to top, use it
self.logger.debug(f"{self.current_observation.merit=}")
self.logger.debug(f"{top_obs_score=}")
if self.current_observation.merit >= top_obs_score:
best_obs.insert(0, (self.current_observation, self.current_observation.merit))

# Set the current
self.current_observation = self.observations[top_obs_name]
self.current_observation.merit = top_obs_score
# If we have a current observation, check if we should keep it
if self.current_observation is not None:
# Case 1: Top observation has same name as current - definitely keep it
if top_obs_name == self.current_observation.name:
keep_current = True
self.logger.info(
f"Top observation is current observation, keeping {self.current_observation.name}"
)
# Case 2: Different name, but check if current is still better
elif self.current_observation.name in valid_obs:
# Get the current observation's fresh score from this round of evaluation
current_obs_score = valid_obs[self.current_observation.name]
self.logger.info(f"Checking if {self.current_observation} is still valid")
# Favor the current observation if still available
end_of_next_set = time + self.current_observation.set_duration
if self.observation_available(self.current_observation, end_of_next_set):
# If current is better or equal to top, use it
self.logger.debug(f"{current_obs_score=}")
self.logger.debug(f"{top_obs_score=}")
if current_obs_score >= top_obs_score:
best_obs.insert(0, (self.current_observation, current_obs_score))
keep_current = True
Comment on lines +124 to +126
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

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

best_obs is documented/tests expect tuples of (obs_name: str, score: float), but this branch inserts (self.current_observation, current_obs_score), changing the type of the first element (and also duplicating the current observation entry in best_obs when show_all=True). Use self.current_observation.name (and consider reordering/removing the existing entry rather than inserting a duplicate).

Copilot uses AI. Check for mistakes.

# Set the current observation
# Only update if we're not keeping the current observation to avoid resetting exposure count
if not keep_current:
self.current_observation = self.observations[top_obs_name]
self.current_observation.merit = top_obs_score
Comment on lines +128 to +132
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

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

This change is specifically meant to prevent exposure counts from being reset when the scheduler keeps the same observation (especially when read_file=True rebuilds self.observations). There are scheduler tests, but none appear to assert that current_observation.exposure_list / current_exp_num is preserved across a re-evaluation that keeps the same observation. Adding a focused test would prevent regressions here.

Copilot uses AI. Check for mistakes.
else:
# Update merit even if keeping current observation
self.current_observation.merit = top_obs_score
Comment on lines +133 to +135
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

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

When keep_current is True because the current observation outscored top_obs_score, self.current_observation.merit is updated to top_obs_score (the other observation’s score). This makes current_observation.merit inconsistent with the chosen observation; it should be set to the current observation’s computed score for this cycle (e.g., current_obs_score).

Copilot uses AI. Check for mistakes.
else:
if self.current_observation is not None:
# Favor the current observation if still available
Expand Down
Loading