From 9592db29f8d1df887dcd2d28f581f5af9564f5da Mon Sep 17 00:00:00 2001 From: Stanislaw Malinowski Date: Wed, 16 Oct 2024 11:22:23 +0000 Subject: [PATCH 01/14] add the class and a test --- src/dodal/common/crystal_metadata.py | 62 +++++++++++++++++++++++++++ tests/common/test_crystal_metadata.py | 39 +++++++++++++++++ 2 files changed, 101 insertions(+) create mode 100644 src/dodal/common/crystal_metadata.py create mode 100644 tests/common/test_crystal_metadata.py diff --git a/src/dodal/common/crystal_metadata.py b/src/dodal/common/crystal_metadata.py new file mode 100644 index 0000000000..88324d9285 --- /dev/null +++ b/src/dodal/common/crystal_metadata.py @@ -0,0 +1,62 @@ +import math +from dataclasses import dataclass +from enum import Enum +from typing import Literal + + +@dataclass(frozen=True) +class Material: + """ + Class representing a crystalline material with a specific lattice parameter. + """ + + name: str + lattice_parameter: float # Lattice parameter in meters + + +class MaterialsEnum(Enum): + Si = Material(name="silicon", lattice_parameter=5.4310205e-10) + Ge = Material(name="germanium", lattice_parameter=5.6575e-10) + + +@dataclass(frozen=True) +class CrystalMetadata: + """ + Metadata used in the NeXus format, + see https://manual.nexusformat.org/classes/base_classes/NXcrystal.html + """ + + usage: Literal["Bragg", "Laue"] | None = None + type: str | None = None + reflection: tuple[int, int, int] | None = None + d_spacing: tuple[float, str] | None = None + + def __init__( + self, material: MaterialsEnum, reflection_plane: tuple[int, int, int] + ) -> None: + # Determine material from reflection plane prefix + # Set attributes using object.__setattr__ since the class is frozen + assert all( + i > 0 for i in reflection_plane + ), "Reflection plane indices must be positive integers" + object.__setattr__(self, "type", material.value.name) + object.__setattr__(self, "reflection", reflection_plane) + d_spacing = self.calculate_d_spacing( + material.value.lattice_parameter, reflection_plane + ) + object.__setattr__(self, "d_spacing", d_spacing) + object.__setattr__(self, "usage", "Bragg") # Assuming "Bragg" usage by default + + @staticmethod + def calculate_d_spacing( + lattice_parameter: float, reflection: tuple[int, int, int] + ) -> tuple[float, str]: + """ + Calculates the d-spacing value in nanometers based on the given lattice parameter and reflection indices. + """ + h_index, k_index, l_index = reflection + d_spacing_m = lattice_parameter / math.sqrt( + h_index**2 + k_index**2 + l_index**2 + ) + d_spacing_nm = d_spacing_m * 1e9 # Convert meters to nanometers + return round(d_spacing_nm, 5), "nm" diff --git a/tests/common/test_crystal_metadata.py b/tests/common/test_crystal_metadata.py new file mode 100644 index 0000000000..d110b07f71 --- /dev/null +++ b/tests/common/test_crystal_metadata.py @@ -0,0 +1,39 @@ +import pytest + +from dodal.common.crystal_metadata import CrystalMetadata, MaterialsEnum + + +def test_happy_path_silicon(): + crystal_metadata = CrystalMetadata( + material=MaterialsEnum.Si, reflection_plane=(3, 1, 1) + ) + + # Check the values + assert crystal_metadata.type == "silicon" + assert crystal_metadata.reflection == (3, 1, 1) + assert crystal_metadata.d_spacing == pytest.approx( + (0.16375, "nm"), rel=1e-3 + ) # Allow for small tolerance + assert crystal_metadata.usage == "Bragg" + + +def test_happy_path_germanium(): + crystal_metadata = CrystalMetadata( + material=MaterialsEnum.Ge, reflection_plane=(1, 1, 1) + ) + + # Check the values + assert crystal_metadata.type == "germanium" + assert crystal_metadata.reflection == (1, 1, 1) + assert crystal_metadata.d_spacing == pytest.approx( + (0.326633, "nm"), rel=1e-3 + ) # Allow for small tolerance + assert crystal_metadata.usage == "Bragg" + + +def test_invalid_reflection_plane_with_negative_number(): + with pytest.raises( + AssertionError, + match="Reflection plane indices must be positive integers", + ): + CrystalMetadata(material=MaterialsEnum.Si, reflection_plane=(-1, 2, 3)) From f707b510bf3e067f0d91cacabfca7ad2b123a3bc Mon Sep 17 00:00:00 2001 From: Stanislaw Malinowski Date: Wed, 16 Oct 2024 11:31:35 +0000 Subject: [PATCH 02/14] add to the beamline files --- src/dodal/beamlines/i22.py | 15 ++++++--------- src/dodal/beamlines/p38.py | 15 ++++++--------- src/dodal/common/crystal_metadata.py | 7 +++++-- src/dodal/devices/i22/dcm.py | 23 ++++------------------- tests/devices/i22/test_dcm.py | 13 +++++-------- 5 files changed, 26 insertions(+), 47 deletions(-) diff --git a/src/dodal/beamlines/i22.py b/src/dodal/beamlines/i22.py index 80797c64ef..d73539fe8a 100644 --- a/src/dodal/beamlines/i22.py +++ b/src/dodal/beamlines/i22.py @@ -11,9 +11,10 @@ ) from dodal.common.beamlines.beamline_utils import set_beamline as set_utils_beamline from dodal.common.beamlines.device_helpers import numbered_slits +from dodal.common.crystal_metadata import CrystalMetadata, MaterialsEnum from dodal.common.visit import RemoteDirectoryServiceClient, StaticVisitPathProvider from dodal.devices.focusing_mirror import FocusingMirror -from dodal.devices.i22.dcm import CrystalMetadata, DoubleCrystalMonochromator +from dodal.devices.i22.dcm import DoubleCrystalMonochromator from dodal.devices.i22.fswitch import FSwitch from dodal.devices.i22.nxsas import NXSasMetadataHolder, NXSasOAV, NXSasPilatus from dodal.devices.linkam3 import Linkam3 @@ -171,16 +172,12 @@ def dcm( bl_prefix=False, temperature_prefix=f"{BeamlinePrefix(BL).beamline_prefix}-DI-DCM-01:", crystal_1_metadata=CrystalMetadata( - usage="Bragg", - type="silicon", - reflection=(1, 1, 1), - d_spacing=(3.13475, "nm"), + material=MaterialsEnum.Si, + reflection_plane=(1, 1, 1), ), crystal_2_metadata=CrystalMetadata( - usage="Bragg", - type="silicon", - reflection=(1, 1, 1), - d_spacing=(3.13475, "nm"), + material=MaterialsEnum.Si, + reflection_plane=(1, 1, 1), ), ) diff --git a/src/dodal/beamlines/p38.py b/src/dodal/beamlines/p38.py index 07bf0d7e80..06bd1c50eb 100644 --- a/src/dodal/beamlines/p38.py +++ b/src/dodal/beamlines/p38.py @@ -10,9 +10,10 @@ ) from dodal.common.beamlines.beamline_utils import set_beamline as set_utils_beamline from dodal.common.beamlines.device_helpers import numbered_slits +from dodal.common.crystal_metadata import CrystalMetadata, MaterialsEnum from dodal.common.visit import LocalDirectoryServiceClient, StaticVisitPathProvider from dodal.devices.focusing_mirror import FocusingMirror -from dodal.devices.i22.dcm import CrystalMetadata, DoubleCrystalMonochromator +from dodal.devices.i22.dcm import DoubleCrystalMonochromator from dodal.devices.i22.fswitch import FSwitch from dodal.devices.linkam3 import Linkam3 from dodal.devices.slits import Slits @@ -228,16 +229,12 @@ def dcm( bl_prefix=False, temperature_prefix=f"{BeamlinePrefix(BL).beamline_prefix}-DI-DCM-01:", crystal_1_metadata=CrystalMetadata( - usage="Bragg", - type="silicon", - reflection=(1, 1, 1), - d_spacing=(3.13475, "nm"), + material=MaterialsEnum.Si, + reflection_plane=(1, 1, 1), ), crystal_2_metadata=CrystalMetadata( - usage="Bragg", - type="silicon", - reflection=(1, 1, 1), - d_spacing=(3.13475, "nm"), + material=MaterialsEnum.Si, + reflection_plane=(1, 1, 1), ), ) diff --git a/src/dodal/common/crystal_metadata.py b/src/dodal/common/crystal_metadata.py index 88324d9285..9ce70cefef 100644 --- a/src/dodal/common/crystal_metadata.py +++ b/src/dodal/common/crystal_metadata.py @@ -32,7 +32,10 @@ class CrystalMetadata: d_spacing: tuple[float, str] | None = None def __init__( - self, material: MaterialsEnum, reflection_plane: tuple[int, int, int] + self, + material: MaterialsEnum, + reflection_plane: tuple[int, int, int], + usage: str = "Bragg", ) -> None: # Determine material from reflection plane prefix # Set attributes using object.__setattr__ since the class is frozen @@ -45,7 +48,7 @@ def __init__( material.value.lattice_parameter, reflection_plane ) object.__setattr__(self, "d_spacing", d_spacing) - object.__setattr__(self, "usage", "Bragg") # Assuming "Bragg" usage by default + object.__setattr__(self, "usage", usage) @staticmethod def calculate_d_spacing( diff --git a/src/dodal/devices/i22/dcm.py b/src/dodal/devices/i22/dcm.py index 70a46a30a8..fdf8d6addb 100644 --- a/src/dodal/devices/i22/dcm.py +++ b/src/dodal/devices/i22/dcm.py @@ -1,6 +1,4 @@ import time -from dataclasses import dataclass -from typing import Literal import numpy as np from bluesky.protocols import Reading @@ -14,24 +12,13 @@ from ophyd_async.epics.motor import Motor from ophyd_async.epics.signal import epics_signal_r +from dodal.common.crystal_metadata import CrystalMetadata + # Conversion constant for energy and wavelength, taken from the X-Ray data booklet # Converts between energy in KeV and wavelength in angstrom _CONVERSION_CONSTANT = 12.3984 -@dataclass(frozen=True, unsafe_hash=True) -class CrystalMetadata: - """ - Metadata used in the NeXus format, - see https://manual.nexusformat.org/classes/base_classes/NXcrystal.html - """ - - usage: Literal["Bragg", "Laue"] | None = None - type: str | None = None - reflection: tuple[int, int, int] | None = None - d_spacing: tuple[float, str] | None = None - - class DoubleCrystalMonochromator(StandardReadable): """ A double crystal monochromator (DCM), used to select the energy of the beam. @@ -45,8 +32,8 @@ class DoubleCrystalMonochromator(StandardReadable): def __init__( self, temperature_prefix: str, - crystal_1_metadata: CrystalMetadata | None = None, - crystal_2_metadata: CrystalMetadata | None = None, + crystal_1_metadata: CrystalMetadata, + crystal_2_metadata: CrystalMetadata, prefix: str = "", name: str = "", ) -> None: @@ -74,8 +61,6 @@ def __init__( # Soft metadata # If supplied include crystal details in output of read_configuration - crystal_1_metadata = crystal_1_metadata or CrystalMetadata() - crystal_2_metadata = crystal_2_metadata or CrystalMetadata() with self.add_children_as_readables(ConfigSignal): if crystal_1_metadata.usage is not None: self.crystal_1_usage, _ = soft_signal_r_and_setter( diff --git a/tests/devices/i22/test_dcm.py b/tests/devices/i22/test_dcm.py index e32588c0a2..359467d634 100644 --- a/tests/devices/i22/test_dcm.py +++ b/tests/devices/i22/test_dcm.py @@ -12,6 +12,7 @@ set_mock_value, ) +from dodal.common.crystal_metadata import MaterialsEnum from dodal.devices.i22.dcm import CrystalMetadata, DoubleCrystalMonochromator @@ -22,16 +23,12 @@ async def dcm() -> DoubleCrystalMonochromator: prefix="FOO-MO", temperature_prefix="FOO-DI", crystal_1_metadata=CrystalMetadata( - usage="Bragg", - type="silicon", - reflection=(1, 1, 1), - d_spacing=(3.13475, "mm"), + material=MaterialsEnum.Si, + reflection_plane=(1, 1, 1), ), crystal_2_metadata=CrystalMetadata( - usage="Bragg", - type="silicon", - reflection=(1, 1, 1), - d_spacing=(3.13475, "mm"), + material=MaterialsEnum.Si, + reflection_plane=(1, 1, 1), ), ) From 103e177625ebb349d9b18bf2e36f9c18f66e0d59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stanis=C5=82aw=20Malinowski?= <56644812+stan-dot@users.noreply.github.com> Date: Mon, 21 Oct 2024 12:15:08 +0100 Subject: [PATCH 03/14] Fix the expected test spacing values Previously this was hard coded to be a static float in milimeters for some reason, forgot to update this for this PR --- tests/devices/i22/test_dcm.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/devices/i22/test_dcm.py b/tests/devices/i22/test_dcm.py index 359467d634..1578210749 100644 --- a/tests/devices/i22/test_dcm.py +++ b/tests/devices/i22/test_dcm.py @@ -247,7 +247,7 @@ async def test_configuration(dcm: DoubleCrystalMonochromator): "alarm_severity": ANY, }, "dcm-crystal_2_d_spacing": { - "value": 3.13475, + "value": 0.31356, "timestamp": ANY, "alarm_severity": ANY, }, @@ -272,7 +272,7 @@ async def test_configuration(dcm: DoubleCrystalMonochromator): "alarm_severity": ANY, }, "dcm-crystal_1_d_spacing": { - "value": 3.13475, + "value": 0.31356, "timestamp": ANY, "alarm_severity": ANY, }, From 0aee3eaf26897ae917e1cdb97a2299c0d76bdce2 Mon Sep 17 00:00:00 2001 From: rtuck99 Date: Mon, 28 Oct 2024 11:00:51 +0000 Subject: [PATCH 04/14] Implement crystal metadata for I03 DCM (#870) * Implement crystal metadata for I03 DCM * Make type-checking happy --- src/dodal/devices/dcm.py | 24 +++++++++++++++++++++++- tests/devices/unit_tests/test_dcm.py | 4 ++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/src/dodal/devices/dcm.py b/src/dodal/devices/dcm.py index cb303ce617..9b9b8bedf4 100644 --- a/src/dodal/devices/dcm.py +++ b/src/dodal/devices/dcm.py @@ -1,7 +1,11 @@ -from ophyd_async.core import StandardReadable +from collections.abc import Sequence + +from ophyd_async.core import StandardReadable, soft_signal_r_and_setter from ophyd_async.epics.motor import Motor from ophyd_async.epics.signal import epics_signal_r +from dodal.common.crystal_metadata import CrystalMetadata, MaterialsEnum + class DCM(StandardReadable): """ @@ -17,6 +21,9 @@ def __init__( self, prefix: str, name: str = "", + crystal_metadata: CrystalMetadata = CrystalMetadata( # noqa: B008 + MaterialsEnum.Si, (1, 1, 1) + ), ) -> None: with self.add_children_as_readables(): self.bragg_in_degrees = Motor(prefix + "BRAGG") @@ -36,4 +43,19 @@ def __init__( self.perp_temp = epics_signal_r(float, prefix + "TEMP6") self.perp_sub_assembly_temp = epics_signal_r(float, prefix + "TEMP7") + self.crystal_metadata_usage, _ = soft_signal_r_and_setter( + str, initial_value=crystal_metadata.usage + ) + self.crystal_metadata_type, _ = soft_signal_r_and_setter( + str, initial_value=crystal_metadata.type + ) + self.crystal_metadata_reflection, _ = soft_signal_r_and_setter( + Sequence[int], + initial_value=list(crystal_metadata.reflection), # type: ignore + ) + self.crystal_metadata_d_spacing, _ = soft_signal_r_and_setter( + float, + initial_value=crystal_metadata.d_spacing[0], # type: ignore + units=crystal_metadata.d_spacing[1], # type: ignore + ) super().__init__(name) diff --git a/tests/devices/unit_tests/test_dcm.py b/tests/devices/unit_tests/test_dcm.py index 60fbc9076b..5fe5bc4af6 100644 --- a/tests/devices/unit_tests/test_dcm.py +++ b/tests/devices/unit_tests/test_dcm.py @@ -20,6 +20,10 @@ async def dcm() -> DCM: "dcm-bragg_in_degrees", "dcm-energy_in_kev", "dcm-offset_in_mm", + "dcm-crystal_metadata_usage", + "dcm-crystal_metadata_type", + "dcm-crystal_metadata_reflection", + "dcm-crystal_metadata_d_spacing", ], ) async def test_read_and_describe_includes( From 11871a62a3428825896bd7b10235a004c8e543c1 Mon Sep 17 00:00:00 2001 From: Stanislaw Malinowski Date: Mon, 28 Oct 2024 14:06:09 +0000 Subject: [PATCH 05/14] make d spacing a signal --- src/dodal/devices/dcm.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/dodal/devices/dcm.py b/src/dodal/devices/dcm.py index 9b9b8bedf4..223fb1d168 100644 --- a/src/dodal/devices/dcm.py +++ b/src/dodal/devices/dcm.py @@ -53,9 +53,6 @@ def __init__( Sequence[int], initial_value=list(crystal_metadata.reflection), # type: ignore ) - self.crystal_metadata_d_spacing, _ = soft_signal_r_and_setter( - float, - initial_value=crystal_metadata.d_spacing[0], # type: ignore - units=crystal_metadata.d_spacing[1], # type: ignore - ) + # todo note this fails at i18 for now, waiting for an EPICS fix + self.crystal_metadata_d_spacing = epics_signal_r(float, "DSPACING") super().__init__(name) From edd9fef138a2f34663d38ddfff675fd833f2c7e2 Mon Sep 17 00:00:00 2001 From: Stanislaw Malinowski Date: Mon, 4 Nov 2024 12:19:49 +0000 Subject: [PATCH 06/14] delete the absent keys test --- tests/devices/i22/test_dcm.py | 23 ----------------------- 1 file changed, 23 deletions(-) diff --git a/tests/devices/i22/test_dcm.py b/tests/devices/i22/test_dcm.py index 1578210749..ce4ed58631 100644 --- a/tests/devices/i22/test_dcm.py +++ b/tests/devices/i22/test_dcm.py @@ -50,29 +50,6 @@ def test_count_dcm( ) -async def test_crystal_metadata_not_propagated_when_not_supplied(): - async with DeviceCollector(mock=True): - dcm = DoubleCrystalMonochromator( - prefix="FOO-MO", - temperature_prefix="FOO-DI", - crystal_1_metadata=None, - crystal_2_metadata=None, - ) - - configuration = await dcm.read_configuration() - expected_absent_keys = { - "crystal-1-usage", - "crystal-1-type", - "crystal-1-reflection", - "crystal-1-d_spacing", - "crystal-2-usage", - "crystal-2-type", - "crystal-2-reflection", - "crystal-2-d_spacing", - } - assert expected_absent_keys.isdisjoint(configuration) - - @pytest.mark.parametrize( "energy,wavelength", [ From f2e892070bda8d7477e4813440372fd3d33e2cc6 Mon Sep 17 00:00:00 2001 From: Stanislaw Malinowski Date: Mon, 4 Nov 2024 12:34:47 +0000 Subject: [PATCH 07/14] remove the d spacing calculation --- src/dodal/common/crystal_metadata.py | 20 -------------------- src/dodal/devices/dcm.py | 3 +-- 2 files changed, 1 insertion(+), 22 deletions(-) diff --git a/src/dodal/common/crystal_metadata.py b/src/dodal/common/crystal_metadata.py index 9ce70cefef..4600820366 100644 --- a/src/dodal/common/crystal_metadata.py +++ b/src/dodal/common/crystal_metadata.py @@ -1,4 +1,3 @@ -import math from dataclasses import dataclass from enum import Enum from typing import Literal @@ -29,7 +28,6 @@ class CrystalMetadata: usage: Literal["Bragg", "Laue"] | None = None type: str | None = None reflection: tuple[int, int, int] | None = None - d_spacing: tuple[float, str] | None = None def __init__( self, @@ -44,22 +42,4 @@ def __init__( ), "Reflection plane indices must be positive integers" object.__setattr__(self, "type", material.value.name) object.__setattr__(self, "reflection", reflection_plane) - d_spacing = self.calculate_d_spacing( - material.value.lattice_parameter, reflection_plane - ) - object.__setattr__(self, "d_spacing", d_spacing) object.__setattr__(self, "usage", usage) - - @staticmethod - def calculate_d_spacing( - lattice_parameter: float, reflection: tuple[int, int, int] - ) -> tuple[float, str]: - """ - Calculates the d-spacing value in nanometers based on the given lattice parameter and reflection indices. - """ - h_index, k_index, l_index = reflection - d_spacing_m = lattice_parameter / math.sqrt( - h_index**2 + k_index**2 + l_index**2 - ) - d_spacing_nm = d_spacing_m * 1e9 # Convert meters to nanometers - return round(d_spacing_nm, 5), "nm" diff --git a/src/dodal/devices/dcm.py b/src/dodal/devices/dcm.py index 223fb1d168..6f0e7569f1 100644 --- a/src/dodal/devices/dcm.py +++ b/src/dodal/devices/dcm.py @@ -53,6 +53,5 @@ def __init__( Sequence[int], initial_value=list(crystal_metadata.reflection), # type: ignore ) - # todo note this fails at i18 for now, waiting for an EPICS fix - self.crystal_metadata_d_spacing = epics_signal_r(float, "DSPACING") + self.crystal_metadata_d_spacing = epics_signal_r(float, "DSPACING:RBV") super().__init__(name) From 060784d8bd98d09ef97147da029430c178168f8f Mon Sep 17 00:00:00 2001 From: Stanislaw Malinowski Date: Mon, 4 Nov 2024 13:55:40 +0000 Subject: [PATCH 08/14] respond to feedback --- src/dodal/beamlines/i22.py | 16 ++++++---- src/dodal/beamlines/p38.py | 15 +++++---- src/dodal/common/crystal_metadata.py | 46 ++++++++++++++++++--------- src/dodal/devices/dcm.py | 17 ++++++---- tests/common/test_crystal_metadata.py | 16 ++++------ tests/devices/i22/test_dcm.py | 19 ++++++----- 6 files changed, 75 insertions(+), 54 deletions(-) diff --git a/src/dodal/beamlines/i22.py b/src/dodal/beamlines/i22.py index d73539fe8a..264982f531 100644 --- a/src/dodal/beamlines/i22.py +++ b/src/dodal/beamlines/i22.py @@ -11,7 +11,10 @@ ) from dodal.common.beamlines.beamline_utils import set_beamline as set_utils_beamline from dodal.common.beamlines.device_helpers import numbered_slits -from dodal.common.crystal_metadata import CrystalMetadata, MaterialsEnum +from dodal.common.crystal_metadata import ( + MaterialsEnum, + make_crystal_metadata_from_material, +) from dodal.common.visit import RemoteDirectoryServiceClient, StaticVisitPathProvider from dodal.devices.focusing_mirror import FocusingMirror from dodal.devices.i22.dcm import DoubleCrystalMonochromator @@ -171,13 +174,12 @@ def dcm( fake_with_ophyd_sim, bl_prefix=False, temperature_prefix=f"{BeamlinePrefix(BL).beamline_prefix}-DI-DCM-01:", - crystal_1_metadata=CrystalMetadata( - material=MaterialsEnum.Si, - reflection_plane=(1, 1, 1), + crystal_1_metadata=make_crystal_metadata_from_material( + MaterialsEnum.Si, (1, 1, 1) ), - crystal_2_metadata=CrystalMetadata( - material=MaterialsEnum.Si, - reflection_plane=(1, 1, 1), + crystal_2_metadata=make_crystal_metadata_from_material( + MaterialsEnum.Si, + (1, 1, 1), ), ) diff --git a/src/dodal/beamlines/p38.py b/src/dodal/beamlines/p38.py index 06bd1c50eb..21f0112913 100644 --- a/src/dodal/beamlines/p38.py +++ b/src/dodal/beamlines/p38.py @@ -10,7 +10,10 @@ ) from dodal.common.beamlines.beamline_utils import set_beamline as set_utils_beamline from dodal.common.beamlines.device_helpers import numbered_slits -from dodal.common.crystal_metadata import CrystalMetadata, MaterialsEnum +from dodal.common.crystal_metadata import ( + MaterialsEnum, + make_crystal_metadata_from_material, +) from dodal.common.visit import LocalDirectoryServiceClient, StaticVisitPathProvider from dodal.devices.focusing_mirror import FocusingMirror from dodal.devices.i22.dcm import DoubleCrystalMonochromator @@ -228,13 +231,11 @@ def dcm( fake_with_ophyd_sim, bl_prefix=False, temperature_prefix=f"{BeamlinePrefix(BL).beamline_prefix}-DI-DCM-01:", - crystal_1_metadata=CrystalMetadata( - material=MaterialsEnum.Si, - reflection_plane=(1, 1, 1), + crystal_1_metadata=make_crystal_metadata_from_material( + MaterialsEnum.Si, (1, 1, 1) ), - crystal_2_metadata=CrystalMetadata( - material=MaterialsEnum.Si, - reflection_plane=(1, 1, 1), + crystal_2_metadata=make_crystal_metadata_from_material( + MaterialsEnum.Si, (1, 1, 1) ), ) diff --git a/src/dodal/common/crystal_metadata.py b/src/dodal/common/crystal_metadata.py index 4600820366..47833e69e7 100644 --- a/src/dodal/common/crystal_metadata.py +++ b/src/dodal/common/crystal_metadata.py @@ -1,3 +1,4 @@ +import math from dataclasses import dataclass from enum import Enum from typing import Literal @@ -28,18 +29,33 @@ class CrystalMetadata: usage: Literal["Bragg", "Laue"] | None = None type: str | None = None reflection: tuple[int, int, int] | None = None - - def __init__( - self, - material: MaterialsEnum, - reflection_plane: tuple[int, int, int], - usage: str = "Bragg", - ) -> None: - # Determine material from reflection plane prefix - # Set attributes using object.__setattr__ since the class is frozen - assert all( - i > 0 for i in reflection_plane - ), "Reflection plane indices must be positive integers" - object.__setattr__(self, "type", material.value.name) - object.__setattr__(self, "reflection", reflection_plane) - object.__setattr__(self, "usage", usage) + d_spacing: tuple[float, str] | None = None + + @staticmethod + def calculate_default_d_spacing( + lattice_parameter: float, reflection: tuple[int, int, int] + ) -> tuple[float, str]: + """ + Calculates the d-spacing value in nanometers based on the given lattice parameter and reflection indices. + """ + h_index, k_index, l_index = reflection + d_spacing_m = lattice_parameter / math.sqrt( + h_index**2 + k_index**2 + l_index**2 + ) + d_spacing_nm = d_spacing_m * 1e9 # Convert meters to nanometers + return round(d_spacing_nm, 5), "nm" + + +def make_crystal_metadata_from_material( + material: MaterialsEnum, + reflection_plane: tuple[int, int, int], + usage: Literal["Bragg", "Laue"] = "Bragg", + d_spacing_param: tuple[float, str] | None = None, +): + d_spacing = d_spacing_param or CrystalMetadata.calculate_default_d_spacing( + material.value.lattice_parameter, reflection_plane + ) + assert all( + isinstance(i, int) and i > 0 for i in reflection_plane + ), "Reflection plane indices must be positive integers" + return CrystalMetadata(usage, material.value.name, reflection_plane, d_spacing) diff --git a/src/dodal/devices/dcm.py b/src/dodal/devices/dcm.py index 6f0e7569f1..18d4e58f1b 100644 --- a/src/dodal/devices/dcm.py +++ b/src/dodal/devices/dcm.py @@ -4,7 +4,11 @@ from ophyd_async.epics.motor import Motor from ophyd_async.epics.signal import epics_signal_r -from dodal.common.crystal_metadata import CrystalMetadata, MaterialsEnum +from dodal.common.crystal_metadata import ( + CrystalMetadata, + MaterialsEnum, + make_crystal_metadata_from_material, +) class DCM(StandardReadable): @@ -21,10 +25,11 @@ def __init__( self, prefix: str, name: str = "", - crystal_metadata: CrystalMetadata = CrystalMetadata( # noqa: B008 - MaterialsEnum.Si, (1, 1, 1) - ), + crystal_metadata: CrystalMetadata | None = None, ) -> None: + cm = crystal_metadata or make_crystal_metadata_from_material( + MaterialsEnum.Si, (1, 1, 1) + ) with self.add_children_as_readables(): self.bragg_in_degrees = Motor(prefix + "BRAGG") self.roll_in_mrad = Motor(prefix + "ROLL") @@ -44,10 +49,10 @@ def __init__( self.perp_sub_assembly_temp = epics_signal_r(float, prefix + "TEMP7") self.crystal_metadata_usage, _ = soft_signal_r_and_setter( - str, initial_value=crystal_metadata.usage + str, initial_value=cm.usage ) self.crystal_metadata_type, _ = soft_signal_r_and_setter( - str, initial_value=crystal_metadata.type + str, initial_value=cm.type ) self.crystal_metadata_reflection, _ = soft_signal_r_and_setter( Sequence[int], diff --git a/tests/common/test_crystal_metadata.py b/tests/common/test_crystal_metadata.py index d110b07f71..3c92491c21 100644 --- a/tests/common/test_crystal_metadata.py +++ b/tests/common/test_crystal_metadata.py @@ -1,12 +1,13 @@ import pytest -from dodal.common.crystal_metadata import CrystalMetadata, MaterialsEnum +from dodal.common.crystal_metadata import ( + MaterialsEnum, + make_crystal_metadata_from_material, +) def test_happy_path_silicon(): - crystal_metadata = CrystalMetadata( - material=MaterialsEnum.Si, reflection_plane=(3, 1, 1) - ) + crystal_metadata = make_crystal_metadata_from_material(MaterialsEnum.Si, (3, 1, 1)) # Check the values assert crystal_metadata.type == "silicon" @@ -18,10 +19,7 @@ def test_happy_path_silicon(): def test_happy_path_germanium(): - crystal_metadata = CrystalMetadata( - material=MaterialsEnum.Ge, reflection_plane=(1, 1, 1) - ) - + crystal_metadata = make_crystal_metadata_from_material(MaterialsEnum.Ge, (1, 1, 1)) # Check the values assert crystal_metadata.type == "germanium" assert crystal_metadata.reflection == (1, 1, 1) @@ -36,4 +34,4 @@ def test_invalid_reflection_plane_with_negative_number(): AssertionError, match="Reflection plane indices must be positive integers", ): - CrystalMetadata(material=MaterialsEnum.Si, reflection_plane=(-1, 2, 3)) + make_crystal_metadata_from_material(MaterialsEnum.Si, (-1, 2, 3)) diff --git a/tests/devices/i22/test_dcm.py b/tests/devices/i22/test_dcm.py index ce4ed58631..ffe38b64c5 100644 --- a/tests/devices/i22/test_dcm.py +++ b/tests/devices/i22/test_dcm.py @@ -12,24 +12,23 @@ set_mock_value, ) -from dodal.common.crystal_metadata import MaterialsEnum -from dodal.devices.i22.dcm import CrystalMetadata, DoubleCrystalMonochromator +from dodal.common.crystal_metadata import ( + MaterialsEnum, + make_crystal_metadata_from_material, +) +from dodal.devices.i22.dcm import DoubleCrystalMonochromator @pytest.fixture async def dcm() -> DoubleCrystalMonochromator: + metadata_1 = make_crystal_metadata_from_material(MaterialsEnum.Si, (1, 1, 1)) + metadata_2 = make_crystal_metadata_from_material(MaterialsEnum.Si, (1, 1, 1)) async with DeviceCollector(mock=True): dcm = DoubleCrystalMonochromator( prefix="FOO-MO", temperature_prefix="FOO-DI", - crystal_1_metadata=CrystalMetadata( - material=MaterialsEnum.Si, - reflection_plane=(1, 1, 1), - ), - crystal_2_metadata=CrystalMetadata( - material=MaterialsEnum.Si, - reflection_plane=(1, 1, 1), - ), + crystal_1_metadata=metadata_1, + crystal_2_metadata=metadata_2, ) return dcm From b489f5767c5960ed6c12c641a5c6131d21a1da28 Mon Sep 17 00:00:00 2001 From: Stanislaw Malinowski Date: Tue, 5 Nov 2024 14:15:52 +0000 Subject: [PATCH 09/14] fix crystal metadata variable namign --- src/dodal/devices/dcm.py | 2 +- tests/devices/unit_tests/test_dcm.py | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/dodal/devices/dcm.py b/src/dodal/devices/dcm.py index 18d4e58f1b..46f3e83182 100644 --- a/src/dodal/devices/dcm.py +++ b/src/dodal/devices/dcm.py @@ -56,7 +56,7 @@ def __init__( ) self.crystal_metadata_reflection, _ = soft_signal_r_and_setter( Sequence[int], - initial_value=list(crystal_metadata.reflection), # type: ignore + initial_value=list(cm.reflection), # type: ignore ) self.crystal_metadata_d_spacing = epics_signal_r(float, "DSPACING:RBV") super().__init__(name) diff --git a/tests/devices/unit_tests/test_dcm.py b/tests/devices/unit_tests/test_dcm.py index 5fe5bc4af6..1a543da075 100644 --- a/tests/devices/unit_tests/test_dcm.py +++ b/tests/devices/unit_tests/test_dcm.py @@ -13,6 +13,13 @@ async def dcm() -> DCM: return dcm +async def test_metadata_reflection(dcm: DCM): + signal = dcm.crystal_metadata_reflection + v = await signal.read() + print(v) + assert v is not None, "Value is not clear" + + @pytest.mark.parametrize( "key", [ From be6aee63bbca57b9387f40ddb0c40b296ed4b7ba Mon Sep 17 00:00:00 2001 From: Stanislaw Malinowski Date: Tue, 5 Nov 2024 14:49:13 +0000 Subject: [PATCH 10/14] convert the types correctly --- src/dodal/devices/dcm.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/dodal/devices/dcm.py b/src/dodal/devices/dcm.py index 46f3e83182..e3c73ee9d1 100644 --- a/src/dodal/devices/dcm.py +++ b/src/dodal/devices/dcm.py @@ -1,5 +1,6 @@ -from collections.abc import Sequence +import numpy as np +from numpy.typing import NDArray from ophyd_async.core import StandardReadable, soft_signal_r_and_setter from ophyd_async.epics.motor import Motor from ophyd_async.epics.signal import epics_signal_r @@ -54,9 +55,10 @@ def __init__( self.crystal_metadata_type, _ = soft_signal_r_and_setter( str, initial_value=cm.type ) + reflection_array = np.array(cm.reflection) self.crystal_metadata_reflection, _ = soft_signal_r_and_setter( - Sequence[int], - initial_value=list(cm.reflection), # type: ignore + NDArray[np.uint64], + initial_value=reflection_array, ) self.crystal_metadata_d_spacing = epics_signal_r(float, "DSPACING:RBV") super().__init__(name) From 961aa706b29293ba07bbf7b546b623f70ff78ee5 Mon Sep 17 00:00:00 2001 From: Stanislaw Malinowski Date: Tue, 5 Nov 2024 14:53:23 +0000 Subject: [PATCH 11/14] ruff --- src/dodal/devices/dcm.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/dodal/devices/dcm.py b/src/dodal/devices/dcm.py index e3c73ee9d1..79652504c9 100644 --- a/src/dodal/devices/dcm.py +++ b/src/dodal/devices/dcm.py @@ -1,4 +1,3 @@ - import numpy as np from numpy.typing import NDArray from ophyd_async.core import StandardReadable, soft_signal_r_and_setter From 749bf8a65c97434b8da19b6115f6f378b62c0580 Mon Sep 17 00:00:00 2001 From: Stanislaw Malinowski Date: Tue, 5 Nov 2024 15:19:18 +0000 Subject: [PATCH 12/14] apparently this fixes tests and coverage --- src/dodal/common/crystal_metadata.py | 8 ++++---- src/dodal/devices/i22/dcm.py | 16 ---------------- 2 files changed, 4 insertions(+), 20 deletions(-) diff --git a/src/dodal/common/crystal_metadata.py b/src/dodal/common/crystal_metadata.py index 47833e69e7..836c69d01e 100644 --- a/src/dodal/common/crystal_metadata.py +++ b/src/dodal/common/crystal_metadata.py @@ -26,10 +26,10 @@ class CrystalMetadata: see https://manual.nexusformat.org/classes/base_classes/NXcrystal.html """ - usage: Literal["Bragg", "Laue"] | None = None - type: str | None = None - reflection: tuple[int, int, int] | None = None - d_spacing: tuple[float, str] | None = None + usage: Literal["Bragg", "Laue"] + type: str + reflection: tuple[int, int, int] + d_spacing: tuple[float, str] @staticmethod def calculate_default_d_spacing( diff --git a/src/dodal/devices/i22/dcm.py b/src/dodal/devices/i22/dcm.py index fdf8d6addb..d4573da0da 100644 --- a/src/dodal/devices/i22/dcm.py +++ b/src/dodal/devices/i22/dcm.py @@ -66,56 +66,40 @@ def __init__( self.crystal_1_usage, _ = soft_signal_r_and_setter( str, initial_value=crystal_1_metadata.usage ) - else: - self.crystal_1_usage = None if crystal_1_metadata.type is not None: self.crystal_1_type, _ = soft_signal_r_and_setter( str, initial_value=crystal_1_metadata.type ) - else: - self.crystal_1_type = None if crystal_1_metadata.reflection is not None: self.crystal_1_reflection, _ = soft_signal_r_and_setter( Array1D[np.int32], initial_value=np.array(crystal_1_metadata.reflection), ) - else: - self.crystal_1_reflection = None if crystal_1_metadata.d_spacing is not None: self.crystal_1_d_spacing, _ = soft_signal_r_and_setter( float, initial_value=crystal_1_metadata.d_spacing[0], units=crystal_1_metadata.d_spacing[1], ) - else: - self.crystal_1_d_spacing = None if crystal_2_metadata.usage is not None: self.crystal_2_usage, _ = soft_signal_r_and_setter( str, initial_value=crystal_2_metadata.usage ) - else: - self.crystal_2_usage = None if crystal_2_metadata.type is not None: self.crystal_2_type, _ = soft_signal_r_and_setter( str, initial_value=crystal_2_metadata.type ) - else: - self.crystal_2_type = None if crystal_2_metadata.reflection is not None: self.crystal_2_reflection, _ = soft_signal_r_and_setter( Array1D[np.int32], initial_value=np.array(crystal_2_metadata.reflection), ) - else: - self.crystal_2_reflection = None if crystal_2_metadata.d_spacing is not None: self.crystal_2_d_spacing, _ = soft_signal_r_and_setter( float, initial_value=crystal_2_metadata.d_spacing[0], units=crystal_2_metadata.d_spacing[1], ) - else: - self.crystal_2_d_spacing = None super().__init__(name) From cb9abcf7c80b2cc40328e40bead958873478a616 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stanis=C5=82aw=20Malinowski?= <56644812+stan-dot@users.noreply.github.com> Date: Wed, 6 Nov 2024 09:22:29 +0000 Subject: [PATCH 13/14] remove print --- tests/devices/unit_tests/test_dcm.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/devices/unit_tests/test_dcm.py b/tests/devices/unit_tests/test_dcm.py index 1a543da075..6c5c09fda5 100644 --- a/tests/devices/unit_tests/test_dcm.py +++ b/tests/devices/unit_tests/test_dcm.py @@ -16,7 +16,6 @@ async def dcm() -> DCM: async def test_metadata_reflection(dcm: DCM): signal = dcm.crystal_metadata_reflection v = await signal.read() - print(v) assert v is not None, "Value is not clear" From d3e79cd6d77c2fef0d4a32a0f898eeae8e5d67f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stanis=C5=82aw=20Malinowski?= <56644812+stan-dot@users.noreply.github.com> Date: Wed, 6 Nov 2024 09:26:48 +0000 Subject: [PATCH 14/14] remove is None checks --- src/dodal/devices/i22/dcm.py | 68 ++++++++++++++++-------------------- 1 file changed, 30 insertions(+), 38 deletions(-) diff --git a/src/dodal/devices/i22/dcm.py b/src/dodal/devices/i22/dcm.py index d4573da0da..59b4902f7e 100644 --- a/src/dodal/devices/i22/dcm.py +++ b/src/dodal/devices/i22/dcm.py @@ -62,44 +62,36 @@ def __init__( # Soft metadata # If supplied include crystal details in output of read_configuration with self.add_children_as_readables(ConfigSignal): - if crystal_1_metadata.usage is not None: - self.crystal_1_usage, _ = soft_signal_r_and_setter( - str, initial_value=crystal_1_metadata.usage - ) - if crystal_1_metadata.type is not None: - self.crystal_1_type, _ = soft_signal_r_and_setter( - str, initial_value=crystal_1_metadata.type - ) - if crystal_1_metadata.reflection is not None: - self.crystal_1_reflection, _ = soft_signal_r_and_setter( - Array1D[np.int32], - initial_value=np.array(crystal_1_metadata.reflection), - ) - if crystal_1_metadata.d_spacing is not None: - self.crystal_1_d_spacing, _ = soft_signal_r_and_setter( - float, - initial_value=crystal_1_metadata.d_spacing[0], - units=crystal_1_metadata.d_spacing[1], - ) - if crystal_2_metadata.usage is not None: - self.crystal_2_usage, _ = soft_signal_r_and_setter( - str, initial_value=crystal_2_metadata.usage - ) - if crystal_2_metadata.type is not None: - self.crystal_2_type, _ = soft_signal_r_and_setter( - str, initial_value=crystal_2_metadata.type - ) - if crystal_2_metadata.reflection is not None: - self.crystal_2_reflection, _ = soft_signal_r_and_setter( - Array1D[np.int32], - initial_value=np.array(crystal_2_metadata.reflection), - ) - if crystal_2_metadata.d_spacing is not None: - self.crystal_2_d_spacing, _ = soft_signal_r_and_setter( - float, - initial_value=crystal_2_metadata.d_spacing[0], - units=crystal_2_metadata.d_spacing[1], - ) + self.crystal_1_usage, _ = soft_signal_r_and_setter( + str, initial_value=crystal_1_metadata.usage + ) + self.crystal_1_type, _ = soft_signal_r_and_setter( + str, initial_value=crystal_1_metadata.type + ) + self.crystal_1_reflection, _ = soft_signal_r_and_setter( + Array1D[np.int32], + initial_value=np.array(crystal_1_metadata.reflection), + ) + self.crystal_1_d_spacing, _ = soft_signal_r_and_setter( + float, + initial_value=crystal_1_metadata.d_spacing[0], + units=crystal_1_metadata.d_spacing[1], + ) + self.crystal_2_usage, _ = soft_signal_r_and_setter( + str, initial_value=crystal_2_metadata.usage + ) + self.crystal_2_type, _ = soft_signal_r_and_setter( + str, initial_value=crystal_2_metadata.type + ) + self.crystal_2_reflection, _ = soft_signal_r_and_setter( + Array1D[np.int32], + initial_value=np.array(crystal_2_metadata.reflection), + ) + self.crystal_2_d_spacing, _ = soft_signal_r_and_setter( + float, + initial_value=crystal_2_metadata.d_spacing[0], + units=crystal_2_metadata.d_spacing[1], + ) super().__init__(name)