Conversation
There was a problem hiding this comment.
Pull request overview
Adjusts the dispatch scheduler’s selection logic to keep using the existing current_observation instance when reselecting the same target, preventing observation progress (exposure count) from being reset during repeat scheduling.
Changes:
- Introduces
keep_currentflow to avoid reassigningself.current_observationwhen the top-ranked observation is the current one. - Recomputes and compares the current observation’s score from the current evaluation round before deciding to keep it.
- Updates how
self.current_observation.meritis set when keeping vs switching observations.
| else: | ||
| # Update merit even if keeping current observation | ||
| self.current_observation.merit = top_obs_score |
There was a problem hiding this comment.
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).
| # 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 |
There was a problem hiding this comment.
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.
| if current_obs_score >= top_obs_score: | ||
| best_obs.insert(0, (self.current_observation, current_obs_score)) | ||
| keep_current = True |
There was a problem hiding this comment.
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).
6857e1b to
3da0d00
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #1400 +/- ##
========================================
Coverage 64.33% 64.33%
========================================
Files 104 104
Lines 9532 9541 +9
Branches 845 848 +3
========================================
+ Hits 6132 6138 +6
- Misses 3254 3256 +2
- Partials 146 147 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
When sticking with the same
Observation, don't reset the image count.