Fix confusion between (x,y) image coordinates and (y,x) matrix indicies

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(
+        return csm_img_to_stage(self.image_to_stage_displacement_matrix, x=x, y=y)
-            self.image_to_stage_displacement_matrix, x=x, y=y, to_int=True
-        )
 
     @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(
+        return csm_stage_to_img(self.image_to_stage_displacement_matrix, x=x, y=y)
-            np.array(self.image_to_stage_displacement_matrix)
-        )
-        return apply_2d_matrix_to_mapping(inverse_matrix, x=x, y=y, to_int=False)
 
     @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])}

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(

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])}

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": -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),
-        ({"x": 0, "y": 0, "z": 0}, {"x": 5000, "y": 0, "z": 0}, "x", -1),
+        # Y is flipped due to CSM sign
-        # Y is not flipped due to CSM sign
+        ({"x": 0, "y": 5000, "z": 0}, {"x": 0, "y": 0, "z": 0}, "y", 1),
-        ({"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", -352.44),
-        ({"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", 352.44),
-        ({"x": 0, "y": 0, "z": 0}, {"x": 5000, "y": 0, "z": 0}, "x", -346.625),
+        ({"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