700 fix deployment issues (#701)

* Changes to make version and folder names match current deployment locations
* Inherit the stdout and stderr from the parent process in the create_venv script instead of piping
* Apply chi for gridscans when it is specified as 0
Apply minimum count threshold of 3 to returned hits
* Fixes to deployment script to improve stdout, stderr handling
* Add tests for phi,chi,omega move, extend behaviour to xyz for consistency
* Refactor threshold into a constant
* Always make sure setuptools_scm is installed in deploy_hyperion_to_k8s.sh
* Make the version check warning more prominent
Fix the path-name munging for the version check
Add a dry run option
* Make the pushed image the same as the regularly built docker image

.github/workflows/_container.yml

@@ -51,6 +51,7 @@ jobs:
         # Docker cache and publish it
         with:
           context: .
+          file: Dockerfile.release
           push: true
           tags: ${{ steps.meta.outputs.tags }}
           labels: ${{ steps.meta.outputs.labels }}

src/mx_bluesky/hyperion/device_setup_plans/manipulate_sample.py

@@ -78,11 +78,11 @@ def move_x_y_z(
     axes are optional."""
 
     LOGGER.info(f"Moving smargon to x, y, z: {(x_mm, y_mm, z_mm)}")
-    if x_mm:
+    if x_mm is not None:
         yield from bps.abs_set(smargon.x, x_mm, group=group)
-    if y_mm:
+    if y_mm is not None:
         yield from bps.abs_set(smargon.y, y_mm, group=group)
-    if z_mm:
+    if z_mm is not None:
         yield from bps.abs_set(smargon.z, z_mm, group=group)
     if wait:
         yield from bps.wait(group)
@@ -100,11 +100,11 @@ def move_phi_chi_omega(
     axes are optional."""
 
     LOGGER.info(f"Moving smargon to phi, chi, omega: {(phi, chi, omega)}")
-    if phi:
+    if phi is not None:
         yield from bps.abs_set(smargon.phi, phi, group=group)
-    if chi:
+    if chi is not None:
         yield from bps.abs_set(smargon.chi, chi, group=group)
-    if omega:
+    if omega is not None:
         yield from bps.abs_set(smargon.omega, omega, group=group)
     if wait:
         yield from bps.wait(group)

src/mx_bluesky/hyperion/experiment_plans/flyscan_xray_centre_plan.py

@@ -79,6 +79,8 @@
 from mx_bluesky.hyperion.parameters.gridscan import HyperionThreeDGridScan
 from mx_bluesky.hyperion.utils.context import device_composite_from_context
 
+ZOCALO_MIN_TOTAL_COUNT_THRESHOLD = 3
+
 
 class SmargonSpeedException(Exception):
     pass
@@ -247,10 +249,20 @@ def run_gridscan_and_fetch_results(
             LOGGER.info("Zocalo triggered and read, interpreting results.")
             xrc_results = yield from get_full_processing_results(fgs_composite.zocalo)
             LOGGER.info(f"Got xray centres, top 5: {xrc_results[:5]}")
-            if xrc_results:
+            filtered_results = [
+                result
+                for result in xrc_results
+                if result["total_count"] >= ZOCALO_MIN_TOTAL_COUNT_THRESHOLD
+            ]
+            discarded_count = len(xrc_results) - len(filtered_results)
+            if discarded_count > 0:
+                LOGGER.info(
+                    f"Removed {discarded_count} results because below threshold"
+                )
+            if filtered_results:
                 flyscan_results = [
                     _xrc_result_in_boxes_to_result_in_mm(xr, parameters)
-                    for xr in xrc_results
+                    for xr in filtered_results
                 ]
             else:
                 LOGGER.warning("No X-ray centre received")

src/mx_bluesky/hyperion/experiment_plans/robot_load_then_centre_plan.py

@@ -1,10 +1,12 @@
 from __future__ import annotations
 
+from math import isclose
 from typing import cast
 
 import bluesky.preprocessors as bpp
 import pydantic
 from blueapi.core import BlueskyContext
+from bluesky import plan_stubs as bps
 from bluesky.utils import MsgGenerator
 from dodal.devices.aperturescatterguard import ApertureScatterguard
 from dodal.devices.attenuator import Attenuator
@@ -174,7 +176,11 @@ def robot_load_then_xray_centre(
         yield from pin_already_loaded(composite.robot, sample_location)
     )
 
-    doing_chi_change = parameters.chi_start_deg is not None
+    current_chi = yield from bps.rd(composite.smargon.chi)
+    LOGGER.info(f"Read back current smargon chi of {current_chi} degrees.")
+    doing_chi_change = parameters.chi_start_deg is not None and not isclose(
+        current_chi, parameters.chi_start_deg, abs_tol=0.001
+    )
 
     if doing_sample_load:
         LOGGER.info("Pin not loaded, loading and centring")

tests/system_tests/hyperion/external_interaction/conftest.py

@@ -69,22 +69,32 @@
     {
         "centre_of_mass": [1, 2, 3],
         "max_voxel": [2, 4, 5],
-        "max_count": 105062,
+        "max_count": 50000,
         "n_voxels": 35,
-        "total_count": 2387574,
+        "total_count": 100000,
         "bounding_box": [[1, 2, 3], [3, 4, 4]],
     }
 ]
 TEST_RESULT_SMALL = [
     {
         "centre_of_mass": [1, 2, 3],
         "max_voxel": [1, 2, 3],
-        "max_count": 105062,
+        "max_count": 1000,
         "n_voxels": 35,
-        "total_count": 1387574,
+        "total_count": 1000,
         "bounding_box": [[2, 2, 2], [3, 3, 3]],
     }
 ]
+TEST_RESULT_BELOW_THRESHOLD = [
+    {
+        "centre_of_mass": [2, 3, 4],
+        "max_voxel": [2, 3, 4],
+        "max_count": 2,
+        "n_voxels": 1,
+        "total_count": 2,
+        "bounding_box": [[1, 2, 3], [2, 3, 4]],
+    }
+]
 
 
 def get_current_datacollection_comment(Session: Callable, dcid: int) -> str:

tests/unit_tests/hyperion/device_setup_plans/test_manipulate_sample.py

@@ -1,12 +1,13 @@
 from unittest.mock import MagicMock, call, patch
 
-import numpy as np
 import pytest
 from bluesky.run_engine import RunEngine
 from dodal.devices.aperturescatterguard import ApertureScatterguard, ApertureValue
 
 from mx_bluesky.hyperion.device_setup_plans.manipulate_sample import (
     move_aperture_if_required,
+    move_phi_chi_omega,
+    move_x_y_z,
 )
 from mx_bluesky.hyperion.experiment_plans.flyscan_xray_centre_plan import (
     FlyScanXRayCentreComposite,
@@ -43,36 +44,85 @@ async def test_move_aperture_does_nothing_when_none_selected(
         mock_set.assert_not_called()
 
 
+@pytest.mark.parametrize(
+    "motor_position, expected_moves",
+    [
+        [[1, 2, 3], [1, 2, 3]],
+        [[0, 0, 0], [None, None, None]],
+        [[None, None, None], [None, None, None]],
+        [[1, 0, 0], [1, 0, 0]],
+        [[0, 1, 0], [0, 1, 0]],
+        [[0, 0, 1], [0, 0, 1]],
+        [[1, None, None], [1, None, None]],
+        [[None, 1, None], [None, 1, None]],
+        [[None, None, 1], [None, None, 1]],
+    ],
+)
 @patch("bluesky.plan_stubs.abs_set", autospec=True)
-def test_results_passed_to_move_motors(
+def test_move_x_y_z(
     bps_abs_set: MagicMock,
     test_fgs_params: HyperionThreeDGridScan,
     fake_fgs_composite: FlyScanXRayCentreComposite,
     RE: RunEngine,
+    motor_position: list[float],
+    expected_moves: list[float | None],
 ):
-    from mx_bluesky.hyperion.device_setup_plans.manipulate_sample import move_x_y_z
-
-    motor_position = test_fgs_params.FGS_params.grid_position_to_motor_position(
-        np.array([1, 2, 3])
-    )
-    RE(move_x_y_z(fake_fgs_composite.sample_motors, *motor_position))
-    bps_abs_set.assert_has_calls(
-        [
-            call(
+    RE(move_x_y_z(fake_fgs_composite.sample_motors, *motor_position))  # type: ignore
+    expected_calls = [
+        call(axis, pos, group="move_x_y_z")
+        for axis, pos in zip(
+            [
                 fake_fgs_composite.sample_motors.x,
-                motor_position[0],
-                group="move_x_y_z",
-            ),
-            call(
                 fake_fgs_composite.sample_motors.y,
-                motor_position[1],
-                group="move_x_y_z",
-            ),
-            call(
                 fake_fgs_composite.sample_motors.z,
-                motor_position[2],
-                group="move_x_y_z",
-            ),
-        ],
+            ],
+            expected_moves,
+            strict=False,
+        )
+        if pos is not None
+    ]
+    bps_abs_set.assert_has_calls(
+        expected_calls,
+        any_order=True,
+    )
+
+
+@pytest.mark.parametrize(
+    "motor_position, expected_moves",
+    [
+        [[1, 2, 3], [1, 2, 3]],
+        [[0, 0, 0], [0, 0, 0]],
+        [[0, None, None], [0, None, None]],
+        [[None, 0, None], [None, 0, None]],
+        [[None, None, 0], [None, None, 0]],
+        [[None, None, None], [None, None, None]],
+        [[1, 0, 0], [1, 0, 0]],
+    ],
+)
+@patch("bluesky.plan_stubs.abs_set", autospec=True)
+def test_move_phi_chi_omega(
+    bps_abs_set: MagicMock,
+    test_fgs_params: HyperionThreeDGridScan,
+    fake_fgs_composite: FlyScanXRayCentreComposite,
+    RE: RunEngine,
+    motor_position: list[float],
+    expected_moves: list[float | None],
+):
+    RE(move_phi_chi_omega(fake_fgs_composite.sample_motors, *motor_position))  # type: ignore
+    expected_calls = [
+        call(axis, pos, group="move_phi_chi_omega")
+        for axis, pos in zip(
+            [
+                fake_fgs_composite.sample_motors.phi,
+                fake_fgs_composite.sample_motors.chi,
+                fake_fgs_composite.sample_motors.omega,
+            ],
+            expected_moves,
+            strict=False,
+        )
+        if pos is not None
+    ]
+    bps_abs_set.assert_has_calls(
+        expected_calls,
         any_order=True,
     )