docs(omny): make the vision pipeline a generic camera-agnostic device
CI for csaxs_bec / test (push) Successful in 5m6s
CI for csaxs_bec / test (push) Successful in 5m6s
Reworks the plan per feedback: instead of building CV logic into IDSCamera / inline in the sample-transfer mixin, propose a standalone SampleVisionMonitor device whose only dependency is the get_last_image() contract already shared by IDSCamera and AlliedVisionAravisCamera, wired to its camera(s) by configured name (ddg_1.py's device_manager.devices.get() pattern, generalized). Same class is then reusable for any endstation's camera(s) via config alone. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018Mupip8x7Hean6E3FFvpbj
This commit is contained in:
+95
-27
@@ -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
|
||||
|
||||
|
||||
Reference in New Issue
Block a user