feat: add QC system for per-FOV metrics and policy checks#514
Open
feat: add QC system for per-FOV metrics and policy checks#514
Conversation
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
…rker Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
- QCJob.run(): wrap metric calculation in try/except, populate QCResult.error on failure instead of letting exceptions abort the acquisition - _handle_qc_result(): log at error level, always store partial metrics (positional data is valid even when focus score fails) - _summarize_job_result(): add QCResult branch for subprocess path - Policy check: wrap in try/except so policy bugs don't crash acquisition - CSV save: wrap in try/except so disk errors don't crash timepoint - _detect_outliers(): filter NaN/inf values before computing statistics - Add TODO comment for unimplemented pause mechanism - Fix unused Callable import in qc.py - Add TYPE_CHECKING imports for forward references in multi_point_utils.py Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Replace O(n) list scan with O(1) set lookup for deduplication in check_timepoint(). Also add cross-reference comment on CSV fieldnames. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Contributor
There was a problem hiding this comment.
Pull request overview
Introduces a new Quality Control (QC) subsystem to compute per-FOV acquisition metrics, persist them to CSV per timepoint, and run configurable QC policies at timepoint boundaries, integrated into the existing MultiPointWorker acquisition loop.
Changes:
- Added
control/core/qc.pyimplementing focus-score calculation, per-timepoint metric storage, and policy evaluation (thresholds + outlier detection). - Integrated QC job dispatch/result handling, per-timepoint CSV persistence, and end-of-timepoint policy evaluation into
MultiPointWorker. - Extended
MultiPointControllerFunctionswith new QC callbacks (no-op defaults) and added comprehensive tests.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| software/control/core/qc.py | New QC primitives: focus score methods, QC job, timepoint metrics store, and policy engine. |
| software/control/core/multi_point_worker.py | Wires QC into acquisition: dispatch inline QC jobs, store metrics, emit signals, run policies, save qc_metrics.csv. |
| software/control/core/multi_point_utils.py | Adds QC callback signals to controller function bundle with safe defaults. |
| software/tests/control/core/test_qc.py | Adds unit tests covering QC metrics, store/CSV, policy logic, and callback defaults. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Comment on lines
+114
to
+127
| def run(self) -> QCResult: | ||
| image = self.capture_image.image_array | ||
| metrics = FOVMetrics( | ||
| fov_id=FOVIdentifier( | ||
| region_id=str(self.capture_info.region_id), | ||
| fov_index=self.capture_info.fov, | ||
| ), | ||
| timestamp=self.capture_info.capture_time, | ||
| z_position_um=self.capture_info.position.z_mm * 1000, | ||
| ) | ||
|
|
||
| try: | ||
| if self.qc_config.calculate_focus_score: | ||
| metrics.focus_score = calculate_focus_score(image, self.qc_config.focus_score_method) |
Comment on lines
+230
to
+234
| def check_timepoint(self, metrics_store: TimepointMetricsStore) -> PolicyDecision: | ||
| flagged: List[FOVIdentifier] = [] | ||
| reasons: Dict[FOVIdentifier, List[str]] = {} | ||
| all_metrics = metrics_store.get_all() | ||
|
|
software/control/core/qc.py
Outdated
Comment on lines
+174
to
+181
| def get_metric_values(self, metric_name: str) -> Dict[FOVIdentifier, float]: | ||
| with self._lock: | ||
| result = {} | ||
| for fov_id, m in self._metrics.items(): | ||
| value = getattr(m, metric_name, None) | ||
| if value is not None: | ||
| result[fov_id] = value | ||
| return result |
Comment on lines
+381
to
+382
| if self._qc_config.enabled: | ||
| self._job_runners.append((QCJob, None)) |
Comment on lines
+381
to
+382
| if self._qc_config.enabled: | ||
| self._job_runners.append((QCJob, None)) |
Comment on lines
+679
to
+684
| # QC policy check | ||
| if self._qc_policy is not None and self._qc_policy_config.check_after_timepoint: | ||
| if self._metrics_store is not None: | ||
| try: | ||
| decision = self._qc_policy.check_timepoint(self._metrics_store) | ||
| self.callbacks.signal_qc_policy_decision(decision) |
- Use Job.image_array() helper instead of direct attribute access for clearer error on None images - Add enabled guard to QCPolicy.check_timepoint() for self-consistent API - Fix _job_runners type annotation to Optional[JobRunner] - Only dispatch QCJob for canonical frame (first channel, z_index=0) to avoid overwriting metrics across channels/z-slices - Warn when QC policy is enabled without metrics collection Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
- Add FocusScoreMethod enum for focus_score_method config — typos now raise ValueError at construction, not deep in an acquisition loop - Add QCMetricField enum for outlier_metric — invalid metric names are caught immediately instead of silently returning empty results - Implement actual pause mechanism: QC policy sets a threading.Event, acquisition blocks between timepoints until resume_from_qc_pause() is called (or abort is requested) - Add qc_channel_index config to select which channel runs QC (default 0, only z_index=0 is used) - Update get_metric_values to validate metric name via enum - Update tests for enum types Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
QCJob now runs on a specific z-slice (default 0) instead of hardcoded z_index=0. Users can set qc_z_index to target the focal plane of interest in a z-stack. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
qc_metrics.csv)QCConfig.enabled=False) — zero behavior change for existing usersNew file:
control/core/qc.py— FOVIdentifier, FOVMetrics, QCConfig, QCPolicyConfig, QCResult, QCJob, TimepointMetricsStore, QCPolicy, PolicyDecision, calculate_focus_score (4 methods)Modified:
multi_point_worker.py(dispatch + result handling + policy check),multi_point_utils.py(2 new callback signals)Test plan
tests/control/core/test_qc.py— all passpython3 main_hcs.py --simulationwith QC enabled and verifyqc_metrics.csvoutput🤖 Generated with Claude Code