From f7d03d8c00cbd0842e31fdf64ff311afd05ded9b Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Wed, 5 Nov 2025 12:27:49 +0000 Subject: [PATCH] Fix confusion between (x,y) image coordinates and (y,x) matrix indicies --- .../things/camera_stage_mapping.py | 61 ++++++++++++++++--- .../things/stage_measure.py | 1 - .../utilities.py | 53 ---------------- tests/test_stage_measure.py | 33 +++++----- 4 files changed, 70 insertions(+), 78 deletions(-) diff --git a/src/openflexure_microscope_server/things/camera_stage_mapping.py b/src/openflexure_microscope_server/things/camera_stage_mapping.py index 608a548b..7257c0f9 100644 --- a/src/openflexure_microscope_server/things/camera_stage_mapping.py +++ b/src/openflexure_microscope_server/things/camera_stage_mapping.py @@ -33,7 +33,6 @@ import labthings_fastapi as lt from labthings_fastapi.types.numpy import DenumpifyingDict from camera_stage_mapping.camera_stage_tracker import Tracker -from openflexure_microscope_server.utilities import apply_2d_matrix_to_mapping from .camera import CameraDependency as CameraClient from .stage import StageDependency as Stage @@ -282,9 +281,7 @@ class CameraStageMapper(lt.Thing): ) -> Mapping[str, int]: """Convert image coordinates to stage coordinates. Only x and y are returned.""" self.assert_calibrated() - return apply_2d_matrix_to_mapping( - self.image_to_stage_displacement_matrix, x=x, y=y, to_int=True - ) + return csm_img_to_stage(self.image_to_stage_displacement_matrix, x=x, y=y) @lt.thing_action def convert_stage_to_image_coordinates( @@ -292,10 +289,7 @@ class CameraStageMapper(lt.Thing): ) -> Mapping[str, float]: """Convert stage coordinates to image coordinates. Only x and y are returned.""" self.assert_calibrated() - inverse_matrix = np.linalg.inv( - np.array(self.image_to_stage_displacement_matrix) - ) - return apply_2d_matrix_to_mapping(inverse_matrix, x=x, y=y, to_int=False) + return csm_stage_to_img(self.image_to_stage_displacement_matrix, x=x, y=y) @lt.thing_property def thing_state(self) -> Mapping[str, Any]: @@ -304,3 +298,54 @@ class CameraStageMapper(lt.Thing): k: getattr(self, k) for k in ["image_to_stage_displacement_matrix", "image_resolution"] } + + +def csm_img_to_stage( + matrix: np.ndarray | list[list[float]], + *, + x: float | int, + y: float | int, + **_kwargs: int, +) -> Mapping[str, int]: + """Apply any CSM matrix to image coordinates. + + This is designed x and y must be kwargs and extra kwargs are ignored allowing: + ``csm_img_to_stage(matrix, **position)`` to run for a mapping position. + + Note that x and y are the actual (x, y) of the image, not the (m, n) indices used + numpy + + :param matrix: The matrix to use in the calculation + :param x: the x image coordinate (keyword only) + :param y: the y image coordinate (keyword only) + :return: The resulting stage coordinates as a mapping. + """ + # Note this is (y,x) not x,y to put it in numpy image indices + relative_move: np.ndarray = np.dot(np.array([y, x]), np.array(matrix)) + return {"x": round(relative_move[0]), "y": round(relative_move[1])} + + +def csm_stage_to_img( + matrix: np.ndarray | list[list[float]], + *, + x: float | int, + y: float | int, + **_kwargs: int, +) -> Mapping[str, float]: + """Apply any CSM matrix to stage coordinates to get image coordinates. + + This is designed x and y must be kwargs and extra kwargs are ignored allowing: + ``csm_img_to_stage(matrix, **position)`` to run for a mapping position. + + Note that x and y are the actual (x, y) of the image, not the (m, n) indices used + numpy + + :param matrix: The matrix to use in the calculation + :param x: the x stage coordinate (keyword only) + :param y: the y stage coordinate (keyword only) + :return: The resulting img coordinates as a mapping. + """ + inverse_matrix = np.linalg.inv(np.array(matrix)) + relative_move = np.dot(np.array([x, y]), inverse_matrix) + # Note that the relative move (y, x) as it is from numpy and is in matrix coords. + return {"x": float(relative_move[1]), "y": float(relative_move[0])} diff --git a/src/openflexure_microscope_server/things/stage_measure.py b/src/openflexure_microscope_server/things/stage_measure.py index 7a9eb395..370943e5 100644 --- a/src/openflexure_microscope_server/things/stage_measure.py +++ b/src/openflexure_microscope_server/things/stage_measure.py @@ -436,7 +436,6 @@ class RangeofMotionThing(lt.Thing): target_im_coords = rom_deps.csm.convert_stage_to_image_coordinates(**target) here = rom_deps.stage.position here_im_coords = rom_deps.csm.convert_stage_to_image_coordinates(**here) - return -1 if target_im_coords[axis] < here_im_coords[axis] else 1 def _distance_in_img_percentage( diff --git a/src/openflexure_microscope_server/utilities.py b/src/openflexure_microscope_server/utilities.py index 07e3c19e..77c2ab06 100644 --- a/src/openflexure_microscope_server/utilities.py +++ b/src/openflexure_microscope_server/utilities.py @@ -11,7 +11,6 @@ from typing import ( overload, TypeAlias, Literal, - Mapping, ) import os import re @@ -23,7 +22,6 @@ import tomllib from functools import wraps import json -import numpy as np from pydantic import BaseModel T = TypeVar("T") @@ -477,54 +475,3 @@ def resolve_path_from_dir(path: str, directory: str) -> str: if not os.path.isabs(path): path = os.path.join(directory, path) return os.path.normpath(path) - - -# Use overload to that the dict is either of integers or of float depending on the -# to_int argument -@overload -def apply_2d_matrix_to_mapping( - matrix: np.ndarray, - *, - x: float | int, - y: float | int, - to_int: Literal[False], - **_kwargs: int, -) -> Mapping[str, float]: ... -@overload -def apply_2d_matrix_to_mapping( - matrix: np.ndarray, - *, - x: float | int, - y: float | int, - to_int: Literal[True], - **_kwargs: int, -) -> Mapping[str, int]: ... -@overload -def apply_2d_matrix_to_mapping( - matrix: np.ndarray, *, x: float | int, y: float | int, **_kwargs: int -) -> Mapping[str, float]: ... - - -def apply_2d_matrix_to_mapping( - matrix: np.ndarray, - *, - x: float | int, - y: float | int, - to_int: bool = False, - **_kwargs: int, -) -> Mapping[str, int] | Mapping[str, float]: - """Perform a dot product with a (x,y) vector and a matrix. The result is a mapping. - - This is designed x and y must be kwargs and extra kwargs are ignored allowing: - ``apply_2d_matrix_to_mapping(matrix, **position)`` to run for a mapping position. - - :param matrix: The matrix to use in the calculation - :param x: the x coordinate (keyword only) - :param y: the y coordinate (keyword only) - :param to_int: Whether to convert the resulting coordinates to integers - :return: The resulting coordinates as a mapping. - """ - relative_move: np.ndarray = np.dot(np.array([y, x]), np.array(matrix)) - if to_int: - return {"x": round(relative_move[0]), "y": round(relative_move[1])} - return {"x": float(relative_move[0]), "y": float(relative_move[1])} diff --git a/tests/test_stage_measure.py b/tests/test_stage_measure.py index 142c7664..74c09e53 100644 --- a/tests/test_stage_measure.py +++ b/tests/test_stage_measure.py @@ -12,7 +12,10 @@ import pytest import labthings_fastapi as lt from openflexure_microscope_server.things import stage_measure -from openflexure_microscope_server.utilities import apply_2d_matrix_to_mapping +from openflexure_microscope_server.things.camera_stage_mapping import ( + csm_img_to_stage, + csm_stage_to_img, +) LOGGER = logging.getLogger("mock-invocation_logger") @@ -163,15 +166,14 @@ def rom_thing(example_rom_data) -> stage_measure.RangeofMotionThing: @pytest.fixture def mock_rom_deps(csm_matrix, mocker) -> stage_measure.RomDeps: """Return a RomDeps object full of mocks, except the logger which is LOGGER.""" - inverse_matrix = np.linalg.inv(csm_matrix) def apply_csm(x: float, y: float, **_kwargs: float) -> dict[str, int]: """Convert image coordinates to stage coordinates.""" - return apply_2d_matrix_to_mapping(csm_matrix, x=x, y=y, to_int=True) + return csm_img_to_stage(csm_matrix, x=x, y=y) def un_apply_csm(x: float, y: float, **_kwargs: float) -> dict[str, float]: """Convert stage coordinates to image coordinates.""" - return apply_2d_matrix_to_mapping(inverse_matrix, x=x, y=y, to_int=True) + return csm_stage_to_img(csm_matrix, x=x, y=y) mock_cam = mocker.Mock() mock_cam.image_is_sample.return_value = (True, "Mocked not measured.") @@ -194,12 +196,11 @@ def mock_rom_deps(csm_matrix, mocker) -> stage_measure.RomDeps: @pytest.mark.parametrize( ("pos", "target_pos", "axis", "expected_dir"), [ - # X is flipped due to CSM sign - ({"x": 5000, "y": 0, "z": 0}, {"x": 0, "y": 0, "z": 0}, "x", 1), - ({"x": -5000, "y": 0, "z": 0}, {"x": 0, "y": 0, "z": 0}, "x", -1), - ({"x": 0, "y": 0, "z": 0}, {"x": 5000, "y": 0, "z": 0}, "x", -1), - # Y is not flipped due to CSM sign - ({"x": 0, "y": 5000, "z": 0}, {"x": 0, "y": 0, "z": 0}, "y", -1), + ({"x": 5000, "y": 0, "z": 0}, {"x": 0, "y": 0, "z": 0}, "x", -1), + ({"x": -5000, "y": 0, "z": 0}, {"x": 0, "y": 0, "z": 0}, "x", 1), + ({"x": 0, "y": 0, "z": 0}, {"x": 5000, "y": 0, "z": 0}, "x", 1), + # Y is flipped due to CSM sign + ({"x": 0, "y": 5000, "z": 0}, {"x": 0, "y": 0, "z": 0}, "y", 1), ], ) def test_img_dir_from_stage_coords( @@ -223,11 +224,11 @@ def test_distance_in_img_percentage_err(rom_thing, mock_rom_deps): @pytest.mark.parametrize( ("pos", "target_pos", "axis", "expected_perc"), [ - ({"x": 5000, "y": 0, "z": 0}, {"x": 0, "y": 0, "z": 0}, "x", 346.625), - ({"x": -5000, "y": 0, "z": 0}, {"x": 0, "y": 0, "z": 0}, "x", -346.625), - ({"x": 0, "y": 0, "z": 0}, {"x": 5000, "y": 0, "z": 0}, "x", -346.625), + ({"x": 5000, "y": 0, "z": 0}, {"x": 0, "y": 0, "z": 0}, "x", -352.44), + ({"x": -5000, "y": 0, "z": 0}, {"x": 0, "y": 0, "z": 0}, "x", 352.44), + ({"x": 0, "y": 0, "z": 0}, {"x": 5000, "y": 0, "z": 0}, "x", 352.44), # Y is flipped due to CSM sign - ({"x": 0, "y": 5000, "z": 0}, {"x": 0, "y": 0, "z": 0}, "y", -470.0), + ({"x": 0, "y": 5000, "z": 0}, {"x": 0, "y": 0, "z": 0}, "y", 462.131), ], ) def test_distance_in_img_percentage( @@ -236,7 +237,7 @@ def test_distance_in_img_percentage( """Check _distance_in_img_percentage calculates expected values based on mock CSM.""" mock_rom_deps.stage.position = pos img_perc = rom_thing._distance_in_img_percentage(target_pos, axis, mock_rom_deps) - assert expected_perc == img_perc + assert round(img_perc, 3) == expected_perc def test_offset_from(rom_thing, mock_rom_deps, mocker): @@ -703,7 +704,7 @@ def test_recentre_axis(true_on, rom_thing, mock_rom_deps, mocker): This doesn't include deciding if we are centred or choosing the direction to move. """ ## Start such that movement starts negative - mock_rom_deps.stage.position = {"x": -10000, "y": 10000, "z": 0} + mock_rom_deps.stage.position = {"x": 10000, "y": 10000, "z": 0} mock_moves = mocker.patch.object(rom_thing, "_moves_for_z_prediction") # Mock _recentre_decision so it returns (False, 1) a number of times then finally