diff --git a/csaxs_bec/bec_ipython_client/plugins/omny/AI_docs/MACHINE_VISION_AUTOCONFIRM_PLAN.md b/csaxs_bec/bec_ipython_client/plugins/omny/AI_docs/MACHINE_VISION_AUTOCONFIRM_PLAN.md index a150b4ff..134ae6ba 100644 --- a/csaxs_bec/bec_ipython_client/plugins/omny/AI_docs/MACHINE_VISION_AUTOCONFIRM_PLAN.md +++ b/csaxs_bec/bec_ipython_client/plugins/omny/AI_docs/MACHINE_VISION_AUTOCONFIRM_PLAN.md @@ -50,28 +50,87 @@ an HDF5 file under `~/data/raw/logs/omny_sample_transfer_images/`, named by step is already the reference-image corpus this plan would compare against** — it exists purely as a post-hoc audit log today, with no comparison logic reading it back. -## Existing infrastructure to build on (no new plumbing needed for a prototype) +## Existing infrastructure to build on -- **`IDSCamera.get_last_image()`** (`csaxs_bec/devices/ids_cameras/ids_camera.py:234`) — already - how `_otransfer_save_reference_images()` gets frames; same call works for a live comparison. +- **`get_last_image() -> np.ndarray`** is already a shared, RPC-exposed contract across camera + device classes, not an `IDSCamera` idiosyncrasy: both `IDSCamera` + (`csaxs_bec/devices/ids_cameras/ids_camera.py:234`, `rpc_access` line 72) and + `AlliedVisionAravisCamera` (`csaxs_bec/devices/allied_vision_cameras/allied_vision_aravis_camera.py:216`, + `rpc_access` line 62) implement it identically, and client-side code already calls it directly + through the device-server RPC boundary — `LamNI/x_ray_eye_align.py:202,821` and + `flomni/x_ray_eye_align.py:127` do exactly this (`dev.cam_xeye.get_last_image()`) regardless of + which concrete camera class backs `cam_xeye` at a given beamline. This is the natural minimal + interface for a generic vision component to depend on — see the architecture section below. - **`OMNYGuiTools._omnycam_active_pair`** (`gui_tools.py:88-105`) — tracks which camera pair (`cam200`+`cam203` parking/shuttle, or `cam201`+`cam202` sample stage) is currently shown, exactly the info needed to pick the matching reference subset. -- **`IDSCamera.push_preview_image()` / `push_smear_preview()`** (`ids_camera.py:240,269`) — a - device-server-side hook to publish a processed/composite array back through the same - `device_preview` channel the GUI already subscribes to, without disturbing the live acquisition - feed. **This exact mechanism is already proven for a similar purpose**: LamNI's rotation-center - calibration (`LamNI/x_ray_eye_align.py:737-797`, see `[[project_lamni_rotation_center_calibration]]`-era - work) pushes a growing composite image via `push_smear_preview()`, and - `bec_widgets/widgets/xray_eye/x_ray_eye.py` (`smear_preview_toggle`, lines 386-873) already has a - working UI toggle to switch an `Image` dock between the live feed and this diagnostic channel. - **A vision-diff overlay for OMNY sample transfer should reuse this pattern** — push an - aligned-reference/diff/contour-overlay composite via `push_smear_preview()` on whichever of - cam200/201/202/203 is active, and add the same toggle affordance to OMNY's camera docks - (`gui_tools.py:127-152`) — rather than inventing a third dock or a new preview channel. +- **Precedent for one device depending on another by configured name, resolved through the device + manager**: `DDG1.prepare_mcs_on_trigger()` (`csaxs_bec/devices/epics/delay_generator_csaxs/ddg_1.py:369-397`) + looks up a sibling device via `self.device_manager.devices.get(mcs_name, None)` at call time (not + at `__init__`, since device construction order across a YAML config isn't guaranteed) and raises + clearly if it's missing. Same pattern, generalized to a *configurable* name instead of `ddg_1.py`'s + hardcoded `"mcs"`, is the mechanism proposed below for a vision device's camera dependency. +- **`IDSCamera.push_preview_image()` / `push_smear_preview()`** (`ids_camera.py:240,269`) — an + `IDSCamera`-specific hook to publish a processed/composite array through its own + `device_preview` channel, already proven for a similar purpose in LamNI's rotation-center + calibration (`LamNI/x_ray_eye_align.py:737-797`) together with the matching UI toggle in + `bec_widgets/widgets/xray_eye/x_ray_eye.py:386-873`. Usable as a visualization *fallback* when the + configured camera happens to be an `IDSCamera`, but not something the generic vision device + should depend on — see "own diagnostic signal vs. borrowing the camera's channel" below. - **`opencv-python`** is already a project dependency (`pyproject.toml:27`); `scikit-image` is not yet (needed only if SSIM specifically is wanted — see below). +## Proposed architecture: a generic, camera-agnostic vision device + +Earlier drafts of this plan sketched the CV pipeline as logic living inside `IDSCamera` itself, or +inline in `omny_sample_transfer_mixin.py` calling `cv2`/`skimage` directly against whichever +`cam200`-`cam203` frame it just pulled. Mirko's feedback: keep the imaging pipeline generic and +reusable against *any* camera, structured as its own device with a camera as a declared dependency, +rather than building it into a specific camera class. Concretely: + +- **A new, endstation-agnostic device class** (working name `SampleVisionMonitor`), living outside + `omny/` — e.g. `csaxs_bec/devices/vision/sample_vision_monitor.py` — so it isn't OMNY-specific + code, just OMNY's first user of it. +- **Its only hard dependency is the shared `get_last_image()` contract** above — nothing + `IDSCamera`-specific (no reliance on `push_smear_preview`, the `image` `PreviewSignal`'s + rotation/transpose handling, etc.). Any current or future camera class that implements + `get_last_image() -> np.ndarray | None` can be wired in, including `AlliedVisionAravisCamera` or a + simulated camera, with zero changes to this device. +- **Config-driven camera dependency, resolved lazily.** A `camera_names: list[str]` constructor + parameter (a list, not a single name, so OMNY's camera-*pairs* — `["cam200", "cam203"]` for + parking, `["cam201", "cam202"]` for the sample stage — are a natural fit; a single-camera setup + like flomni's gripper cam is just a length-1 list). Resolved through + `self.device_manager.devices.get(name, None)` at call time, mirroring `ddg_1.py`'s pattern above, + with a clear error if a configured name isn't present in the active session's device config. + **This is what makes the device reusable across endstations without touching its code**: the same + class, pointed at a different `camera_names` list in a different beamline's device-config YAML, + is the entire integration cost for a new camera or a new endstation. +- **Runs device-server-side** (a `PSIDeviceBase` subclass, like `IDSCamera` itself), consistent with + where camera image processing already happens in this codebase, and appropriate for the + registration/scoring compute cost. +- **Opaque, caller-defined reference tags**, not OMNY vocabulary baked into the device: reference + images and scores are grouped by a plain string tag the caller picks (for OMNY, + `f"{step_name}_{pin_position}"`; a different endstation could tag however makes sense to it) — the + device itself has no notion of "pin position" or "shuttle slot". +- **Client-facing RPC surface** (sketch, names not final): + - `save_reference(tag: str) -> None` — pulls the current frame(s) from the configured camera(s) + and stores them as a labeled reference under `tag`. + - `score(tag: str) -> dict` — pulls the current frame(s), registers + scores each against every + stored reference under `tag` (see CV pipeline below), publishes a diagnostic overlay (see next + bullet), and returns the best-match score(s) plus enough detail to build a confirm message. + - `omny_sample_transfer_mixin.py`'s gates (1-4 above) become *callers* of this device + (`dev.omny_vision_parking.score(...)`, `dev.omny_vision_samplestage.score(...)`, one device + instance per camera pair) instead of doing any `cv2`/`skimage` work themselves — the mixin only + turns the returned score into a confirm-dialog decision (per the staged rollout below). + - **Own diagnostic signal vs. borrowing the target camera's channel** — open decision, not + resolved here: (a) give `SampleVisionMonitor` its own `diagnostic_image` `PreviewSignal` + component, fully decoupled from whichever camera it's pointed at, requiring one new `Image` dock + in `gui_tools.py` per vision device; vs. (b) when the configured camera happens to expose + `push_smear_preview()` (i.e. it's an `IDSCamera`), feature-detect that and reuse the existing + dock + toggle pattern instead of adding a new one. (a) is more genuinely camera-agnostic (matches + the rest of this design); (b) is less new GUI surface for OMNY specifically, at the cost of only + working when the backing camera happens to support it. Worth prototyping both before deciding. + ## Proposed CV pipeline (classical, no deep learning) Per the original discussion this grew out of: classical machine vision is enough for a @@ -92,10 +151,10 @@ with hardware changes. general structural-anomaly fallback, in case a defect doesn't show up as a clean contour change. (Pulls in `scikit-image` as a new dependency — only if this signal proves worth its weight; contour matching alone may be sufficient given how constrained the geometry is.) -4. **Multi-reference scoring.** Score against *every* reference image for the given - `(pin_position, step_name, camera)` combination and take the best (minimum-distance) match, not - a single fixed reference — natural sample-to-sample and lighting variation shouldn't inflate the - anomaly score just because one particular reference happened to be a bad match. +4. **Multi-reference scoring.** Score against *every* reference image stored under the given tag + and take the best (minimum-distance) match, not a single fixed reference — natural + sample-to-sample and lighting variation shouldn't inflate the anomaly score just because one + particular reference happened to be a bad match. 5. **Escalate only if needed.** If classical contour/SSIM scoring proves insufficiently robust to subtle real defects, `anomalib` (e.g. PatchCore) is the documented fallback — but per the original assessment this is very likely overkill for a fixed pin/gripper geometry, and shouldn't @@ -107,10 +166,11 @@ Physical hardware, real samples, real damage risk (see the top-pin threshold in this should not be an all-or-nothing flag flip. Proposed phases for the later implementation branch: - **Phase 0 — instrument, don't automate.** Compute and log the vision score at every existing gate - (1–4 above); still always show the human the same confirm prompt, but now alongside the score and - a diff/overlay visualization (via the `push_smear_preview()` pattern above). Purpose: build a - labeled dataset of real operator pass/fail decisions vs. score, across real lighting and sample - variety, before trusting any threshold. No behavior change to the transfer flow itself. + (1–4 above) via `SampleVisionMonitor.score()`; still always show the human the same confirm + prompt, but now alongside the score and the diagnostic diff/overlay visualization it publishes. + Purpose: build a labeled dataset of real operator pass/fail decisions vs. score, across real + lighting and sample variety, before trusting any threshold. No behavior change to the transfer + flow itself. - **Phase 1 — assist, never silently reject.** A comfortably-high score can auto-answer "yes" at a gate (with the decision logged and visibly flagged as auto-confirmed, and an easy override); a low score never fails silently or auto-aborts — it escalates to the same confirm prompt, now with @@ -129,17 +189,25 @@ this should not be an all-or-nothing flag flip. Proposed phases for the later im - **Reference corpus curation.** Today's saved images are a timestamped append-only run log, not a curated "golden" set — every run adds a new file, nothing marks one as trustworthy for - comparison. Need either a promotion step (operator marks a saved image as a reference) or a - separate deliberately-curated directory, distinct from the audit-log files - `_otransfer_save_reference_images()` already writes. + comparison. Need either a promotion step (operator marks a saved image as a reference, e.g. via + `save_reference()` above) or a separate deliberately-curated directory, distinct from the + audit-log files `_otransfer_save_reference_images()` already writes. - **Per-camera-pair segmentation.** Parking/shuttle view (cam200+cam203) and sample-stage view (cam201+cam202) have different backgrounds/lighting; silhouette extraction likely needs tuning - per pair, not one global set of CV parameters. + per pair, not one global set of CV parameters — one `SampleVisionMonitor` instance per pair (its + own config, own reference store) is the natural place for that tuning to live. - **Threshold calibration.** Can't be picked a priori — needs the Phase 0 logged-score dataset across real sessions before any auto-confirm threshold is chosen. - **Where scores/results live.** Likely alongside the existing HDF5 reference-image files (same per-step file, additional dataset for score + registration transform), rather than a new storage mechanism. +- **Diagnostic visualization: own `PreviewSignal` vs. borrowing the camera's `smear_preview`** — + see the architecture section above; needs a short prototype of both before committing. +- **One `SampleVisionMonitor` config per camera pair vs. one instance whose `camera_names` gets + reconfigured at call time** — static per-pair config (e.g. `omny_vision_parking`, + `omny_vision_samplestage` as two fixed device-config entries) is simpler and matches how OMNY's + own two camera pairs are already fixed, not dynamic; leaning toward this unless a concrete need + for a dynamic pairing shows up. ## Non-goals for this session