diff --git a/software/control/core/laser_auto_focus_controller.py b/software/control/core/laser_auto_focus_controller.py index c18dc5a31..88ed56aff 100644 --- a/software/control/core/laser_auto_focus_controller.py +++ b/software/control/core/laser_auto_focus_controller.py @@ -392,6 +392,19 @@ def _move_z(self, um_to_move: float) -> None: else: self.stage.move_z(um_to_move / 1000) + def apply_relative_offset_um(self, offset_um: float) -> None: + """Open-loop relative Z move of ``offset_um`` (displacement µm, 1:1 with Z), with NO + spot re-verification. + + Intended to run right after ``move_to_target(0.0)`` has anchored — and verified — the + spot at the reference plane, to then place the sample at a target displacement from + that reference. Spot-alignment verification (``_verify_spot_alignment``) always crops + at ``x_reference`` and would fail for a deliberately-displaced spot, so it must NOT be + used to reach a nonzero target; this method deliberately skips it. No-op for offset 0. + """ + if offset_um: + self._move_z(offset_um) + def set_reference(self) -> bool: """Set the current spot position as the reference position. diff --git a/software/control/core/multi_point_controller.py b/software/control/core/multi_point_controller.py index 680c22b8d..92c667c4a 100644 --- a/software/control/core/multi_point_controller.py +++ b/software/control/core/multi_point_controller.py @@ -226,6 +226,7 @@ def __init__( self.overlap_percent = 10.0 # FOV overlap percentage self.focus_map = None + self.region_laser_af_offsets = {} self.gen_focus_map = False self.focus_map_storage = [] self.already_using_fmap = False @@ -415,6 +416,11 @@ def set_gen_focus_map_flag(self, flag): def set_focus_map(self, focusMap): self.focus_map = focusMap # None if dont use focusMap + def set_region_laser_af_offsets(self, offsets): + # region_id -> µm offset from the global laser-AF reference plane. Empty dict means + # every FOV targets the reference (displacement 0), i.e. current behavior. + self.region_laser_af_offsets = dict(offsets or {}) + def set_base_path(self, path): self.base_path = path @@ -674,6 +680,13 @@ def get_estimated_mosaic_ram_bytes(self) -> int: return mosaic_width * mosaic_height * bytes_per_pixel * num_channels def run_acquisition(self, acquire_current_fov=False): + # Consume the per-region laser-AF offsets for THIS run and clear the sticky controller + # copy up-front. Any early return below — or a prior GUI abort that pushed offsets but + # never reached run_acquisition (e.g. aborted on the disk/RAM dialog) — then cannot leak + # them into a later acquisition from an entry point that never sets them (fluidics + # widget, TCP control server). + run_region_laser_af_offsets = self.region_laser_af_offsets + self.region_laser_af_offsets = {} if not self.validate_acquisition_settings(): # emit acquisition finished signal to re-enable the UI self.callbacks.signal_acquisition_finished() @@ -840,7 +853,10 @@ def finish_fn(): updated_callbacks = dataclasses.replace(self.callbacks, signal_acquisition_finished=finish_fn) - acquisition_params = self.build_params(scan_position_information=scan_position_information) + acquisition_params = self.build_params( + scan_position_information=scan_position_information, + region_laser_af_offsets=run_region_laser_af_offsets, + ) # Gather objective and camera info for YAML current_objective = self.objectiveStore.current_objective @@ -918,7 +934,9 @@ def finish_fn(): self._memory_monitor.stop() self._memory_monitor = None - def build_params(self, scan_position_information: ScanPositionInformation) -> AcquisitionParameters: + def build_params( + self, scan_position_information: ScanPositionInformation, region_laser_af_offsets: Optional[dict] = None + ) -> AcquisitionParameters: # Determine plate dimensions from wellplate format if available plate_num_rows = 8 # Default for 96-well plate_num_cols = 12 @@ -958,6 +976,7 @@ def build_params(self, scan_position_information: ScanPositionInformation) -> Ac plate_num_rows=plate_num_rows, plate_num_cols=plate_num_cols, xy_mode=self.xy_mode, + region_laser_af_offsets=region_laser_af_offsets if region_laser_af_offsets is not None else {}, ) def _on_acquisition_completed(self): diff --git a/software/control/core/multi_point_utils.py b/software/control/core/multi_point_utils.py index 11157e1f9..79dbbb68c 100644 --- a/software/control/core/multi_point_utils.py +++ b/software/control/core/multi_point_utils.py @@ -1,4 +1,4 @@ -from dataclasses import dataclass +from dataclasses import dataclass, field from typing import List, Tuple, Dict, Optional, Callable, TYPE_CHECKING from control.core.job_processing import CaptureInfo @@ -64,6 +64,11 @@ class AcquisitionParameters: # XY mode for determining scan type xy_mode: str = "Current Position" # "Current Position", "Select Wells", "Manual", "Load Coordinates" + # Per-region laser-AF target offsets (µm from the global laser-AF reference plane), + # keyed by region_id. Empty unless the focus-map + laser-AF combined mode is active, + # in which case each FOV in a region targets that region's offset instead of 0. + region_laser_af_offsets: Dict[str, float] = field(default_factory=dict) + @dataclass class OverallProgressUpdate: diff --git a/software/control/core/multi_point_worker.py b/software/control/core/multi_point_worker.py index 9c834ed83..018417747 100644 --- a/software/control/core/multi_point_worker.py +++ b/software/control/core/multi_point_worker.py @@ -120,6 +120,7 @@ def __init__( self.do_reflection_af = acquisition_parameters.do_reflection_autofocus self.use_piezo = acquisition_parameters.use_piezo self.apply_channel_offset = acquisition_parameters.apply_channel_offset + self.region_laser_af_offsets = acquisition_parameters.region_laser_af_offsets self.display_resolution_scaling = acquisition_parameters.display_resolution_scaling self.experiment_ID = acquisition_parameters.experiment_ID @@ -1155,7 +1156,15 @@ def perform_autofocus(self, region_id, fov): # value, NOT by raising — both paths must mark the FOV's AF as failed or # the per-channel z-offset gate would apply offsets from an unanchored z. try: - af_succeeded = self.laser_auto_focus_controller.move_to_target(0) + target_um = self.region_laser_af_offsets.get(region_id, 0.0) + # Anchor closed-loop at the reference (displacement 0), where spot-alignment + # verification is valid, then apply the per-region offset as an OPEN-LOOP + # relative move. Driving move_to_target() straight to a nonzero displacement + # would always fail verification (its crop is fixed at x_reference) and revert. + af_succeeded = self.laser_auto_focus_controller.move_to_target(0.0) + if af_succeeded and target_um: + self._log.info(f"applying per-region laser AF offset for region '{region_id}': {target_um:+.2f} µm") + self.laser_auto_focus_controller.apply_relative_offset_um(target_um) except Exception as e: file_ID = f"{region_id}_focus_camera.bmp" saving_path = os.path.join(self.base_path, self.experiment_ID, str(self.time_point), file_ID) diff --git a/software/control/gui_hcs.py b/software/control/gui_hcs.py index 361f76836..b443520fe 100644 --- a/software/control/gui_hcs.py +++ b/software/control/gui_hcs.py @@ -974,7 +974,12 @@ def load_widgets(self): self.wellSelectionWidget = widgets.Well1536SelectionWidget(self.wellplateFormatWidget) self.scanCoordinates.add_well_selector(self.wellSelectionWidget) self.focusMapWidget = widgets.FocusMapWidget( - self.stage, self.navigationViewer, self.scanCoordinates, core.FocusMap() + self.stage, + self.navigationViewer, + self.scanCoordinates, + core.FocusMap(), + self.laserAutofocusController, + self.liveController, ) if SUPPORT_LASER_AUTOFOCUS: diff --git a/software/control/widgets.py b/software/control/widgets.py index 1607b1f4b..bb471c506 100644 --- a/software/control/widgets.py +++ b/software/control/widgets.py @@ -6519,6 +6519,14 @@ def toggle_acquisition(self, pressed): self.signal_acquisition_started.emit(True) self.signal_acquisition_shape.emit(self.entry_NZ.value(), self.entry_deltaZ.value()) + # Push per-region laser-AF offsets only now that all pre-flight checks have passed, + # so a disk/RAM abort above cannot strand them on the shared controller for a later run. + self.multipointController.set_region_laser_af_offsets( + self.focusMapWidget.get_offsets_for_acquisition(self.checkbox_withReflectionAutofocus.isChecked()) + if self.checkbox_useFocusMap.isChecked() + else {} + ) + # Start coordinate-based acquisition self.multipointController.run_acquisition() else: @@ -8871,6 +8879,14 @@ def toggle_acquisition(self, pressed): # Update UI to show acquisition is running self._set_ui_acquisition_running(self.entry_NZ.value(), self.entry_deltaZ.value()) + # Push per-region laser-AF offsets only now that all pre-flight checks have passed, + # so a disk/RAM abort above cannot strand them on the shared controller for a later run. + self.multipointController.set_region_laser_af_offsets( + self.focusMapWidget.get_offsets_for_acquisition(self.checkbox_withReflectionAutofocus.isChecked()) + if self.checkbox_useFocusMap.isChecked() + else {} + ) + # Start acquisition self.multipointController.run_acquisition() @@ -10368,26 +10384,50 @@ def _is_valid_column_value(self, column_name: str, value: int) -> bool: class FocusMapWidget(QFrame): """Widget for managing focus map points and surface fitting""" - def __init__(self, stage: AbstractStage, navigationViewer, scanCoordinates, focusMap): + def __init__( + self, + stage: AbstractStage, + navigationViewer, + scanCoordinates, + focusMap, + laserAutofocusController=None, + liveController=None, + ): super().__init__() self.setFrameStyle(QFrame.Panel | QFrame.Raised) self._allow_updating_focus_points_on_signal = True + self._log = squid.logging.get_logger(self.__class__.__name__) # Store controllers self.stage = stage self.navigationViewer = navigationViewer self.scanCoordinates = scanCoordinates self.focusMap = focusMap + self.laserAutofocusController = laserAutofocusController + # Main imaging live controller — suspended around laser-AF offset captures (see + # _capture_region_offset). May be None when laser AF is unsupported. + self.liveController = liveController # Store focus points in widget self.focus_points = [] # list of (region_id, x, y, z) tuples self.enabled = False # toggled when focus map enabled for next acquisition + # Per-region laser-AF offsets (µm from the global laser-AF reference), keyed by + # region_id. Captured at each focus point when the combined mode is enabled. + self.region_laser_af_offsets = {} + self.capture_laser_af_offset_enabled = False + # The laser-AF reference (x_reference) the current offsets were measured against. + # Used to distinguish a genuine re-reference (which invalidates offsets) from the + # benign, unchanged re-emit of signal_reference_changed on every config reload. + self._offsets_reference_x = None self.setup_ui() self.make_connections() self.setEnabled(False) self.add_margin = True # margin for focus grid makes it smaller, but will avoid points at the borders + if self.laserAutofocusController is not None: + self.laserAutofocusController.signal_reference_changed.connect(self._on_laser_af_reference_changed) + def setup_ui(self): """Create and arrange UI components""" self.layout = QVBoxLayout(self) @@ -10453,6 +10493,14 @@ def setup_ui(self): self.by_region_checkbox = QCheckBox("Fit by Region") self.by_region_checkbox.setChecked(False) settings_layout.addWidget(self.by_region_checkbox) + self.checkbox_perRegionLaserAFOffset = QCheckBox("Laser AF Offset") + self.checkbox_perRegionLaserAFOffset.setChecked(False) + self.checkbox_perRegionLaserAFOffset.setEnabled(False) + self.checkbox_perRegionLaserAFOffset.setToolTip( + "With laser AF and focus map both on: capture each region's offset from the laser AF " + "reference at its focus point, and drive laser AF to that per-region target during acquisition." + ) + settings_layout.addWidget(self.checkbox_perRegionLaserAFOffset) self.layout.addLayout(settings_layout) # Status label - reserve space even when hidden @@ -10481,6 +10529,9 @@ def make_connections(self): # Connect fitting method change self.fit_method_combo.currentTextChanged.connect(self._match_by_region_box) + self.fit_method_combo.currentTextChanged.connect(self._update_per_region_offset_enabled) + self.by_region_checkbox.toggled.connect(self._update_per_region_offset_enabled) + self.checkbox_perRegionLaserAFOffset.toggled.connect(self._on_per_region_offset_toggled) def update_point_list(self): """Update point selection combo showing grid coordinates for points""" @@ -10554,6 +10605,13 @@ def edit_current_point(self): new_y = y_spin.value() new_z = z_spin.value() / 1000 # Convert μm back to mm for storage self.focus_points[index] = (region_id, new_x, new_y, new_z) + # The point moved but the stage isn't there to re-measure, so any captured + # laser-AF offset is now stale — drop it (do NOT blind-capture) so it can't + # override the edited z at acquisition. Prompt the user to re-capture. + if self.region_laser_af_offsets.pop(region_id, None) is not None: + msg = f"Region {region_id}: edited — laser AF offset cleared; re-capture with Update Z" + self.status_label.setText(msg) + self._log.info(msg) self.update_point_list() self.update_focus_point_display() @@ -10568,6 +10626,7 @@ def generate_grid(self, rows=4, cols=4): if self.enabled: self.point_combo.blockSignals(True) self.focus_points.clear() + self._clear_region_offsets() self.navigationViewer.clear_focus_points() self.status_label.setText(" ") current_z = self.stage.get_pos().z_mm @@ -10631,6 +10690,7 @@ def add_current_point(self): self.focus_points.append((region_id, pos.x_mm, pos.y_mm, pos.z_mm)) self.update_point_list() self.navigationViewer.register_focus_point(pos.x_mm, pos.y_mm) + self._capture_region_offset(region_id) else: QMessageBox.warning(self, "Region Error", "Could not determine a valid region for this focus point.") @@ -10638,6 +10698,7 @@ def remove_current_point(self): index = self.point_combo.currentIndex() if 0 <= index < len(self.focus_points): self.focus_points.pop(index) + self._sync_offsets_to_focus_points() self.update_point_list() self.update_focus_point_display() @@ -10665,6 +10726,7 @@ def update_current_z(self): region_id, x, y, _ = self.focus_points[index] self.focus_points[index] = (region_id, x, y, new_z) self.update_point_list() + self._capture_region_offset(region_id) def get_region_points_dict(self): points_dict = {} @@ -10674,6 +10736,137 @@ def get_region_points_dict(self): points_dict[region_id].append((x, y, z)) return points_dict + def _capture_region_offset(self, region_id): + """Record the laser-AF displacement at the current z as this region's offset. + + Called after a focus point's z is (re)set (Add / Update Z). The region's previous + offset is dropped up-front: the point's z just changed, so any prior offset is stale + and only a fresh, valid reading may replace it. On any failure (mode off / no + controller / no reference / bad reading) the region is left WITHOUT an offset — it + falls back to the reference plane at acquisition — and the reason is both shown on the + status line and logged (get_offsets_for_acquisition also warns at run start about + regions missing an offset). Warns but still stores when the offset exceeds range. + """ + self.region_laser_af_offsets.pop(region_id, None) + if not self.capture_laser_af_offset_enabled or self.laserAutofocusController is None: + return + if not self.laserAutofocusController.laser_af_properties.has_reference: + msg = f"Laser AF reference not set — offset not captured for region {region_id}" + self.status_label.setText(msg) + self._log.warning(msg) + return + # Suspend the main camera's live stream during the measurement: laser AF toggles the + # AF laser via the microcontroller and waits for completion, while live keeps queuing + # triggers on the same serial link; the contention can time out and return NaN. Mirror + # LiveControlWidget.capture_current_z_offset. start_live() alone (no signal) resumes the + # stream without yanking the user to the Live tab. + was_live = self.liveController is not None and self.liveController.is_live + try: + if was_live: + self.liveController.stop_live() + offset_um = self.laserAutofocusController.measure_displacement() + finally: + if was_live: + self.liveController.start_live() + # measure_displacement() returns float('nan') on a soft failure; guard with isfinite + # (matching capture_current_z_offset) so an invalid reading is never stored. + if offset_um is None or not math.isfinite(offset_um): + msg = f"Laser AF spot not detected — offset not captured for region {region_id}" + self.status_label.setText(msg) + self._log.warning(msg) + return + laser_af_range = self.laserAutofocusController.laser_af_properties.laser_af_range + if abs(offset_um) > laser_af_range: + msg = ( + f"Warning: region {region_id} offset {offset_um:.1f} µm exceeds laser AF range " + f"({laser_af_range:.1f} µm); it may fail AF during acquisition" + ) + self.status_label.setText(msg) + self._log.warning(msg) + else: + self.status_label.setText(f"Region {region_id}: Laser AF offset {offset_um:+.2f} µm") + self._log.info(f"Captured laser AF offset for region {region_id}: {offset_um:+.2f} µm") + self.region_laser_af_offsets[region_id] = offset_um + # Remember which reference these offsets were measured against (see _on_laser_af_reference_changed). + self._offsets_reference_x = self.laserAutofocusController.laser_af_properties.x_reference + + def _on_per_region_offset_toggled(self, checked): + # Toggling only gates whether NEW captures happen; it deliberately does not clear + # already-captured offsets. Those are cleared only when the laser-AF reference + # changes or the focus points change. get_offsets_for_acquisition returns {} while + # unchecked, so retaining them here is safe and avoids losing data on an incidental + # uncheck (e.g. switching focus-map method). + self.capture_laser_af_offset_enabled = checked + + def _update_per_region_offset_enabled(self, *args): + # The per-region laser-AF offset is only valid for the constant, one-point-per-well + # focus map (method="constant" + Fit by Region): a non-constant surface has many + # points per region, so a single per-region offset cannot represent it. Enable is + # driven ONLY by these two SHARED focus-map controls — deliberately NOT by any + # multipoint tab's Reflection AF checkbox, because this one widget is shared across + # all tabs and a per-tab toggle would clobber the others ("last toggle wins"). + # Whether laser AF is actually active is enforced at acquisition by + # get_offsets_for_acquisition, using the running tab's own checkbox. + # (*args absorbs the value emitted by the combo/checkbox signals.) + enabled = self.fit_method_combo.currentText() == "constant" and self.by_region_checkbox.isChecked() + self.checkbox_perRegionLaserAFOffset.setEnabled(enabled) + if not enabled and self.checkbox_perRegionLaserAFOffset.isChecked(): + self.checkbox_perRegionLaserAFOffset.setChecked(False) + + def _clear_region_offsets(self): + self.region_laser_af_offsets = {} + self._offsets_reference_x = None + + def _sync_offsets_to_focus_points(self): + """Drop offsets for regions that no longer have any focus point.""" + live_regions = {rid for rid, _, _, _ in self.focus_points} + self.region_laser_af_offsets = { + rid: offset for rid, offset in self.region_laser_af_offsets.items() if rid in live_regions + } + + def get_offsets_for_acquisition(self, reflection_af_active): + """Per-region laser-AF offsets to apply for an acquisition, or {} when the combined + mode is inactive (reflection AF off, or the per-region checkbox unchecked). + + Centralizes the reflection-AF/checkbox gating so both multipoint widgets — and any + future acquisition entry point — share one source of truth. + """ + if not (reflection_af_active and self.checkbox_perRegionLaserAFOffset.isChecked()): + return {} + offsets = dict(self.region_laser_af_offsets) + # Flag scan regions with no captured offset (they fall back to the reference plane), + # so a silently-missing well is visible at run start rather than only diagnosable later. + try: + scan_regions = set(self.scanCoordinates.region_centers.keys()) + except AttributeError: + scan_regions = set() + missing = sorted(str(r) for r in scan_regions if r not in offsets) + if missing: + msg = ( + f"Laser AF Offset: {len(missing)} region(s) without a captured offset will use " + f"the reference plane: {', '.join(missing)}" + ) + self.status_label.setText(msg) + self._log.warning(msg) + return offsets + + def _on_laser_af_reference_changed(self, has_reference): + # signal_reference_changed is re-emitted on every config reload (objective/profile + # switch) with an UNCHANGED reference, so clear only when the reference actually differs + # from the one the current offsets were measured against — otherwise a benign reload + # would silently wipe valid offsets. + if not self.region_laser_af_offsets: + return + current_x = None + if has_reference and self.laserAutofocusController is not None: + current_x = self.laserAutofocusController.laser_af_properties.x_reference + if current_x == self._offsets_reference_x: + return + self._clear_region_offsets() + msg = "Laser AF reference changed — captured per-region offsets cleared" + self.status_label.setText(msg) + self._log.info(msg) + def fit_surface(self): try: method = self.fit_method_combo.currentText() @@ -10726,6 +10919,59 @@ def _match_by_region_box(self): if self.fit_method_combo.currentText() == "constant": self.by_region_checkbox.setChecked(True) + def _write_focus_points_csv(self, file_path): + with open(file_path, "w", newline="") as csvfile: + writer = csv.writer(csvfile) + writer.writerow(["Region_ID", "X_mm", "Y_mm", "Z_um", "Offset_um"]) + for region_id, x, y, z in self.focus_points: + offset = self.region_laser_af_offsets.get(region_id, "") + writer.writerow([region_id, x, y, z, offset]) + + def _read_focus_points_csv(self, file_path): + """Parse a focus-points CSV. Returns (points, offsets). + + points: list of (region_id, x, y, z). offsets: {region_id: float} for rows that + carry a non-empty Offset_um (column optional, for back-compat). Raises ValueError + if any required column is missing. + """ + points = [] + offsets = {} + with open(file_path, "r", newline="") as csvfile: + reader = csv.reader(csvfile) + header = next(reader) + required_columns = ["Region_ID", "X_mm", "Y_mm", "Z_um"] + if not all(col in header for col in required_columns): + raise ValueError(f"CSV file must contain columns: {', '.join(required_columns)}") + region_idx = header.index("Region_ID") + x_idx = header.index("X_mm") + y_idx = header.index("Y_mm") + z_idx = header.index("Z_um") + offset_idx = header.index("Offset_um") if "Offset_um" in header else None + for row in reader: + if len(row) >= 4: + try: + region_id = str(row[region_idx]) + x = float(row[x_idx]) + y = float(row[y_idx]) + z = float(row[z_idx]) + except (ValueError, IndexError): + continue + # Reject non-finite coordinates: float('nan')/'inf' parse fine but blow up + # later in FOV pixel mapping (round(nan)/round(inf)). + if not (math.isfinite(x) and math.isfinite(y) and math.isfinite(z)): + continue + points.append((region_id, x, y, z)) + if offset_idx is not None and offset_idx < len(row) and row[offset_idx] != "": + try: + offset_val = float(row[offset_idx]) + except ValueError: + offset_val = None + # Skip non-finite offsets — a NaN/inf target would make laser AF fail + # for every FOV in that region. + if offset_val is not None and math.isfinite(offset_val): + offsets[region_id] = offset_val + return points, offsets + def export_focus_points(self): """Export focus points to a CSV file""" if not self.focus_points: @@ -10739,65 +10985,27 @@ def export_focus_points(self): file_path += ".csv" try: - with open(file_path, "w", newline="") as csvfile: - writer = csv.writer(csvfile) - # Write header - writer.writerow(["Region_ID", "X_mm", "Y_mm", "Z_um"]) - - # Write data - for region_id, x, y, z in self.focus_points: - writer.writerow([region_id, x, y, z]) - + self._write_focus_points_csv(file_path) self.status_label.setText(f"Exported {len(self.focus_points)} points to {file_path}") - except Exception as e: QMessageBox.critical(self, "Export Error", f"Failed to export focus points: {str(e)}") def import_focus_points(self): - """Import focus points from a CSV file""" + """Import focus points (and optional per-region offsets) from a CSV file""" file_path, _ = QFileDialog.getOpenFileName(self, "Import Focus Points", "", "CSV Files (*.csv);;All Files (*)") - if not file_path: return + # Whole body guarded so a failure anywhere (parse, region-mismatch dialog, state + # assignment, display refresh) surfaces a dialog rather than only logging via the Qt + # excepthook and leaving the widget silently half-updated. try: - # Read the CSV file - imported_points = [] - with open(file_path, "r", newline="") as csvfile: - reader = csv.reader(csvfile) - header = next(reader) # Skip header row - - # Validate header - required_columns = ["Region_ID", "X_mm", "Y_mm", "Z_um"] - if not all(col in header for col in required_columns): - QMessageBox.warning( - self, "Invalid Format", f"CSV file must contain columns: {', '.join(required_columns)}" - ) - return - - # Get column indices - region_idx = header.index("Region_ID") - x_idx = header.index("X_mm") - y_idx = header.index("Y_mm") - z_idx = header.index("Z_um") - - # Read data - for row in reader: - if len(row) >= 4: - try: - region_id = str(row[region_idx]) - x = float(row[x_idx]) - y = float(row[y_idx]) - z = float(row[z_idx]) - imported_points.append((region_id, x, y, z)) - except (ValueError, IndexError): - continue + imported_points, imported_offsets = self._read_focus_points_csv(file_path) # If by_region is checked, validate regions if self.by_region_checkbox.isChecked(): scan_regions = set(self.scanCoordinates.region_centers.keys()) focus_regions = set(region_id for region_id, _, _, _ in imported_points) - if not focus_regions == scan_regions: response = QMessageBox.warning( self, @@ -10808,20 +11016,39 @@ def import_focus_points(self): QMessageBox.Ok | QMessageBox.Cancel, QMessageBox.Cancel, ) - if response == QMessageBox.Cancel: return else: # User chose to continue, uncheck by_region self.by_region_checkbox.setChecked(False) + # Imported offsets are relative to the laser-AF reference they were captured + # against, which the CSV does not record. Only apply them if a reference is set + # now, and flag that they must match it (there is no way to verify automatically). + offset_note = "" + if imported_offsets: + has_reference = ( + self.laserAutofocusController is not None + and self.laserAutofocusController.laser_af_properties.has_reference + ) + if has_reference: + self._offsets_reference_x = self.laserAutofocusController.laser_af_properties.x_reference + offset_note = ( + f"; {len(imported_offsets)} laser AF offset(s) loaded — verify they match " + f"the current laser AF reference" + ) + else: + imported_offsets = {} + offset_note = "; laser AF offsets ignored (no reference set)" + # Clear existing points and add imported ones self.focus_points = imported_points + self.region_laser_af_offsets = imported_offsets self.update_point_list() self.update_focus_point_display() - - self.status_label.setText(f"Imported {len(imported_points)} focus points") - + self.status_label.setText(f"Imported {len(imported_points)} focus points{offset_note}") + except ValueError as e: + QMessageBox.warning(self, "Invalid Format", str(e)) except Exception as e: QMessageBox.critical(self, "Import Error", f"Failed to import focus points: {str(e)}") diff --git a/software/docs/per-region-laser-af-offset.md b/software/docs/per-region-laser-af-offset.md new file mode 100644 index 000000000..03596ba1e --- /dev/null +++ b/software/docs/per-region-laser-af-offset.md @@ -0,0 +1,63 @@ +# Per-region laser-AF offset + +Focus each well/region at its **own height relative to the laser-AF reference plane**, +while laser autofocus keeps that height stable against drift over a long acquisition. + +Normally, when laser reflection autofocus (laser AF) is on, every field of view is driven +back to the **single** laser-AF reference plane. This feature instead lets each region +keep the focus offset you recorded for it — useful when different wells need slightly +different focus relative to the reference (e.g. sample-height differences across a plate). + +## When it applies + +The feature is active only when **all** of these are true: + +- Laser AF (**Reflection AF**) is enabled for the acquisition. +- A **focus map** is used, set to **`constant`** method with **`Fit by Region`** checked + (i.e. one focus point per well/region — the "constant-z" case). +- The **`Laser AF Offset`** checkbox (in the Focus Map tab) is checked. + +If any of these is off, behavior is unchanged: laser AF drives every FOV to the single +reference plane. + +## How to use it + +1. **Set the laser-AF reference** once (the usual "Set Reference" step), with the sample at + a representative focus. All per-region offsets are measured relative to this plane. +2. Open the **Focus Map** tab. Set **Fitting Method** to `constant` and check **Fit by + Region**. The **`Laser AF Offset`** checkbox now becomes available — check it. +3. For **each well/region**, add one focus point: + - Navigate to the region and bring it into the focus you want. + - Click **Add** (or select the region's point and click **Update Z**). + - The current laser-AF displacement is recorded as that region's offset and shown on the + status line (e.g. `Region A1: Laser AF offset +2.30 µm`). The live view briefly pauses + while the reading is taken, then resumes. Problems are reported on the status line too: + no reference set, spot not detected, or an offset larger than the laser-AF range (that + region may fail AF during the run). +4. In the multipoint panel, enable **Reflection AF** and **Use Focus Map**. +5. Start the acquisition. At each FOV, laser AF drives the stage to that region's recorded + offset instead of to the shared reference plane. + +## Good to know + +- **Re-setting the laser-AF reference clears all captured offsets** — they were measured + against the old reference and are no longer valid. Re-record them after changing the + reference. +- **Changing focus points keeps offsets in sync**: removing a region's point drops its + offset; regenerating the focus grid clears them. +- **Save / reuse**: Export the focus points to CSV — the file includes an `Offset_um` + column. Importing restores both the points and their offsets. Older CSV files without + that column still import (with no offsets). +- **Per-channel Z-offsets still apply** on top of the per-region offset, exactly as before. +- The offset value is the laser-AF displacement (µm) from the reference plane at the focus + point, so focus tracks the reference as the sample drifts — even across time-lapse + timepoints. + +## Troubleshooting + +- **The `Laser AF Offset` checkbox is greyed out.** It only enables for a + `constant` + `Fit by Region` focus map. Pick those in the Focus Map tab. +- **Offsets don't seem to apply during the run.** Confirm **Reflection AF** is enabled in + the multipoint panel you started the run from, and that the checkbox is still checked. +- **A region keeps failing autofocus.** Its offset may exceed the laser-AF range; re-record + it closer to the reference plane, or move the reference. diff --git a/software/docs/superpowers/plans/2026-06-18-per-region-laser-af-offset.md b/software/docs/superpowers/plans/2026-06-18-per-region-laser-af-offset.md new file mode 100644 index 000000000..29d87dbd7 --- /dev/null +++ b/software/docs/superpowers/plans/2026-06-18-per-region-laser-af-offset.md @@ -0,0 +1,831 @@ +# Per-region laser-AF offset Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Let each well/region be focused at its own offset from the single global laser-AF reference plane, by capturing that offset at the focus point and driving laser AF to it during acquisition (instead of always to displacement 0). + +**Architecture:** A `region_laser_af_offsets: dict[region_id → µm]` is captured in `FocusMapWidget` when the user records a focus point (one per well, the constant-z case), threaded to `MultiPointController` → `AcquisitionParameters` → `MultiPointWorker`, and consumed by changing the single `move_to_target(0)` call in `perform_autofocus` to `move_to_target(offset_for_region)`. The mode is gated behind an explicit checkbox that requires both Reflection AF and Use Focus Map. The focus map's absolute z stays as the coarse pre-position (no change to z-baking); the per-channel z-offset still composes additively because it is relative to wherever laser AF lands. + +**Tech Stack:** Python 3, PyQt5, pytest. Hardware-abstraction via `squid/` simulated backends. No new dependencies. + +Full design: `docs/superpowers/specs/2026-06-18-per-region-laser-af-offset-design.md`. + +## Global Constraints + +- Black line length is **120** (`pyproject.toml`); run `black --config pyproject.toml .` before each commit. +- Logging: GUI status via `self.status_label.setText(...)` (existing `FocusMapWidget` pattern); worker logs via `self._log`. +- Tests run under pytest/pytest-qt; CI command: `python3 -m pytest --ignore=tests/control/test_HighContentScreeningGui.py`. +- Branch for this work: `per-region-laser-af-offset` (already created off `master`, design spec already committed). +- Offset is **µm displacement from the global laser-AF reference**, equal to `LaserAutofocusController.measure_displacement()` at the focus z. Empty dict → current behavior (target 0 everywhere). +- Mode is active only when ALL of: `checkbox_useFocusMap` ON, `checkbox_withReflectionAutofocus` ON, and `FocusMapWidget.checkbox_perRegionLaserAFOffset` ON (a Qt checkbox stays checked while disabled — verify all three at acquisition start). +- All feature unit tests live in `tests/control/test_per_region_laser_af_offset.py`. + +--- + +### Task 1: Backend — thread offsets through params and apply them in the worker + +**Files:** +- Modify: `control/core/multi_point_utils.py:1` (import) and `:28-65` (`AcquisitionParameters`) +- Modify: `control/core/multi_point_controller.py:228` (field), `:415` (setter), `:935-961` (`build_params`) +- Modify: `control/core/multi_point_worker.py:120-122` (unpack), `:1157-1158` (apply) +- Test: `tests/control/test_per_region_laser_af_offset.py` + +**Interfaces:** +- Produces: `AcquisitionParameters.region_laser_af_offsets: Dict[str, float]` (default `{}`); `MultiPointController.set_region_laser_af_offsets(offsets)`; `MultiPointWorker.region_laser_af_offsets`; `MultiPointWorker.perform_autofocus(region_id, fov)` now targets the region's offset. +- Consumes (existing): `MultiPointWorker.laser_auto_focus_controller.move_to_target(target_um)` (already supports arbitrary targets). + +- [ ] **Step 1: Write the failing test (dataclass field + default)** + +Create `tests/control/test_per_region_laser_af_offset.py`: + +```python +"""Unit tests for the per-region laser-AF offset feature. + +Backend tests construct minimal MultiPointWorker-/FocusMapWidget-shaped stubs and +call the real methods in isolation, mirroring tests/control/test_MultiPointWorker_offsets.py. +""" + +import math +from dataclasses import fields +from unittest.mock import MagicMock + +from control.core.multi_point_utils import AcquisitionParameters +from control.core.multi_point_worker import MultiPointWorker + + +def test_acquisition_parameters_has_region_offsets_field(): + names = {f.name for f in fields(AcquisitionParameters)} + assert "region_laser_af_offsets" in names + + +def test_region_offsets_default_factory_is_empty_dict(): + fld = next(f for f in fields(AcquisitionParameters) if f.name == "region_laser_af_offsets") + assert fld.default_factory() == {} +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `python3 -m pytest tests/control/test_per_region_laser_af_offset.py -v` +Expected: FAIL — `region_laser_af_offsets` not in fields (and `StopIteration`/AttributeError on the second test). + +- [ ] **Step 3: Add the field to `AcquisitionParameters`** + +In `control/core/multi_point_utils.py`, change line 1 import: + +```python +from dataclasses import dataclass, field +``` + +Then add, immediately after the `xy_mode` field (currently line 65) inside `AcquisitionParameters`: + +```python + # Per-region laser-AF target offsets (µm from the global laser-AF reference plane), + # keyed by region_id. Empty unless the focus-map + laser-AF combined mode is active, + # in which case each FOV in a region targets that region's offset instead of 0. + region_laser_af_offsets: Dict[str, float] = field(default_factory=dict) +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `python3 -m pytest tests/control/test_per_region_laser_af_offset.py -v` +Expected: PASS (2 passed). + +- [ ] **Step 5: Write the failing test (apply path)** + +Append to `tests/control/test_per_region_laser_af_offset.py`: + +```python +class _AFStub: + """MultiPointWorker-ish object with just what perform_autofocus's laser-AF branch reads.""" + + def __init__(self, offsets, move_result=True): + self.do_reflection_af = True + self.region_laser_af_offsets = offsets + self._log = MagicMock() + self.laser_auto_focus_controller = MagicMock() + self.laser_auto_focus_controller.move_to_target.return_value = move_result + self._laser_af_successes = 0 + self._laser_af_failures = 0 + # Only touched on the exception path: + self.base_path = "/tmp" + self.experiment_ID = "exp" + self.time_point = 0 + + perform_autofocus = MultiPointWorker.perform_autofocus + + +def test_perform_autofocus_uses_region_offset(): + w = _AFStub({"A1": 5.0}) + assert w.perform_autofocus("A1", 0) is True + w.laser_auto_focus_controller.move_to_target.assert_called_once_with(5.0) + assert w._laser_af_successes == 1 + + +def test_perform_autofocus_defaults_to_zero_for_unmapped_region(): + w = _AFStub({"A1": 5.0}) + assert w.perform_autofocus("B2", 0) is True + w.laser_auto_focus_controller.move_to_target.assert_called_once_with(0.0) + + +def test_perform_autofocus_failure_increments_and_returns_false(): + w = _AFStub({"A1": 5.0}, move_result=False) + assert w.perform_autofocus("A1", 0) is False + assert w._laser_af_failures == 1 +``` + +- [ ] **Step 6: Run test to verify it fails** + +Run: `python3 -m pytest tests/control/test_per_region_laser_af_offset.py -k perform_autofocus -v` +Expected: FAIL — `move_to_target` called with `0`, not `5.0` (current code hardcodes `move_to_target(0)`). + +- [ ] **Step 7: Apply the per-region target in `perform_autofocus`** + +In `control/core/multi_point_worker.py`, replace lines 1157-1158: + +```python + try: + af_succeeded = self.laser_auto_focus_controller.move_to_target(0) +``` + +with: + +```python + try: + target_um = self.region_laser_af_offsets.get(region_id, 0.0) + self._log.info(f"laser AF target for region '{region_id}': {target_um:.2f} µm") + af_succeeded = self.laser_auto_focus_controller.move_to_target(target_um) +``` + +- [ ] **Step 8: Run test to verify it passes** + +Run: `python3 -m pytest tests/control/test_per_region_laser_af_offset.py -v` +Expected: PASS (5 passed). + +- [ ] **Step 9: Wire the plumbing (controller field, setter, build_params, worker unpack)** + +In `control/core/multi_point_controller.py`, after line 228 (`self.focus_map = None`): + +```python + self.region_laser_af_offsets = {} +``` + +After `set_focus_map` (currently lines 415-416), add: + +```python + def set_region_laser_af_offsets(self, offsets): + # region_id -> µm offset from the global laser-AF reference plane. Empty dict means + # every FOV targets the reference (displacement 0), i.e. current behavior. + self.region_laser_af_offsets = dict(offsets) if offsets else {} +``` + +In `build_params`, inside the `AcquisitionParameters(...)` call (after `xy_mode=self.xy_mode,` at line 960): + +```python + region_laser_af_offsets=self.region_laser_af_offsets, +``` + +In `control/core/multi_point_worker.py`, after line 122 (`self.apply_channel_offset = acquisition_parameters.apply_channel_offset`): + +```python + self.region_laser_af_offsets = acquisition_parameters.region_laser_af_offsets +``` + +- [ ] **Step 10: Verify import smoke + format + full backend test** + +Run: `python3 -c "import control.core.multi_point_controller, control.core.multi_point_worker, control.core.multi_point_utils"` +Expected: no error. +Run: `black --config pyproject.toml control/core/multi_point_utils.py control/core/multi_point_controller.py control/core/multi_point_worker.py tests/control/test_per_region_laser_af_offset.py` +Run: `python3 -m pytest tests/control/test_per_region_laser_af_offset.py -v` +Expected: PASS (5 passed). + +- [ ] **Step 11: Commit** + +```bash +git add control/core/multi_point_utils.py control/core/multi_point_controller.py control/core/multi_point_worker.py tests/control/test_per_region_laser_af_offset.py +git commit -m "feat: apply per-region laser-AF target offset in multipoint worker + +Co-Authored-By: Claude Opus 4.8 (1M context) " +``` + +--- + +### Task 2: Capture logic in `FocusMapWidget` + +**Files:** +- Modify: `control/core/__init__`? No. Modify: `control/widgets.py` (`FocusMapWidget`, `__init__` ~10374-10392; helpers added near 10670) +- Test: `tests/control/test_per_region_laser_af_offset.py` + +**Interfaces:** +- Produces: `FocusMapWidget.region_laser_af_offsets: dict[str, float]`; `FocusMapWidget.capture_laser_af_offset_enabled: bool`; methods `_capture_region_offset(region_id)`, `_clear_region_offsets()`, `_sync_offsets_to_focus_points()`, `_on_laser_af_reference_changed(has_reference)`. +- Consumes: `self.laserAutofocusController` (may be `None`); its `.measure_displacement() -> float` and `.laser_af_properties.has_reference` / `.laser_af_range`. + +- [ ] **Step 1: Write the failing test (capture matrix + clear + sync)** + +Append to `tests/control/test_per_region_laser_af_offset.py`: + +```python +from control.widgets import FocusMapWidget + + +def _laser_controller(displacement, has_reference=True, laser_af_range=200.0): + c = MagicMock() + c.laser_af_properties.has_reference = has_reference + c.laser_af_properties.laser_af_range = laser_af_range + c.measure_displacement.return_value = displacement + return c + + +class _FMStub: + """FocusMapWidget-ish object exposing just what the capture/persistence helpers read.""" + + def __init__(self, *, enabled=True, controller=None, focus_points=None, offsets=None): + self.capture_laser_af_offset_enabled = enabled + self.laserAutofocusController = controller + self.focus_points = focus_points if focus_points is not None else [] + self.region_laser_af_offsets = offsets if offsets is not None else {} + self.status_label = MagicMock() + + _capture_region_offset = FocusMapWidget._capture_region_offset + _clear_region_offsets = FocusMapWidget._clear_region_offsets + _sync_offsets_to_focus_points = FocusMapWidget._sync_offsets_to_focus_points + + +def test_capture_stores_displacement_when_enabled(): + w = _FMStub(controller=_laser_controller(3.5)) + w._capture_region_offset("A1") + assert w.region_laser_af_offsets == {"A1": 3.5} + + +def test_capture_noop_when_mode_disabled(): + ctrl = _laser_controller(3.5) + w = _FMStub(enabled=False, controller=ctrl) + w._capture_region_offset("A1") + assert w.region_laser_af_offsets == {} + ctrl.measure_displacement.assert_not_called() + + +def test_capture_noop_when_no_controller(): + w = _FMStub(controller=None) + w._capture_region_offset("A1") + assert w.region_laser_af_offsets == {} + + +def test_capture_noop_when_no_reference(): + ctrl = _laser_controller(3.5, has_reference=False) + w = _FMStub(controller=ctrl) + w._capture_region_offset("A1") + assert w.region_laser_af_offsets == {} + ctrl.measure_displacement.assert_not_called() + + +def test_capture_does_not_store_nan(): + w = _FMStub(controller=_laser_controller(float("nan"))) + w._capture_region_offset("A1") + assert "A1" not in w.region_laser_af_offsets + + +def test_capture_stores_but_warns_when_out_of_range(): + w = _FMStub(controller=_laser_controller(500.0, laser_af_range=200.0)) + w._capture_region_offset("A1") + assert w.region_laser_af_offsets == {"A1": 500.0} + assert w.status_label.setText.called + + +def test_capture_replaces_stale_entry_when_disabled(): + # Re-capturing with mode off must not leave a stale value for that region. + w = _FMStub(enabled=False, controller=_laser_controller(3.5), offsets={"A1": 9.0}) + w._capture_region_offset("A1") + assert "A1" not in w.region_laser_af_offsets + + +def test_clear_region_offsets(): + w = _FMStub(offsets={"A1": 1.0, "B2": 2.0}) + w._clear_region_offsets() + assert w.region_laser_af_offsets == {} + + +def test_sync_drops_orphaned_offsets(): + w = _FMStub(focus_points=[("A1", 0.0, 0.0, 1.0)], offsets={"A1": 1.0, "B2": 2.0}) + w._sync_offsets_to_focus_points() + assert w.region_laser_af_offsets == {"A1": 1.0} +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `python3 -m pytest tests/control/test_per_region_laser_af_offset.py -k "capture or clear or sync" -v` +Expected: FAIL — `AttributeError: type object 'FocusMapWidget' has no attribute '_capture_region_offset'`. + +- [ ] **Step 3: Add capture state to `FocusMapWidget.__init__`** + +In `control/widgets.py`, change the constructor signature (line 10374): + +```python + def __init__(self, stage: AbstractStage, navigationViewer, scanCoordinates, focusMap, laserAutofocusController=None): +``` + +After `self.focusMap = focusMap` (line 10383) add: + +```python + self.laserAutofocusController = laserAutofocusController +``` + +After `self.enabled = False # toggled when focus map enabled for next acquisition` (line 10387) add: + +```python + # Per-region laser-AF offsets (µm from the global laser-AF reference), keyed by + # region_id. Captured at each focus point when the combined mode is enabled. + self.region_laser_af_offsets = {} + self.capture_laser_af_offset_enabled = False + self._reflection_af_available = False +``` + +At the end of `__init__` (after `self.add_margin = True`, line 10392) add: + +```python + if self.laserAutofocusController is not None: + self.laserAutofocusController.signal_reference_changed.connect(self._on_laser_af_reference_changed) +``` + +- [ ] **Step 4: Add the capture/clear/sync helper methods** + +In `control/widgets.py`, add these methods to `FocusMapWidget` (e.g. immediately after `get_region_points_dict`, around line 10678): + +```python + def _capture_region_offset(self, region_id): + """Record the laser-AF displacement at the current z as this region's offset. + + No-op (and clears any stale entry for region_id) unless the combined mode is + enabled, a laser-AF controller exists, and a reference is set. Never stores NaN + (failed spot detection). Warns — but still stores — when the offset exceeds the + laser-AF range, since that region would fail AF during acquisition. + """ + self.region_laser_af_offsets.pop(region_id, None) + if not self.capture_laser_af_offset_enabled or self.laserAutofocusController is None: + return + if not self.laserAutofocusController.laser_af_properties.has_reference: + self.status_label.setText("Laser AF reference not set — per-region offset not captured") + return + offset_um = self.laserAutofocusController.measure_displacement() + if math.isnan(offset_um): + self.status_label.setText(f"Laser AF spot not detected — offset not captured for {region_id}") + return + laser_af_range = self.laserAutofocusController.laser_af_properties.laser_af_range + if abs(offset_um) > laser_af_range: + self.status_label.setText( + f"Warning: region {region_id} offset {offset_um:.1f} µm exceeds laser AF range " + f"({laser_af_range:.1f} µm); it may fail AF during acquisition" + ) + self.region_laser_af_offsets[region_id] = offset_um + + def _clear_region_offsets(self): + self.region_laser_af_offsets = {} + + def _sync_offsets_to_focus_points(self): + """Drop offsets for regions that no longer have any focus point.""" + live_regions = {rid for rid, _, _, _ in self.focus_points} + for rid in list(self.region_laser_af_offsets.keys()): + if rid not in live_regions: + self.region_laser_af_offsets.pop(rid, None) + + def _on_laser_af_reference_changed(self, has_reference): + # The reference plane moved; previously-captured offsets are relative to the old + # reference and are now meaningless. Clear them so they cannot be applied stale. + if self.region_laser_af_offsets: + self._clear_region_offsets() + self.status_label.setText("Laser AF reference changed — captured per-region offsets cleared") +``` + +- [ ] **Step 5: Hook capture into the focus-point lifecycle** + +In `add_current_point`, after `self.navigationViewer.register_focus_point(pos.x_mm, pos.y_mm)` (line 10636) add: + +```python + self._capture_region_offset(region_id) +``` + +In `update_current_z`, after `self.update_point_list()` (line 10670) add: + +```python + self._capture_region_offset(region_id) +``` + +Replace `remove_current_point` (lines 10640-10645) with: + +```python + def remove_current_point(self): + index = self.point_combo.currentIndex() + if 0 <= index < len(self.focus_points): + self.focus_points.pop(index) + self._sync_offsets_to_focus_points() + self.update_point_list() + self.update_focus_point_display() +``` + +In `generate_grid`, after `self.focus_points.clear()` (line 10570) add: + +```python + self._clear_region_offsets() +``` + +- [ ] **Step 6: Run test to verify it passes** + +Run: `python3 -m pytest tests/control/test_per_region_laser_af_offset.py -k "capture or clear or sync" -v` +Expected: PASS (9 passed). + +- [ ] **Step 7: Format + full feature test** + +Run: `black --config pyproject.toml control/widgets.py tests/control/test_per_region_laser_af_offset.py` +Run: `python3 -m pytest tests/control/test_per_region_laser_af_offset.py -v` +Expected: PASS (14 passed total). + +- [ ] **Step 8: Commit** + +```bash +git add control/widgets.py tests/control/test_per_region_laser_af_offset.py +git commit -m "feat: capture per-region laser-AF offset in FocusMapWidget + +Co-Authored-By: Claude Opus 4.8 (1M context) " +``` + +--- + +### Task 3: Persist offsets in the focus-points CSV + +**Files:** +- Modify: `control/widgets.py` (`FocusMapWidget.export_focus_points` 10729-10754, `import_focus_points` 10756-10826; add two pure helpers) +- Test: `tests/control/test_per_region_laser_af_offset.py` + +**Interfaces:** +- Produces: `FocusMapWidget._write_focus_points_csv(file_path)`, `FocusMapWidget._read_focus_points_csv(file_path) -> (points, offsets)`. + +- [ ] **Step 1: Write the failing test (CSV round-trip incl. offsets + back-compat)** + +Append to `tests/control/test_per_region_laser_af_offset.py`: + +```python +def test_csv_roundtrip_includes_offsets(tmp_path): + src = _FMStub( + focus_points=[("A1", 1.0, 2.0, 0.5), ("B2", 3.0, 4.0, 0.6)], + offsets={"A1": 7.0}, # B2 intentionally has no offset + ) + src._write_focus_points_csv = FocusMapWidget._write_focus_points_csv.__get__(src) + path = str(tmp_path / "fp.csv") + src._write_focus_points_csv(path) + + dst = _FMStub() + dst._read_focus_points_csv = FocusMapWidget._read_focus_points_csv.__get__(dst) + points, offsets = dst._read_focus_points_csv(path) + assert points == [("A1", 1.0, 2.0, 0.5), ("B2", 3.0, 4.0, 0.6)] + assert offsets == {"A1": 7.0} + + +def test_csv_read_back_compat_without_offset_column(tmp_path): + path = tmp_path / "legacy.csv" + path.write_text("Region_ID,X_mm,Y_mm,Z_um\nA1,1.0,2.0,0.5\n") + dst = _FMStub() + dst._read_focus_points_csv = FocusMapWidget._read_focus_points_csv.__get__(dst) + points, offsets = dst._read_focus_points_csv(str(path)) + assert points == [("A1", 1.0, 2.0, 0.5)] + assert offsets == {} + + +def test_csv_read_rejects_missing_required_columns(tmp_path): + path = tmp_path / "bad.csv" + path.write_text("Region_ID,X_mm\nA1,1.0\n") + dst = _FMStub() + dst._read_focus_points_csv = FocusMapWidget._read_focus_points_csv.__get__(dst) + import pytest + + with pytest.raises(ValueError): + dst._read_focus_points_csv(str(path)) +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `python3 -m pytest tests/control/test_per_region_laser_af_offset.py -k csv -v` +Expected: FAIL — `AttributeError: ... has no attribute '_write_focus_points_csv'`. + +- [ ] **Step 3: Add the pure CSV helpers** + +In `control/widgets.py`, add to `FocusMapWidget` (near the export/import methods): + +```python + def _write_focus_points_csv(self, file_path): + with open(file_path, "w", newline="") as csvfile: + writer = csv.writer(csvfile) + writer.writerow(["Region_ID", "X_mm", "Y_mm", "Z_um", "Offset_um"]) + for region_id, x, y, z in self.focus_points: + offset = self.region_laser_af_offsets.get(region_id, "") + writer.writerow([region_id, x, y, z, offset]) + + def _read_focus_points_csv(self, file_path): + """Parse a focus-points CSV. Returns (points, offsets). + + points: list of (region_id, x, y, z). offsets: {region_id: float} for rows that + carry a non-empty Offset_um (column optional, for back-compat). Raises ValueError + if any required column is missing. + """ + points = [] + offsets = {} + with open(file_path, "r", newline="") as csvfile: + reader = csv.reader(csvfile) + header = next(reader) + required_columns = ["Region_ID", "X_mm", "Y_mm", "Z_um"] + if not all(col in header for col in required_columns): + raise ValueError(f"CSV file must contain columns: {', '.join(required_columns)}") + region_idx = header.index("Region_ID") + x_idx = header.index("X_mm") + y_idx = header.index("Y_mm") + z_idx = header.index("Z_um") + offset_idx = header.index("Offset_um") if "Offset_um" in header else None + for row in reader: + if len(row) >= 4: + try: + region_id = str(row[region_idx]) + x = float(row[x_idx]) + y = float(row[y_idx]) + z = float(row[z_idx]) + except (ValueError, IndexError): + continue + points.append((region_id, x, y, z)) + if offset_idx is not None and offset_idx < len(row) and row[offset_idx] != "": + try: + offsets[region_id] = float(row[offset_idx]) + except ValueError: + pass + return points, offsets +``` + +- [ ] **Step 4: Route the dialog methods through the helpers** + +Replace the body of `export_focus_points` (the `try`/`except` block, lines 10741-10754) with: + +```python + try: + self._write_focus_points_csv(file_path) + self.status_label.setText(f"Exported {len(self.focus_points)} points to {file_path}") + except Exception as e: + QMessageBox.critical(self, "Export Error", f"Failed to export focus points: {str(e)}") +``` + +Replace the **entire** `import_focus_points` method (lines 10756-10826) with this version, which routes parsing through the helper and loads the offsets (the by-region validation flow is preserved verbatim): + +```python + def import_focus_points(self): + """Import focus points (and optional per-region offsets) from a CSV file""" + file_path, _ = QFileDialog.getOpenFileName(self, "Import Focus Points", "", "CSV Files (*.csv);;All Files (*)") + if not file_path: + return + + try: + imported_points, imported_offsets = self._read_focus_points_csv(file_path) + except ValueError as e: + QMessageBox.warning(self, "Invalid Format", str(e)) + return + except Exception as e: + QMessageBox.critical(self, "Import Error", f"Failed to import focus points: {str(e)}") + return + + # If by_region is checked, validate regions + if self.by_region_checkbox.isChecked(): + scan_regions = set(self.scanCoordinates.region_centers.keys()) + focus_regions = set(region_id for region_id, _, _, _ in imported_points) + if not focus_regions == scan_regions: + response = QMessageBox.warning( + self, + "Region Mismatch", + f"The imported focus points have regions: {', '.join(sorted(focus_regions))}\n\n" + f"Current scan has regions: {', '.join(sorted(scan_regions))}\n\n" + "Import anyway (disable 'By Region') or cancel?", + QMessageBox.Ok | QMessageBox.Cancel, + QMessageBox.Cancel, + ) + if response == QMessageBox.Cancel: + return + else: + # User chose to continue, uncheck by_region + self.by_region_checkbox.setChecked(False) + + # Clear existing points and add imported ones + self.focus_points = imported_points + self.region_laser_af_offsets = imported_offsets + self.update_point_list() + self.update_focus_point_display() + self.status_label.setText(f"Imported {len(imported_points)} focus points") +``` + +- [ ] **Step 5: Run test to verify it passes** + +Run: `python3 -m pytest tests/control/test_per_region_laser_af_offset.py -k csv -v` +Expected: PASS (3 passed). + +- [ ] **Step 6: Format + full feature test + import smoke** + +Run: `black --config pyproject.toml control/widgets.py tests/control/test_per_region_laser_af_offset.py` +Run: `python3 -c "import control.widgets"` +Run: `python3 -m pytest tests/control/test_per_region_laser_af_offset.py -v` +Expected: PASS (17 passed total). + +- [ ] **Step 7: Commit** + +```bash +git add control/widgets.py tests/control/test_per_region_laser_af_offset.py +git commit -m "feat: persist per-region laser-AF offsets in focus-points CSV + +Co-Authored-By: Claude Opus 4.8 (1M context) " +``` + +--- + +### Task 4: GUI wiring — checkbox, controller handle, multipoint connections, acquisition push + +This task is GUI integration (Qt-heavy); it is verified by a `--simulation` smoke test rather than unit tests. Implement all sub-steps, then run the smoke test before committing. + +**Files:** +- Modify: `control/widgets.py` (`FocusMapWidget.setup_ui` ~10456-10459, `make_connections` ~10486; new handlers; FlexibleMultiPointWidget `make_connections` ~6154 + init ~6188 + acquisition block 6480-6484; WellplateMultiPointWidget `make_connections` ~7693 + init ~7742 + acquisition block 8827-8838) +- Modify: `control/gui_hcs.py:976` (pass the controller) + +**Interfaces:** +- Consumes: `FocusMapWidget.set_reflection_af_available(bool)`, `FocusMapWidget.checkbox_perRegionLaserAFOffset`, `FocusMapWidget.region_laser_af_offsets`, `MultiPointController.set_region_laser_af_offsets`. + +- [ ] **Step 1: Add the checkbox to `FocusMapWidget.setup_ui`** + +In `control/widgets.py`, after `settings_layout.addWidget(self.by_region_checkbox)` (line 10458) add: + +```python + self.checkbox_perRegionLaserAFOffset = QCheckBox("Per-region laser AF offset") + self.checkbox_perRegionLaserAFOffset.setChecked(False) + self.checkbox_perRegionLaserAFOffset.setEnabled(False) + self.checkbox_perRegionLaserAFOffset.setToolTip( + "With laser AF and focus map both on: capture each region's offset from the laser AF " + "reference at its focus point, and drive laser AF to that per-region target during acquisition." + ) + settings_layout.addWidget(self.checkbox_perRegionLaserAFOffset) +``` + +- [ ] **Step 2: Wire the checkbox + add handlers** + +In `FocusMapWidget.make_connections`, after `self.fit_method_combo.currentTextChanged.connect(self._match_by_region_box)` (line 10486) add: + +```python + self.checkbox_perRegionLaserAFOffset.toggled.connect(self._on_per_region_offset_toggled) +``` + +Add these methods to `FocusMapWidget` (near the capture helpers from Task 2): + +```python + def _on_per_region_offset_toggled(self, checked): + self.capture_laser_af_offset_enabled = checked + if not checked: + self._clear_region_offsets() + + def set_reflection_af_available(self, available): + # The per-region offset only makes sense with laser AF on. Disable + uncheck (and + # via the toggle handler, clear) the checkbox when reflection AF is off — mirrors + # the per-channel offset checkbox behavior in the multipoint widgets. + self._reflection_af_available = bool(available) + self.checkbox_perRegionLaserAFOffset.setEnabled(self._reflection_af_available) + if not self._reflection_af_available and self.checkbox_perRegionLaserAFOffset.isChecked(): + self.checkbox_perRegionLaserAFOffset.setChecked(False) +``` + +- [ ] **Step 3: Pass the laser-AF controller into the widget** + +In `control/gui_hcs.py`, replace the `FocusMapWidget(...)` construction (lines 976-978): + +```python + self.focusMapWidget = widgets.FocusMapWidget( + self.stage, self.navigationViewer, self.scanCoordinates, core.FocusMap(), self.laserAutofocusController + ) +``` + +(`self.laserAutofocusController` is defined at gui_hcs.py:647 — `Optional`, `None` when `SUPPORT_LASER_AUTOFOCUS` is off — so this is safe; the widget tolerates `None`.) + +- [ ] **Step 4: Connect Reflection-AF availability from both multipoint widgets** + +In FlexibleMultiPointWidget `make_connections`, after line 6157 (`...connect(self._update_apply_channel_offset_enable_state)`) add: + +```python + self.checkbox_withReflectionAutofocus.toggled.connect(self.focusMapWidget.set_reflection_af_available) +``` + +After line 6188 (`self._update_apply_channel_offset_enable_state(self.checkbox_withReflectionAutofocus.isChecked())`) add: + +```python + self.focusMapWidget.set_reflection_af_available(self.checkbox_withReflectionAutofocus.isChecked()) +``` + +In WellplateMultiPointWidget `make_connections`, after line 7693 add the same `set_reflection_af_available` connect line, and after line 7742 add the same init call. + +- [ ] **Step 5: Push offsets at acquisition start (Flexible path)** + +In `control/widgets.py`, replace the Flexible focus-map block (lines 6480-6484): + +```python + if self.checkbox_useFocusMap.isChecked(): + self.focusMapWidget.fit_surface() + self.multipointController.set_focus_map(self.focusMapWidget.focusMap) + else: + self.multipointController.set_focus_map(None) +``` + +with: + +```python + if self.checkbox_useFocusMap.isChecked(): + self.focusMapWidget.fit_surface() + self.multipointController.set_focus_map(self.focusMapWidget.focusMap) + if ( + self.checkbox_withReflectionAutofocus.isChecked() + and self.focusMapWidget.checkbox_perRegionLaserAFOffset.isChecked() + ): + self.multipointController.set_region_laser_af_offsets(self.focusMapWidget.region_laser_af_offsets) + else: + self.multipointController.set_region_laser_af_offsets({}) + else: + self.multipointController.set_focus_map(None) + self.multipointController.set_region_laser_af_offsets({}) +``` + +- [ ] **Step 6: Push offsets at acquisition start (Wellplate path)** + +Replace the Wellplate focus-map block (lines 8827-8838): + +```python + if self.checkbox_useFocusMap.isChecked(): + # Try to fit the surface + if self.focusMapWidget.fit_surface(): + # If fit successful, set the surface fitter in controller + self.multipointController.set_focus_map(self.focusMapWidget.focusMap) + else: + QMessageBox.warning(self, "Warning", "Failed to fit focus surface") + self.btn_startAcquisition.setChecked(False) + return + else: + # If checkbox not checked, set surface fitter to None + self.multipointController.set_focus_map(None) +``` + +with: + +```python + if self.checkbox_useFocusMap.isChecked(): + # Try to fit the surface + if self.focusMapWidget.fit_surface(): + # If fit successful, set the surface fitter in controller + self.multipointController.set_focus_map(self.focusMapWidget.focusMap) + if ( + self.checkbox_withReflectionAutofocus.isChecked() + and self.focusMapWidget.checkbox_perRegionLaserAFOffset.isChecked() + ): + self.multipointController.set_region_laser_af_offsets( + self.focusMapWidget.region_laser_af_offsets + ) + else: + self.multipointController.set_region_laser_af_offsets({}) + else: + QMessageBox.warning(self, "Warning", "Failed to fit focus surface") + self.btn_startAcquisition.setChecked(False) + return + else: + # If checkbox not checked, set surface fitter to None + self.multipointController.set_focus_map(None) + self.multipointController.set_region_laser_af_offsets({}) +``` + +- [ ] **Step 7: Format + import smoke + full suite** + +Run: `black --config pyproject.toml control/widgets.py control/gui_hcs.py` +Run: `python3 -c "import control.widgets, control.gui_hcs"` +Expected: no error. +Run: `python3 -m pytest --ignore=tests/control/test_HighContentScreeningGui.py -q` +Expected: PASS (no regressions; the 17 feature tests included). + +- [ ] **Step 8: Manual `--simulation` smoke test** + +Run: `python3 main_hcs.py --simulation` +Verify by hand: +1. With Reflection AF OFF, the "Per-region laser AF offset" checkbox in the Focus Map panel is disabled. +2. Turn Reflection AF ON → checkbox becomes enabled. Turn it OFF → checkbox disables and unchecks. +3. With Use Focus Map ON + Reflection AF ON + the new checkbox ON, set a laser-AF reference, define one focus point per region (constant + Fit by Region, 1×1), and confirm Add/Update-Z populates `focusMapWidget.region_laser_af_offsets` (visible via the status label messages / a debugger). +4. Re-setting the laser-AF reference clears the captured offsets (status label message). +5. Export then import a focus-points CSV and confirm the `Offset_um` column round-trips. + +Note any deviations; none should occur. + +- [ ] **Step 9: Commit** + +```bash +git add control/widgets.py control/gui_hcs.py +git commit -m "feat: per-region laser-AF offset GUI wiring (checkbox + acquisition push) + +Co-Authored-By: Claude Opus 4.8 (1M context) " +``` + +--- + +## Notes for the implementer + +- Line numbers are from the current `per-region-laser-af-offset` branch HEAD; if a prior task shifted lines, re-locate by the quoted code, not the number. +- Do not touch `move_to_target`, the focus-map z-baking (`multi_point_controller.py:755-765`), or `_apply_channel_z_offset` — composition is by construction (the per-channel offset is relative to wherever laser AF lands, which now includes the per-region target). +- The third multipoint widget that uses `_ApplyChannelOffsetMixin` (widgets.py ~9433) has no focus map, so it needs no changes. diff --git a/software/docs/superpowers/specs/2026-06-18-per-region-laser-af-offset-design.md b/software/docs/superpowers/specs/2026-06-18-per-region-laser-af-offset-design.md new file mode 100644 index 000000000..880c2cd49 --- /dev/null +++ b/software/docs/superpowers/specs/2026-06-18-per-region-laser-af-offset-design.md @@ -0,0 +1,246 @@ +# Per-region laser-AF offset (focus-map constant-z + laser AF) + +- **Date:** 2026-06-18 +- **Status:** Approved (design) +- **Scope:** `software/` tree + +## Summary + +Let the user combine laser autofocus (laser AF) with a constant-z focus map so +that each well/region is focused at its **own offset relative to the single +global laser-AF reference plane**, rather than all wells being driven to that one +shared plane. + +Workflow: the user sets the laser-AF reference once, defines one focus point per +well (the existing focus-map "constant + Fit by Region, 1×1" case), and at each +well navigates to the desired focus z and records it. At that moment the system +captures the laser-AF displacement — `measure_displacement()` = `(x − +x_reference)·pixel_to_um` — and stores it as that region's offset. During +acquisition, laser AF at every FOV in that well drives the stage to the well's +stored offset (`move_to_target(offset)`) instead of to the global reference +(`move_to_target(0)`). + +This "pins" each well to its capture-time relationship with the reference plane; +laser AF then maintains that pin against z drift, including across time-lapse +timepoints (because the existing per-FOV laser-AF behavior is unchanged — only +its *target* changes). + +## Motivation + +Today the two subsystems can both be enabled, but they fight: + +- The focus map bakes an **absolute** stage z into every FOV + (`multi_point_controller.py:755-765`). Absolute z does not track sample/stage + drift over a long acquisition. +- Laser AF runs per FOV and drives the spot back to **one** global reference + plane (`move_to_target(0)` hardcoded at `multi_point_worker.py:1158`). It has + no per-well notion, so it overrides the focus map's per-well z by snapping + every well to the same plane. + +There is no way to say "well A1 should be focused 3 µm above the reference plane, +well B2 at −1 µm, and keep them there as the system drifts." This feature adds +exactly that, reusing existing machinery. + +## Background: current behavior (verified against code) + +### Laser AF — `control/core/laser_auto_focus_controller.py` +- Single global reference: `laser_af_properties.x_reference` / `has_reference`, + set once via `set_reference()` (line 395). `signal_reference_changed` is emitted + on change (line 464). +- `measure_displacement()` (line 302) returns `(x − x_reference)·pixel_to_um` in + µm, or `float('nan')` on failure (lines 319/335/339). +- `move_to_target(target_um)` (line 346): measures current displacement, then + moves `um_to_move = target_um − current_displacement_um` (line 372) so the spot + lands at `target_um` of displacement from reference. **Already supports an + arbitrary target** — no controller change needed. + - Returns `False` (soft failure, no raise) if: no reference (355), NaN + displacement (362), `|measured displacement| > laser_af_range` (366), or + cross-correlation mismatch (378). Note the range guard checks the *measured* + displacement, not `(measured − target)`. + +### Focus map — `control/core/core.py` (`FocusMap`) + `control/widgets.py` (`FocusMapWidget`) +- `FocusMapWidget` (widgets.py:10371) holds `focus_points: list[(region_id, x, y, + z)]` (10386). Capture points: `add_current_point` (10596) and + `update_current_z` (10664). Grid generation (`generate_grid`) replaces + `focus_points`. +- The "1 point per well" case is `method="constant"` + `Fit by Region` + 1×1 grid. + `fit_surface` (10680) validates `focus_regions == scan_regions` when by-region + (10688-10697) and enforces 1×1 for constant (10699-10705). +- On acquisition start the controller bakes absolute z into every FOV + (`multi_point_controller.py:755-765` via `scanCoordinates.update_fov_z_level`). + This stays as the **coarse pre-position** in the new mode (unchanged). + +### Per-channel z-offset — `control/core/multi_point_worker.py` +- `_apply_channel_z_offset` (1220) applies each channel's `z_offset_um` **relative + to wherever laser AF left the stage**, gated on `apply_channel_offset AND + do_reflection_af AND af_succeeded`, tracked in `_current_z_offset_um`, reset per + z-level (1256). Because it is relative to the laser-AF-anchored z, it composes + additively with the new per-region target **with no math change**. + +### How they combine today +Both `set_focus_map(focusMap)` and `set_reflection_af_flag(True)` can be set in +one run. Per-FOV z precedence: base z → focus-map absolute z (baked) → laser AF +relative correction to global plane (overrides focus-map z) → per-channel offset. +So the focus-map per-well z is effectively discarded the moment laser AF runs. + +## Goal / non-goals + +**Goal:** a per-region µm offset, captured in the Focus Map panel and applied as +the laser-AF target during acquisition, active only when explicitly enabled and +only for the constant 1-point-per-well focus map. + +**Non-goals (YAGNI):** +- No change to laser-AF per-FOV scheduling or timepoint behavior — laser AF runs + exactly as today; only its target changes. +- No combining offsets with non-constant (spline/RBF surface) focus maps. +- No "measure once per well and reuse for all FOVs" optimization. +- No change to `move_to_target`, to the focus-map z-baking, or to + `_apply_channel_z_offset` internals. + +## Design + +### State / data model +- A new dict `region_laser_af_offsets: dict[str, float]` (region_id → offset µm). +- **Owned by** `FocusMapWidget` (parallel to `focus_points`). Rationale: the + offset is a laser-AF concern captured in the Focus Map panel, not surface-fit + geometry, so it stays out of the `FocusMap` core class. (Approved over the + alternative of co-locating it with `FocusMap.region_surface_fits`.) +- Threaded to the worker via `AcquisitionParameters` (the focus map itself is not + passed to the worker; its z is pre-baked, but offsets must reach + `perform_autofocus`). + +### Capture — `FocusMapWidget` +1. Add a `laserAutofocusController` parameter to `FocusMapWidget.__init__` + (widgets.py:10374), passed from `gui_hcs.py:976` as + `self.laserAutofocusController` (which is `Optional` and may be `None` when + `SUPPORT_LASER_AUTOFOCUS` is off — already defined by gui_hcs.py:647, before + line 976). Store as `self.laserAutofocusController`. +2. New instance state: `self.region_laser_af_offsets: dict[str, float] = {}` and a + `self.capture_laser_af_offset_enabled: bool = False` flag. +3. Capture helper `_capture_region_offset(region_id)`: + - No-op (and clear any stale entry for that region) unless + `capture_laser_af_offset_enabled` is true, the controller is not `None`, and + `laser_af_properties.has_reference` is true. + - Call `measure_displacement()`. If `NaN` → warn (status label + log), do **not** + store. If `|offset| > laser_af_properties.laser_af_range` → warn the well will + fail AF at acquisition, but still store (user may re-target). + - Else store `region_laser_af_offsets[region_id] = offset`. +4. Call `_capture_region_offset(region_id)` at the end of `add_current_point` + (10633-10636) and `update_current_z` (10664-10670). +5. Keep offsets in sync with the focus-point lifecycle: + - `remove_current_point` (10640): drop the region's offset if that was its last + point. + - `generate_grid` (replaces `focus_points`) and any clear path: clear + `region_laser_af_offsets`. + - `import_focus_points`: load offsets if present (see Persistence). + +### Activation / UI +- New checkbox **in the Focus Map panel** (the shared `FocusMapWidget`), e.g. + `self.checkbox_perRegionLaserAFOffset = QCheckBox("Per-region laser AF offset")`, + added in `setup_ui` (near 10456). Its `toggled` sets + `self.capture_laser_af_offset_enabled` and updates the status hint. +- Enable-state: enabled only when (a) the panel is enabled (the panel is already + toggled by `checkbox_useFocusMap.toggled → focusMapWidget.setEnabled`, at + widgets.py:6154 and 7698) **and** (b) Reflection AF is on. Add a + `FocusMapWidget.set_reflection_af_available(bool)` method that stores the flag and + re-derives the checkbox `setEnabled`. Connect **both** multipoint widgets' + `checkbox_withReflectionAutofocus.toggled` to it (alongside the existing + connections at widgets.py:6156-6157 and 7695-7696), mirroring the + `_update_apply_channel_offset_enable_state` precedent. Set the initial state + during each widget's init (next to 6191 / 7745). +- When the checkbox is disabled or unchecked, capture is off and (see below) no + offsets are pushed to the controller — so behavior is identical to today. + +### Acquisition apply +1. `MultiPointController` (multi_point_controller.py): + - New field `self.region_laser_af_offsets: dict[str, float] = {}` near line 228. + - New setter `set_region_laser_af_offsets(self, offsets)` near line 415, + mirroring `set_focus_map`. + - In `build_params` (921), pass `region_laser_af_offsets=self.region_laser_af_offsets` + into the new `AcquisitionParameters` field. +2. `AcquisitionParameters` (multi_point_utils.py:29): add + `region_laser_af_offsets: Dict[str, float]` (default `field(default_factory=dict)` + if the dataclass uses defaults; otherwise add to the constructor call in + `build_params` and to the field list). +3. `MultiPointWorker` (multi_point_worker.py): unpack + `self.region_laser_af_offsets = params.region_laser_af_offsets` in `__init__` + (next to where `self.do_reflection_af` is set, ~line 120). +4. `perform_autofocus` (multi_point_worker.py:1158): replace + `self.laser_auto_focus_controller.move_to_target(0)` with + `self.laser_auto_focus_controller.move_to_target(self.region_laser_af_offsets.get(region_id, 0.0))`. + `region_id` is already a parameter (1133). Log the target used; a missing + region defaults to `0.0` (current behavior). +5. GUI gating — both acquisition-start blocks. The mode is active only when **all + three** hold: `checkbox_useFocusMap.isChecked()` **and** + `checkbox_withReflectionAutofocus.isChecked()` **and** + `focusMapWidget.checkbox_perRegionLaserAFOffset.isChecked()`. (A Qt checkbox + stays checked while disabled, so checking only the mode box is not sufficient — + all three must be verified at start.) + - Flexible path (widgets.py:6480-6493): inside the existing + `if self.checkbox_useFocusMap.isChecked():` block, when the other two + conditions also hold, call + `self.multipointController.set_region_laser_af_offsets(self.focusMapWidget.region_laser_af_offsets)`; + in every other branch call `set_region_laser_af_offsets({})`. + - Wellplate path (widgets.py:8830-8849): same addition. + - Optionally warn at start if the mode is on but some scan regions lack an + offset (those FOVs fall back to target 0). + +### Correctness / edge cases +- **Reference invalidation:** connect `laserAutofocusController.signal_reference_changed` + (laser_auto_focus_controller.py:464) to a `FocusMapWidget` slot that clears + `region_laser_af_offsets` and shows a status message. This covers the auto-reset + on z-range edits (`widgets.py:6285` calls `set_reference`), which otherwise would + silently invalidate every captured offset. +- **No controller / feature off:** all capture is a clean no-op when the controller + is `None` (`SUPPORT_LASER_AUTOFOCUS` off) or the mode checkbox is off. +- **NaN / no reference at capture:** never store; warn. +- **Range at acquisition:** unchanged `move_to_target` range guard; an out-of-range + measured displacement returns `False` → that FOV's AF fails (existing path: + `_laser_af_failures += 1`, per-channel offset skipped). Documented, not changed. +- **Composition with per-channel offset:** unchanged; additive by construction. + +### Persistence +- Extend `export_focus_points` to write an optional `offset_um` column and + `import_focus_points` to read it when present (back-compatible: files without the + column import as before, with empty offsets). Keep this minimal; it is the only + persistence added. + +## File-by-file change list + +| File | Change | +|---|---| +| `control/gui_hcs.py:976` | Pass `self.laserAutofocusController` to `FocusMapWidget(...)`. | +| `control/widgets.py` (`FocusMapWidget`, ~10371-10720) | New ctor param + state; `checkbox_perRegionLaserAFOffset`; `_capture_region_offset`; hook `add_current_point`/`update_current_z`/`remove_current_point`/`generate_grid`/import-export; `set_reflection_af_available`; `signal_reference_changed` handler. | +| `control/widgets.py` (FlexibleMultiPointWidget) | Connect `checkbox_withReflectionAutofocus.toggled` → `focusMapWidget.set_reflection_af_available` (~6156); init enable state (~6191); push offsets at acquisition start (~6480-6493). | +| `control/widgets.py` (WellplateMultiPointWidget) | Same connections (~7695), init (~7745), and push (~8830-8849). | +| `control/core/multi_point_controller.py` | `region_laser_af_offsets` field (~228); `set_region_laser_af_offsets` (~415); pass through `build_params` (~921). | +| `control/core/multi_point_utils.py:29` | Add `region_laser_af_offsets: Dict[str, float]` to `AcquisitionParameters`. | +| `control/core/multi_point_worker.py` | Unpack field in `__init__` (~120); use per-region target at `perform_autofocus` (1158). | + +## Testing + +Use simulated backends (`tests/tools.py`, `tests/control/conftest.py`): +- **Capture:** with a stub laser-AF controller returning a known displacement, + `add_current_point`/`update_current_z` store the offset per region; `NaN` + and no-reference cases store nothing; controller `None` is a no-op. +- **Apply:** `MultiPointWorker.perform_autofocus(region_id, fov)` calls + `move_to_target` with the region's offset (and `0.0` for an unmapped region). + Mock `laser_auto_focus_controller.move_to_target` and assert the argument. +- **Invalidation:** emitting `signal_reference_changed` clears stored offsets. +- **Plumbing:** `set_region_laser_af_offsets` → `build_params` → + `AcquisitionParameters` → worker field round-trips. +- **Regression:** mode off / unmapped regions → `move_to_target(0)` (no behavior + change). Per-channel offset math unchanged when a region offset is present. + +Run: `python3 -m pytest --ignore=tests/control/test_HighContentScreeningGui.py` +plus `black --config pyproject.toml --check .` + +## Risks + +- Two multipoint widgets share one `FocusMapWidget`; the reflection-AF enable + wiring must be connected from **both** (only one is active at a time). +- The laser-AF range guard is against measured-from-reference displacement, so a + large per-well offset plus drift can exceed `laser_af_range` and silently fail AF + for that FOV. Mitigated by the capture-time `|offset| > laser_af_range` warning. +- `measure_displacement()` toggles the AF laser and adds latency at each capture; + acceptable because capture is a manual, per-well setup action. diff --git a/software/tests/control/test_MultiPointWorker_offsets.py b/software/tests/control/test_MultiPointWorker_offsets.py index c0ad996ec..da604eff5 100644 --- a/software/tests/control/test_MultiPointWorker_offsets.py +++ b/software/tests/control/test_MultiPointWorker_offsets.py @@ -360,6 +360,9 @@ def _af_stub(*, move_to_target_result=True, move_to_target_raises=False): w.laser_auto_focus_controller.move_to_target.return_value = move_to_target_result w._laser_af_successes = 0 w._laser_af_failures = 0 + # perform_autofocus reads this per-region target map (empty -> target 0.0, the + # pre-existing behavior); the real worker sets it in __init__. + w.region_laser_af_offsets = {} w.perform_autofocus = MultiPointWorker.perform_autofocus.__get__(w) return w diff --git a/software/tests/control/test_per_region_laser_af_offset.py b/software/tests/control/test_per_region_laser_af_offset.py new file mode 100644 index 000000000..1c2f53216 --- /dev/null +++ b/software/tests/control/test_per_region_laser_af_offset.py @@ -0,0 +1,437 @@ +"""Unit tests for the per-region laser-AF offset feature. + +Backend tests construct minimal MultiPointWorker-/FocusMapWidget-shaped stubs and +call the real methods in isolation, mirroring tests/control/test_MultiPointWorker_offsets.py. +""" + +import pytest +from dataclasses import fields +from unittest.mock import MagicMock + +from control.core.multi_point_utils import AcquisitionParameters, ScanPositionInformation +from control.core.multi_point_controller import MultiPointController +from control.core.multi_point_worker import MultiPointWorker +from control.widgets import FocusMapWidget + + +def test_acquisition_parameters_has_region_offsets_field(): + names = {f.name for f in fields(AcquisitionParameters)} + assert "region_laser_af_offsets" in names + + +class _BuildParamsStub: + """MultiPointController-shaped stub exercising build_params' offset threading in isolation.""" + + def __init__(self): + self.scanCoordinates = object() # no 'format' attr -> default plate dims, no wellplate lookup + self._log = MagicMock() + self.experiment_ID = "exp" + self.base_path = "/tmp" + self.selected_configurations = [] + self.timestamp_acquisition_started = 0.0 + self.NX = self.NY = self.NZ = self.Nt = 1 + self.deltaX = self.deltaY = self.deltaZ = self.deltat = 0.0 + self.do_autofocus = False + self.do_reflection_af = True + self.apply_channel_offset = True + self.use_piezo = False + self.display_resolution_scaling = 1.0 + self.z_stacking_config = "FROM CENTER" + self.z_range = (0.0, 0.0) + self.use_fluidics = False + self.skip_saving = False + self.xy_mode = "Current Position" + + build_params = MultiPointController.build_params + + +def _empty_scan_position_information(): + return ScanPositionInformation(scan_region_coords_mm=[], scan_region_names=[], scan_region_fov_coords_mm={}) + + +def test_build_params_threads_region_offsets(): + params = _BuildParamsStub().build_params(_empty_scan_position_information(), region_laser_af_offsets={"A1": 4.0}) + assert params.region_laser_af_offsets == {"A1": 4.0} + + +def test_build_params_defaults_offsets_to_empty_when_not_passed(): + # run_acquisition clears self.region_laser_af_offsets up-front and passes the snapshot + # explicitly; a None/absent argument must yield {} (no leaked sticky state). + params = _BuildParamsStub().build_params(_empty_scan_position_information()) + assert params.region_laser_af_offsets == {} + + +def test_region_offsets_default_factory_is_empty_dict(): + fld = next(f for f in fields(AcquisitionParameters) if f.name == "region_laser_af_offsets") + assert fld.default_factory() == {} + + +class _AFStub: + """MultiPointWorker-ish object with just what perform_autofocus's laser-AF branch reads.""" + + def __init__(self, offsets, move_result=True): + self.do_reflection_af = True + self.region_laser_af_offsets = offsets + self._log = MagicMock() + self.laser_auto_focus_controller = MagicMock() + self.laser_auto_focus_controller.move_to_target.return_value = move_result + self._laser_af_successes = 0 + self._laser_af_failures = 0 + # Only touched on the exception path: + self.base_path = "/tmp" + self.experiment_ID = "exp" + self.time_point = 0 + + perform_autofocus = MultiPointWorker.perform_autofocus + + +def test_perform_autofocus_uses_region_offset(): + # Anchor closed-loop at the reference (move_to_target(0.0)), then apply the per-region + # offset open-loop — driving move_to_target to a nonzero displacement would fail the + # reference-crop verification and revert. + w = _AFStub({"A1": 5.0}) + assert w.perform_autofocus("A1", 0) is True + w.laser_auto_focus_controller.move_to_target.assert_called_once_with(0.0) + w.laser_auto_focus_controller.apply_relative_offset_um.assert_called_once_with(5.0) + assert w._laser_af_successes == 1 + + +def test_perform_autofocus_defaults_to_zero_for_unmapped_region(): + w = _AFStub({"A1": 5.0}) + assert w.perform_autofocus("B2", 0) is True + w.laser_auto_focus_controller.move_to_target.assert_called_once_with(0.0) + # No offset for B2 → no open-loop move. + w.laser_auto_focus_controller.apply_relative_offset_um.assert_not_called() + + +def test_perform_autofocus_offset_not_applied_when_anchor_fails(): + # If anchoring at the reference fails, the open-loop offset must NOT be applied. + w = _AFStub({"A1": 5.0}, move_result=False) + assert w.perform_autofocus("A1", 0) is False + w.laser_auto_focus_controller.apply_relative_offset_um.assert_not_called() + assert w._laser_af_failures == 1 + + +def _laser_controller(displacement, has_reference=True, laser_af_range=200.0): + c = MagicMock() + c.laser_af_properties.has_reference = has_reference + c.laser_af_properties.laser_af_range = laser_af_range + c.measure_displacement.return_value = displacement + return c + + +class _FMStub: + """FocusMapWidget-ish object exposing just what the capture/persistence helpers read.""" + + def __init__(self, *, enabled=True, controller=None, focus_points=None, offsets=None, live=False): + self.capture_laser_af_offset_enabled = enabled + self.laserAutofocusController = controller + self.focus_points = focus_points if focus_points is not None else [] + self.region_laser_af_offsets = offsets if offsets is not None else {} + self.status_label = MagicMock() + self._log = MagicMock() + self._offsets_reference_x = None + self.liveController = MagicMock() + self.liveController.is_live = live + self.checkbox_perRegionLaserAFOffset = MagicMock() + self.fit_method_combo = MagicMock() + self.fit_method_combo.currentText.return_value = "constant" + self.by_region_checkbox = MagicMock() + self.by_region_checkbox.isChecked.return_value = True + + _capture_region_offset = FocusMapWidget._capture_region_offset + _clear_region_offsets = FocusMapWidget._clear_region_offsets + _sync_offsets_to_focus_points = FocusMapWidget._sync_offsets_to_focus_points + _on_laser_af_reference_changed = FocusMapWidget._on_laser_af_reference_changed + _on_per_region_offset_toggled = FocusMapWidget._on_per_region_offset_toggled + _update_per_region_offset_enabled = FocusMapWidget._update_per_region_offset_enabled + get_offsets_for_acquisition = FocusMapWidget.get_offsets_for_acquisition + + +def test_capture_stores_displacement_when_enabled(): + w = _FMStub(controller=_laser_controller(3.5)) + w._capture_region_offset("A1") + assert w.region_laser_af_offsets == {"A1": 3.5} + + +def test_capture_shows_offset_on_status_line(): + # On a successful in-range capture, the focus-map status line reports the offset. + w = _FMStub(controller=_laser_controller(2.3)) + w._capture_region_offset("A1") + msg = w.status_label.setText.call_args[0][0] + assert "A1" in msg and "2.3" in msg + + +def test_capture_suspends_main_live_during_measurement(): + # measure_displacement contends with the main live stream on the microcontroller serial + # link; capture must stop live around the measurement and restore it after. + w = _FMStub(controller=_laser_controller(2.3), live=True) + w._capture_region_offset("A1") + w.liveController.stop_live.assert_called_once() + w.liveController.start_live.assert_called_once() + assert w.region_laser_af_offsets == {"A1": 2.3} + + +def test_capture_does_not_toggle_live_when_not_live(): + w = _FMStub(controller=_laser_controller(2.3), live=False) + w._capture_region_offset("A1") + w.liveController.stop_live.assert_not_called() + w.liveController.start_live.assert_not_called() + + +def test_capture_noop_when_mode_disabled(): + ctrl = _laser_controller(3.5) + w = _FMStub(enabled=False, controller=ctrl) + w._capture_region_offset("A1") + assert w.region_laser_af_offsets == {} + ctrl.measure_displacement.assert_not_called() + + +def test_capture_noop_when_no_controller(): + w = _FMStub(controller=None) + w._capture_region_offset("A1") + assert w.region_laser_af_offsets == {} + + +def test_capture_noop_when_no_reference(): + ctrl = _laser_controller(3.5, has_reference=False) + w = _FMStub(controller=ctrl) + w._capture_region_offset("A1") + assert w.region_laser_af_offsets == {} + ctrl.measure_displacement.assert_not_called() + + +def test_capture_does_not_store_nan(): + w = _FMStub(controller=_laser_controller(float("nan"))) + w._capture_region_offset("A1") + assert "A1" not in w.region_laser_af_offsets + + +def test_capture_stores_but_warns_when_out_of_range(): + w = _FMStub(controller=_laser_controller(500.0, laser_af_range=200.0)) + w._capture_region_offset("A1") + assert w.region_laser_af_offsets == {"A1": 500.0} + assert w.status_label.setText.called + assert "exceeds" in w.status_label.setText.call_args[0][0] + + +def test_capture_replaces_stale_entry_when_disabled(): + # Re-capturing with mode off must not leave a stale value for that region. + w = _FMStub(enabled=False, controller=_laser_controller(3.5), offsets={"A1": 9.0}) + w._capture_region_offset("A1") + assert "A1" not in w.region_laser_af_offsets + + +def test_clear_region_offsets(): + w = _FMStub(offsets={"A1": 1.0, "B2": 2.0}) + w._clear_region_offsets() + assert w.region_laser_af_offsets == {} + + +def test_sync_drops_orphaned_offsets(): + w = _FMStub(focus_points=[("A1", 0.0, 0.0, 1.0)], offsets={"A1": 1.0, "B2": 2.0}) + w._sync_offsets_to_focus_points() + assert w.region_laser_af_offsets == {"A1": 1.0} + + +def test_capture_sets_offsets_reference_x_on_success(): + ctrl = _laser_controller(3.5) + ctrl.laser_af_properties.x_reference = 123.0 + w = _FMStub(controller=ctrl) + w._capture_region_offset("A1") + assert w._offsets_reference_x == 123.0 + + +def test_capture_failure_logs_warning(): + # A failed re-capture (spot not detected) drops the offset AND logs a warning — not just + # a transient status label — so silent data loss is visible. + w = _FMStub(controller=_laser_controller(float("nan")), offsets={"A1": 4.0}) + w._capture_region_offset("A1") + assert "A1" not in w.region_laser_af_offsets + assert w._log.warning.called + + +def test_reference_change_clears_offsets_when_reference_actually_changed(): + ctrl = _laser_controller(0.0) + ctrl.laser_af_properties.x_reference = 200.0 # reference is now different + w = _FMStub(controller=ctrl, offsets={"A1": 1.0}) + w._offsets_reference_x = 100.0 # offsets were captured against the OLD reference + w._on_laser_af_reference_changed(True) + assert w.region_laser_af_offsets == {} + assert w.status_label.setText.called + + +def test_reference_change_keeps_offsets_on_benign_reemit(): + # A config reload re-emits signal_reference_changed with the SAME reference; offsets must survive. + ctrl = _laser_controller(0.0) + ctrl.laser_af_properties.x_reference = 100.0 + w = _FMStub(controller=ctrl, offsets={"A1": 1.0}) + w._offsets_reference_x = 100.0 # same reference the offsets were captured against + w._on_laser_af_reference_changed(True) + assert w.region_laser_af_offsets == {"A1": 1.0} + w.status_label.setText.assert_not_called() + + +def test_reference_lost_clears_offsets(): + ctrl = _laser_controller(0.0, has_reference=False) + w = _FMStub(controller=ctrl, offsets={"A1": 1.0}) + w._offsets_reference_x = 100.0 + w._on_laser_af_reference_changed(False) + assert w.region_laser_af_offsets == {} + + +def test_reference_change_no_status_when_nothing_to_clear(): + w = _FMStub() + w._on_laser_af_reference_changed(True) + w.status_label.setText.assert_not_called() + + +def test_csv_roundtrip_includes_offsets(tmp_path): + src = _FMStub( + focus_points=[("A1", 1.0, 2.0, 0.5), ("B2", 3.0, 4.0, 0.6)], + offsets={"A1": 7.0}, # B2 intentionally has no offset + ) + src._write_focus_points_csv = FocusMapWidget._write_focus_points_csv.__get__(src) + path = str(tmp_path / "fp.csv") + src._write_focus_points_csv(path) + + dst = _FMStub() + dst._read_focus_points_csv = FocusMapWidget._read_focus_points_csv.__get__(dst) + points, offsets = dst._read_focus_points_csv(path) + assert points == [("A1", 1.0, 2.0, 0.5), ("B2", 3.0, 4.0, 0.6)] + assert offsets == {"A1": 7.0} + + +def test_csv_read_back_compat_without_offset_column(tmp_path): + path = tmp_path / "legacy.csv" + path.write_text("Region_ID,X_mm,Y_mm,Z_um\nA1,1.0,2.0,0.5\n") + dst = _FMStub() + dst._read_focus_points_csv = FocusMapWidget._read_focus_points_csv.__get__(dst) + points, offsets = dst._read_focus_points_csv(str(path)) + assert points == [("A1", 1.0, 2.0, 0.5)] + assert offsets == {} + + +def test_csv_read_rejects_missing_required_columns(tmp_path): + path = tmp_path / "bad.csv" + path.write_text("Region_ID,X_mm\nA1,1.0\n") + dst = _FMStub() + dst._read_focus_points_csv = FocusMapWidget._read_focus_points_csv.__get__(dst) + + with pytest.raises(ValueError): + dst._read_focus_points_csv(str(path)) + + +def test_csv_read_skips_nonfinite_offsets_and_coords(tmp_path): + path = tmp_path / "bad.csv" + path.write_text( + "Region_ID,X_mm,Y_mm,Z_um,Offset_um\n" + "A1,1.0,2.0,0.5,3.0\n" # good + "B2,1.0,2.0,0.5,nan\n" # good coords, NaN offset -> point kept, offset rejected + "C3,nan,2.0,0.5,4.0\n" # NaN coordinate -> whole row dropped + "D4,1.0,2.0,0.5,inf\n" # inf offset -> point kept, offset rejected + ) + dst = _FMStub() + dst._read_focus_points_csv = FocusMapWidget._read_focus_points_csv.__get__(dst) + points, offsets = dst._read_focus_points_csv(str(path)) + assert [p[0] for p in points] == ["A1", "B2", "D4"] # C3 dropped (non-finite coord) + assert offsets == {"A1": 3.0} # NaN/inf offsets rejected + + +# --------------------------------------------------------------------------- +# _on_per_region_offset_toggled +# --------------------------------------------------------------------------- + + +def test_toggle_on_sets_capture_enabled(): + w = _FMStub(enabled=False) + w._on_per_region_offset_toggled(True) + assert w.capture_laser_af_offset_enabled is True + + +def test_toggle_off_disables_capture_but_retains_offsets(): + # Unchecking only stops new captures; it must NOT discard already-captured offsets + # (those are cleared only on reference change or focus-point edits). The acquisition + # gate returns {} while unchecked, so retaining them is safe and avoids data loss when + # an unrelated state change (e.g. another tab) incidentally unchecks the box. + w = _FMStub(enabled=True, offsets={"A1": 1.0, "B2": 2.0}) + w._on_per_region_offset_toggled(False) + assert w.capture_laser_af_offset_enabled is False + assert w.region_laser_af_offsets == {"A1": 1.0, "B2": 2.0} + + +# --------------------------------------------------------------------------- +# _update_per_region_offset_enabled — gated ONLY on the shared focus-map controls +# (constant + Fit by Region), NOT on any tab's Reflection AF checkbox, so a tab +# toggle cannot clobber the shared widget. Reflection AF is enforced at acquisition. +# --------------------------------------------------------------------------- + + +def test_update_per_region_offset_disabled_when_method_is_spline(): + w = _FMStub() + w.fit_method_combo.currentText.return_value = "spline" + w.by_region_checkbox.isChecked.return_value = True + w.checkbox_perRegionLaserAFOffset.isChecked.return_value = False + w._update_per_region_offset_enabled() + w.checkbox_perRegionLaserAFOffset.setEnabled.assert_called_with(False) + + +def test_update_per_region_offset_disabled_when_by_region_unchecked(): + w = _FMStub() + w.fit_method_combo.currentText.return_value = "constant" + w.by_region_checkbox.isChecked.return_value = False + w.checkbox_perRegionLaserAFOffset.isChecked.return_value = False + w._update_per_region_offset_enabled() + w.checkbox_perRegionLaserAFOffset.setEnabled.assert_called_with(False) + + +def test_update_per_region_offset_enabled_when_constant_and_by_region(): + w = _FMStub() + w.fit_method_combo.currentText.return_value = "constant" + w.by_region_checkbox.isChecked.return_value = True + w.checkbox_perRegionLaserAFOffset.isChecked.return_value = False + w._update_per_region_offset_enabled() + w.checkbox_perRegionLaserAFOffset.setEnabled.assert_called_with(True) + + +def test_update_per_region_offset_unchecks_when_conditions_not_met_and_was_checked(): + """When conditions fail and the checkbox was checked, it must be unchecked.""" + w = _FMStub() + w.fit_method_combo.currentText.return_value = "rbf" + w.by_region_checkbox.isChecked.return_value = True + w.checkbox_perRegionLaserAFOffset.isChecked.return_value = True + w._update_per_region_offset_enabled() + w.checkbox_perRegionLaserAFOffset.setEnabled.assert_called_with(False) + w.checkbox_perRegionLaserAFOffset.setChecked.assert_called_once_with(False) + + +def test_get_offsets_for_acquisition_returns_offsets_when_active(): + w = _FMStub(offsets={"A1": 3.0}) + w.checkbox_perRegionLaserAFOffset.isChecked.return_value = True + result = w.get_offsets_for_acquisition(reflection_af_active=True) + assert result == {"A1": 3.0} + assert result is not w.region_laser_af_offsets # returns a copy, not the live dict + + +def test_get_offsets_for_acquisition_empty_when_reflection_af_off(): + w = _FMStub(offsets={"A1": 3.0}) + w.checkbox_perRegionLaserAFOffset.isChecked.return_value = True + assert w.get_offsets_for_acquisition(reflection_af_active=False) == {} + + +def test_get_offsets_for_acquisition_empty_when_checkbox_off(): + w = _FMStub(offsets={"A1": 3.0}) + w.checkbox_perRegionLaserAFOffset.isChecked.return_value = False + assert w.get_offsets_for_acquisition(reflection_af_active=True) == {} + + +def test_get_offsets_warns_about_regions_missing_offset(): + # A scan region with no captured offset falls back to the reference plane — warn at run + # start so a silently-missing well is visible. + w = _FMStub(offsets={"A1": 3.0}) + w.checkbox_perRegionLaserAFOffset.isChecked.return_value = True + w.scanCoordinates = MagicMock() + w.scanCoordinates.region_centers.keys.return_value = ["A1", "B2"] + result = w.get_offsets_for_acquisition(reflection_af_active=True) + assert result == {"A1": 3.0} + assert w._log.warning.called # B2 has no offset