diff --git a/docs/changes/newsfragments/8679.breaking b/docs/changes/newsfragments/8679.breaking new file mode 100644 index 000000000000..aa3b30145dfd --- /dev/null +++ b/docs/changes/newsfragments/8679.breaking @@ -0,0 +1,12 @@ +The field units of an ``AMIModel430`` can no longer be changed while it is part of an +``AMIModel4303D``, and doing so now raises a ``RuntimeError``. The set point and field limit of +the 3D driver are in the field units of the magnets, so changing them silently changed their +meaning by a factor of 10. Set the field units before creating the 3D driver, or close the 3D +driver first. + +The coordinate vectors of ``AMIModel4303D`` (``cartesian``, ``spherical``, ``cylindrical`` and +their ``*_measured`` versions) now return a named tuple (``CartesianVector``, +``SphericalVector`` or ``CylindricalVector`` from ``qcodes.math_utils``) instead of a list, so +the components can also be accessed by name, e.g. ``cartesian_measured().z``. Code that compares +the result to a list, or modifies it, needs updating. Setting these parameters still accepts +lists, tuples and numpy arrays. diff --git a/docs/changes/newsfragments/8679.improved_driver b/docs/changes/newsfragments/8679.improved_driver new file mode 100644 index 000000000000..34a92b5306cb --- /dev/null +++ b/docs/changes/newsfragments/8679.improved_driver @@ -0,0 +1,7 @@ +The field parameters of ``AMIModel4303D``, such as ``cartesian``, ``x`` and ``field_measured``, +now use the field units of the individual magnets, tesla or kilogauss, instead of always +reporting tesla. A warning is logged if the magnets use different field units. Also removed the +placeholder unit ``tuple?`` of ``AMIModel4303D.spherical`` and ``cylindrical``, whose components +have different units, which are now described in their docstrings. Closing an +``AMIModel4303D`` now detaches its magnets, so ``set_field``, ``zero`` and changing the units of +a magnet that is still open no longer fail. diff --git a/docs/changes/newsfragments/8679.new b/docs/changes/newsfragments/8679.new new file mode 100644 index 000000000000..507312a7c804 --- /dev/null +++ b/docs/changes/newsfragments/8679.new @@ -0,0 +1,3 @@ +Added the named tuples ``CartesianVector`` (``x``, ``y``, ``z``), ``SphericalVector`` +(``field``, ``theta``, ``phi``) and ``CylindricalVector`` (``rho``, ``phi``, ``z``) to +``qcodes.math_utils``, for the components of a field vector in a given coordinate system. diff --git a/src/qcodes/instrument_drivers/american_magnetics/AMI430_visa.py b/src/qcodes/instrument_drivers/american_magnetics/AMI430_visa.py index 64d490b9eb7d..d89985792ef5 100644 --- a/src/qcodes/instrument_drivers/american_magnetics/AMI430_visa.py +++ b/src/qcodes/instrument_drivers/american_magnetics/AMI430_visa.py @@ -18,22 +18,39 @@ VisaInstrument, VisaInstrumentKWArgs, ) -from qcodes.math_utils import FieldVector +from qcodes.math_utils import ( + CartesianVector, + CylindricalVector, + FieldVector, + SphericalVector, +) from qcodes.metadatable import normalize_snapshot_update -from qcodes.parameters import Parameter from qcodes.utils.types import NumberType from qcodes.validators import Anything, Bool, Enum, Ints, Numbers if TYPE_CHECKING: from typing import Unpack + import numpy.typing as npt + from qcodes.metadatable import SnapshotUpdate + from qcodes.parameters import Parameter log = logging.getLogger(__name__) CartesianFieldLimitFunction = Callable[[float, float, float], bool] +# the FieldVector components of each coordinate vector +_VECTOR_COMPONENTS: dict[ + type[CartesianVector | SphericalVector | CylindricalVector], + tuple[str, str, str], +] = { + CartesianVector: ("x", "y", "z"), + SphericalVector: ("r", "theta", "phi"), + CylindricalVector: ("rho", "phi", "z"), +} + class AMI430Exception(Exception): pass @@ -259,7 +276,7 @@ def __init__( self.field_units: Parameter[str, Self] = self.add_parameter( "field_units", get_cmd="FIELD:UNITS?", - set_cmd=(lambda units: self._update_units(field_units=units)), + set_cmd=self._set_field_units, val_mapping={"kilogauss": 0, "tesla": 1}, ) """Parameter field_units""" @@ -551,6 +568,31 @@ def _update_coil_constant(self, new_coil_constant: float | None = None) -> float # Return new coil constant return new_coil_constant + def _set_field_units(self, field_units: int) -> None: + """ + Set the field units, 0 for kilogauss and 1 for tesla. + + Raises: + RuntimeError: if the units would change while this magnet is part + of a 3D driver, whose set point and field limit are in the + field units of the magnets. + + """ + if self._parent_instrument is not None: + assert self.field_units.inverse_val_mapping is not None + new_units = self._SHORT_UNITS[ + self.field_units.inverse_val_mapping[field_units] + ] + if new_units != self.field.unit: + raise RuntimeError( + f"Cannot change the field units of {self.name} while it is " + f"part of the 3D driver {self._parent_instrument.name}, " + "since its set point and field limit are in the field " + "units of the magnets. Set the field units before " + "creating the 3D driver, or close the 3D driver first." + ) + self._update_units(field_units=field_units) + def _update_units( self, ramp_rate_units: int | None = None, field_units: int | None = None ) -> None: @@ -663,7 +705,10 @@ def __init__( instance for controlling the Z axis of magnetic field field_limit: a number for maximum allows magnetic field or an iterable of callable field limit functions that define - region(s) of allowed values in 3D magnetic field space + region(s) of allowed values in 3D magnetic field space, in + the field units of the magnets (tesla or kilogauss). The + field units of the magnets cannot be changed while they are + part of this driver. **kwargs: kwargs are forwarded to base class. """ @@ -729,148 +774,176 @@ def find_ami430_with_name(ami430_name: str) -> AMIModel430: ) # Get-only parameters that return a measured value - self.cartesian_measured: Parameter = self.add_parameter( + self.cartesian_measured: Parameter[CartesianVector, Self] = self.add_parameter( "cartesian_measured", - get_cmd=partial(self._get_measured, "x", "y", "z"), + get_cmd=partial(self._get_measured, CartesianVector), unit="T", ) """Parameter cartesian_measured""" - self.x_measured: Parameter = self.add_parameter( - "x_measured", get_cmd=partial(self._get_measured, "x"), unit="T" + self.x_measured: Parameter[float, Self] = self.add_parameter( + "x_measured", get_cmd=partial(self._get_measured_component, "x"), unit="T" ) """Parameter x_measured""" - self.y_measured: Parameter = self.add_parameter( - "y_measured", get_cmd=partial(self._get_measured, "y"), unit="T" + self.y_measured: Parameter[float, Self] = self.add_parameter( + "y_measured", get_cmd=partial(self._get_measured_component, "y"), unit="T" ) """Parameter y_measured""" - self.z_measured: Parameter = self.add_parameter( - "z_measured", get_cmd=partial(self._get_measured, "z"), unit="T" + self.z_measured: Parameter[float, Self] = self.add_parameter( + "z_measured", get_cmd=partial(self._get_measured_component, "z"), unit="T" ) """Parameter z_measured""" - self.spherical_measured: Parameter = self.add_parameter( + self.spherical_measured: Parameter[SphericalVector, Self] = self.add_parameter( "spherical_measured", - get_cmd=partial(self._get_measured, "r", "theta", "phi"), + get_cmd=partial(self._get_measured, SphericalVector), unit="T", ) """Parameter spherical_measured""" - self.phi_measured: Parameter = self.add_parameter( - "phi_measured", get_cmd=partial(self._get_measured, "phi"), unit="deg" + self.phi_measured: Parameter[float, Self] = self.add_parameter( + "phi_measured", + get_cmd=partial(self._get_measured_component, "phi"), + unit="deg", ) """Parameter phi_measured""" - self.theta_measured: Parameter = self.add_parameter( - "theta_measured", get_cmd=partial(self._get_measured, "theta"), unit="deg" + self.theta_measured: Parameter[float, Self] = self.add_parameter( + "theta_measured", + get_cmd=partial(self._get_measured_component, "theta"), + unit="deg", ) """Parameter theta_measured""" - self.field_measured: Parameter = self.add_parameter( - "field_measured", get_cmd=partial(self._get_measured, "r"), unit="T" + self.field_measured: Parameter[float, Self] = self.add_parameter( + "field_measured", + get_cmd=partial(self._get_measured_component, "r"), + unit="T", ) """Parameter field_measured""" - self.cylindrical_measured: Parameter = self.add_parameter( - "cylindrical_measured", - get_cmd=partial(self._get_measured, "rho", "phi", "z"), - unit="T", + self.cylindrical_measured: Parameter[CylindricalVector, Self] = ( + self.add_parameter( + "cylindrical_measured", + get_cmd=partial(self._get_measured, CylindricalVector), + unit="T", + ) ) """Parameter cylindrical_measured""" - self.rho_measured: Parameter = self.add_parameter( - "rho_measured", get_cmd=partial(self._get_measured, "rho"), unit="T" + self.rho_measured: Parameter[float, Self] = self.add_parameter( + "rho_measured", + get_cmd=partial(self._get_measured_component, "rho"), + unit="T", ) """Parameter rho_measured""" - # Get and set parameters for the set points of the coordinates - self.cartesian: Parameter = self.add_parameter( - "cartesian", - get_cmd=partial(self._get_setpoints, ("x", "y", "z")), - set_cmd=partial(self._set_setpoints, ("x", "y", "z")), - unit="T", - vals=Anything(), + # Get and set parameters for the set points of the coordinates. These + # return a named tuple (CartesianVector, SphericalVector or + # CylindricalVector) but can be set from any sequence or a numpy array. + self.cartesian: Parameter[Sequence[float] | npt.NDArray, Self] = ( + self.add_parameter( + "cartesian", + get_cmd=partial(self._get_setpoints, CartesianVector), + set_cmd=partial(self._set_setpoints, ("x", "y", "z")), + unit="T", + vals=Anything(), + ) ) """Parameter cartesian""" - self.x: Parameter = self.add_parameter( + self.x: Parameter[float, Self] = self.add_parameter( "x", - get_cmd=partial(self._get_setpoints, ("x",)), - set_cmd=partial(self._set_setpoints, ("x",)), + get_cmd=partial(self._get_setpoint, "x"), + set_cmd=partial(self._set_setpoint, "x"), unit="T", vals=Numbers(), ) """Parameter x""" - self.y: Parameter = self.add_parameter( + self.y: Parameter[float, Self] = self.add_parameter( "y", - get_cmd=partial(self._get_setpoints, ("y",)), - set_cmd=partial(self._set_setpoints, ("y",)), + get_cmd=partial(self._get_setpoint, "y"), + set_cmd=partial(self._set_setpoint, "y"), unit="T", vals=Numbers(), ) """Parameter y""" - self.z: Parameter = self.add_parameter( + self.z: Parameter[float, Self] = self.add_parameter( "z", - get_cmd=partial(self._get_setpoints, ("z",)), - set_cmd=partial(self._set_setpoints, ("z",)), + get_cmd=partial(self._get_setpoint, "z"), + set_cmd=partial(self._set_setpoint, "z"), unit="T", vals=Numbers(), ) """Parameter z""" - self.spherical: Parameter = self.add_parameter( - "spherical", - get_cmd=partial(self._get_setpoints, ("r", "theta", "phi")), - set_cmd=partial(self._set_setpoints, ("r", "theta", "phi")), - unit="tuple?", - vals=Anything(), + self.spherical: Parameter[Sequence[float] | npt.NDArray, Self] = ( + self.add_parameter( + "spherical", + get_cmd=partial(self._get_setpoints, SphericalVector), + set_cmd=partial(self._set_setpoints, ("r", "theta", "phi")), + vals=Anything(), + docstring="Set point in spherical coordinates (field, theta, phi), " + "with the field in the field units of the magnets and the angles " + "in degrees.", + ) ) - """Parameter spherical""" + """ + Set point in spherical coordinates (field, theta, phi), with the field in + the field units of the magnets and the angles in degrees. + """ - self.phi: Parameter = self.add_parameter( + self.phi: Parameter[float, Self] = self.add_parameter( "phi", - get_cmd=partial(self._get_setpoints, ("phi",)), - set_cmd=partial(self._set_setpoints, ("phi",)), + get_cmd=partial(self._get_setpoint, "phi"), + set_cmd=partial(self._set_setpoint, "phi"), unit="deg", vals=Numbers(), ) """Parameter phi""" - self.theta: Parameter = self.add_parameter( + self.theta: Parameter[float, Self] = self.add_parameter( "theta", - get_cmd=partial(self._get_setpoints, ("theta",)), - set_cmd=partial(self._set_setpoints, ("theta",)), + get_cmd=partial(self._get_setpoint, "theta"), + set_cmd=partial(self._set_setpoint, "theta"), unit="deg", vals=Numbers(), ) """Parameter theta""" - self.field: Parameter = self.add_parameter( + self.field: Parameter[float, Self] = self.add_parameter( "field", - get_cmd=partial(self._get_setpoints, ("r",)), - set_cmd=partial(self._set_setpoints, ("r",)), + get_cmd=partial(self._get_setpoint, "r"), + set_cmd=partial(self._set_setpoint, "r"), unit="T", vals=Numbers(), ) """Parameter field""" - self.cylindrical: Parameter = self.add_parameter( - "cylindrical", - get_cmd=partial(self._get_setpoints, ("rho", "phi", "z")), - set_cmd=partial(self._set_setpoints, ("rho", "phi", "z")), - unit="tuple?", - vals=Anything(), + self.cylindrical: Parameter[Sequence[float] | npt.NDArray, Self] = ( + self.add_parameter( + "cylindrical", + get_cmd=partial(self._get_setpoints, CylindricalVector), + set_cmd=partial(self._set_setpoints, ("rho", "phi", "z")), + vals=Anything(), + docstring="Set point in cylindrical coordinates (rho, phi, z), " + "with rho and z in the field units of the magnets and phi in " + "degrees.", + ) ) - """Parameter cylindrical""" + """ + Set point in cylindrical coordinates (rho, phi, z), with rho and z in the + field units of the magnets and phi in degrees. + """ - self.rho: Parameter = self.add_parameter( + self.rho: Parameter[float, Self] = self.add_parameter( "rho", - get_cmd=partial(self._get_setpoints, ("rho",)), - set_cmd=partial(self._set_setpoints, ("rho",)), + get_cmd=partial(self._get_setpoint, "rho"), + set_cmd=partial(self._set_setpoint, "rho"), unit="T", vals=Numbers(), ) @@ -881,28 +954,28 @@ def find_ami430_with_name(ami430_name: str) -> AMIModel430: ) """Parameter block_during_ramp""" - self.ramp_mode = Parameter( - name="ramp_mode", - instrument=self, + self.ramp_mode: Parameter[str, Self] = self.add_parameter( + "ramp_mode", get_cmd=None, set_cmd=None, vals=Enum("default", "simultaneous"), initial_value="default", ) + """Parameter ramp_mode""" - self.ramping_state_check_interval = Parameter( - name="ramping_state_check_interval", - instrument=self, + self.ramping_state_check_interval: Parameter[float, Self] = self.add_parameter( + "ramping_state_check_interval", initial_value=0.05, unit="s", vals=Numbers(0, 10), set_cmd=None, get_cmd=None, ) + """Parameter ramping_state_check_interval""" - self.vector_ramp_rate = Parameter( - name="vector_ramp_rate", - instrument=self, + # None until set, which is checked before a simultaneous ramp + self.vector_ramp_rate: Parameter[float | None, Self] = self.add_parameter( + "vector_ramp_rate", unit="T/s", vals=Numbers(min_value=0.0), set_cmd=None, @@ -913,12 +986,67 @@ def find_ami430_with_name(ami430_name: str) -> AMIModel430: ) """Ramp rate along a line (vector) in 3D field space""" + # the field parameters are in the field units of the magnets + self._update_field_units() + self._exit_stack = ExitStack() def get_idn(self) -> dict[str, str | None]: idparts = ["American Magnetics", self.name, None, None] return dict(zip(("vendor", "model", "serial", "firmware"), idparts)) + def close(self) -> None: + # the magnets can stay open after this driver is closed, so they must + # no longer delegate to it. A magnet that was closed first no longer + # has the attribute. + for name in ("_instrument_x", "_instrument_y", "_instrument_z"): + instrument: AMIModel430 | None = getattr(self, name, None) + if ( + instrument is not None + and getattr(instrument, "_parent_instrument", None) is self + ): + instrument._parent_instrument = None + super().close() + + def _update_field_units(self) -> None: + """ + Use the field units of the magnets (tesla or kilogauss) as the unit of + the field parameters, since their values are in the units of the + magnets. The field units of the magnets cannot be changed while they + are part of this driver. + """ + instruments = (self._instrument_x, self._instrument_y, self._instrument_z) + units = {instrument.field.unit for instrument in instruments} + if len(units) == 1: + (unit,) = units + else: + unit = "" + self.log.warning( + "The magnets use different field units (%s), so the field " + "values of %s combine different units.", + ", ".join( + f"{instrument.short_name}: {instrument.field.unit}" + for instrument in instruments + ), + self.name, + ) + field_parameters: tuple[Parameter, ...] = ( + self.cartesian, + self.x, + self.y, + self.z, + self.field, + self.rho, + self.cartesian_measured, + self.x_measured, + self.y_measured, + self.z_measured, + self.field_measured, + self.rho_measured, + ) + for parameter in field_parameters: + parameter.unit = unit + def _set_vector_ramp_rate_units(self, val: float) -> float: _, common_ramp_rate_units = self._raise_if_not_same_field_and_ramp_rate_units() self.vector_ramp_rate.unit = common_ramp_rate_units @@ -1104,7 +1232,8 @@ def _adjust_child_instruments(self, values: tuple[float, float, float]) -> None: def _update_individual_axes_ramp_rates( self, values: tuple[float, float, float] ) -> None: - if self.vector_ramp_rate() is None or self.vector_ramp_rate() == 0: + vector_ramp_rate = self.vector_ramp_rate() + if vector_ramp_rate is None or vector_ramp_rate == 0: raise ValueError( "The value of the `vector_ramp_rate` Parameter is " "currently None or 0. Set it to an appropriate " @@ -1114,7 +1243,7 @@ def _update_individual_axes_ramp_rates( new_axes_ramp_rates = self.calculate_axes_ramp_rates_from_vector_ramp_rate( start=self._get_measured_field_vector(), setpoint=FieldVector(x=values[0], y=values[1], z=values[2]), - vector_ramp_rate=self.vector_ramp_rate.get(), + vector_ramp_rate=vector_ramp_rate, ) instruments = (self._instrument_x, self._instrument_y, self._instrument_z) for instrument, new_axis_ramp_rate in zip(instruments, new_axes_ramp_rates): @@ -1247,11 +1376,11 @@ def _request_field_change(self, instrument: AMIModel430, value: NumberType) -> N performed by this 3D driver. """ if instrument is self._instrument_x: - self._set_setpoints(("x",), (float(value),)) + self._set_setpoint("x", float(value)) elif instrument is self._instrument_y: - self._set_setpoints(("y",), (float(value),)) + self._set_setpoint("y", float(value)) elif instrument is self._instrument_z: - self._set_setpoints(("z",), (float(value),)) + self._set_setpoint("z", float(value)) else: msg = "This magnet doesnt belong to its specified parent {}" raise NameError(msg.format(self)) @@ -1263,44 +1392,32 @@ def _get_measured_field_vector(self) -> FieldVector: z=self._instrument_z.field(), ) - def _get_measured(self, *names: str) -> float | list[float]: - measured_field_vector = self._get_measured_field_vector() + def _get_measured_component(self, name: str) -> float: + return self._get_measured_field_vector().get_components(name)[0] - measured_values = measured_field_vector.get_components(*names) + def _get_measured[V: (CartesianVector, SphericalVector, CylindricalVector)]( + self, vector_type: type[V] + ) -> V: + # get_components returns the values in the order of the names + names = _VECTOR_COMPONENTS[vector_type] + return vector_type(*self._get_measured_field_vector().get_components(*names)) - # Convert angles from radians to degrees - d = dict(zip(names, measured_values)) - - # Do not do "return list(d.values())", because then there is - # no guaranty that the order in which the values are returned - # is the same as the original intention - value_list = [d[name] for name in names] - - if len(names) == 1: - return_value: list[float] | float = value_list[0] - else: - return_value = value_list + def _get_setpoint(self, name: str) -> float: + return self._set_point.get_components(name)[0] - return return_value + def _get_setpoints[V: (CartesianVector, SphericalVector, CylindricalVector)]( + self, vector_type: type[V] + ) -> V: + # get_components returns the values in the order of the names + names = _VECTOR_COMPONENTS[vector_type] + return vector_type(*self._set_point.get_components(*names)) - def _get_setpoints(self, names: Sequence[str]) -> float | list[float]: - measured_values = self._set_point.get_components(*names) + def _set_setpoint(self, name: str, value: float) -> None: + self._set_setpoints((name,), (value,)) - # Convert angles from radians to degrees - d = dict(zip(names, measured_values)) - value_list = [d[name] for name in names] - # Do not do "return list(d.values())", because then there is - # no guarantee that the order in which the values are returned - # is the same as the original intention - - if len(names) == 1: - return_value: list[float] | float = value_list[0] - else: - return_value = value_list - - return return_value - - def _set_setpoints(self, names: Sequence[str], values: Sequence[float]) -> None: + def _set_setpoints( + self, names: Sequence[str], values: Sequence[float] | npt.NDArray + ) -> None: kwargs = dict(zip(names, np.atleast_1d(values))) set_point = FieldVector() diff --git a/src/qcodes/math_utils/__init__.py b/src/qcodes/math_utils/__init__.py index 90fc1f213e06..818471f18249 100644 --- a/src/qcodes/math_utils/__init__.py +++ b/src/qcodes/math_utils/__init__.py @@ -3,6 +3,11 @@ such as coordinate system mappings. """ -from .field_vector import FieldVector +from .field_vector import ( + CartesianVector, + CylindricalVector, + FieldVector, + SphericalVector, +) -__all__ = ["FieldVector"] +__all__ = ["CartesianVector", "CylindricalVector", "FieldVector", "SphericalVector"] diff --git a/src/qcodes/math_utils/field_vector.py b/src/qcodes/math_utils/field_vector.py index 837756523817..847631cf8aac 100644 --- a/src/qcodes/math_utils/field_vector.py +++ b/src/qcodes/math_utils/field_vector.py @@ -5,7 +5,7 @@ from __future__ import annotations -from typing import TYPE_CHECKING, Any, ClassVar, Literal +from typing import TYPE_CHECKING, Any, ClassVar, Literal, NamedTuple import numpy as np import numpy.typing as npt @@ -20,6 +20,39 @@ NormOrder = Literal["fro", "nuc"] | None | float +class CartesianVector(NamedTuple): + """ + The components of a field vector in Cartesian coordinates. + """ + + x: float + y: float + z: float + + +class SphericalVector(NamedTuple): + """ + The components of a field vector in spherical coordinates, with the angles + in degrees. ``field`` is the magnitude of the vector, called ``r`` in + :class:`FieldVector`. + """ + + field: float + theta: float + phi: float + + +class CylindricalVector(NamedTuple): + """ + The components of a field vector in cylindrical coordinates, with the angle + in degrees. + """ + + rho: float + phi: float + z: float + + class FieldVector: """ A convenient class to keep track of vectors representing physical fields. diff --git a/tests/drivers/test_ami430_visa.py b/tests/drivers/test_ami430_visa.py index 3bb380f1458e..256ec5fef493 100644 --- a/tests/drivers/test_ami430_visa.py +++ b/tests/drivers/test_ami430_visa.py @@ -21,7 +21,12 @@ AMIModel430, AMIModel4303D, ) -from qcodes.math_utils import FieldVector +from qcodes.math_utils import ( + CartesianVector, + CylindricalVector, + FieldVector, + SphericalVector, +) from qcodes.utils.types import ( numpy_concrete_floats, numpy_concrete_ints, @@ -630,6 +635,166 @@ def test_measured( assert np.allclose(cylindrical, [cylindrical_rho, spherical_phi, cartesian_z]) +_SINGLE_COMPONENTS = ("x", "y", "z", "field", "theta", "phi", "rho") +_VECTORS = { + "cartesian": CartesianVector, + "spherical": SphericalVector, + "cylindrical": CylindricalVector, +} + + +def test_component_and_vector_types(current_driver: AMIModel4303D) -> None: + # single components are plain floats while the coordinate vectors are + # named tuples of three floats, both for the set points and the measured + # values + current_driver.cartesian((0.1, 0.2, 0.3)) + + for name in _SINGLE_COMPONENTS: + for parameter_name in (name, f"{name}_measured"): + value = current_driver.parameters[parameter_name]() + assert isinstance(value, float), parameter_name + + for name, vector_type in _VECTORS.items(): + for parameter_name in (name, f"{name}_measured"): + value = current_driver.parameters[parameter_name]() + assert type(value) is vector_type, parameter_name + assert len(value) == 3 + assert all(isinstance(component, float) for component in value) + + +def test_vector_components_by_name(current_driver: AMIModel4303D) -> None: + current_driver.cartesian((0.1, 0.2, 0.3)) + + cartesian = current_driver.cartesian_measured() + assert (cartesian.x, cartesian.y, cartesian.z) == pytest.approx((0.1, 0.2, 0.3)) + + spherical = current_driver.spherical() + # the settable vectors are typed as what they accept, so narrow the type + assert isinstance(spherical, SphericalVector) + assert spherical.field == pytest.approx(current_driver.field()) + assert spherical.theta == pytest.approx(current_driver.theta()) + assert spherical.phi == pytest.approx(current_driver.phi()) + + cylindrical = current_driver.cylindrical_measured() + assert cylindrical.rho == pytest.approx(current_driver.rho_measured()) + assert cylindrical.z == pytest.approx(0.3) + + # named tuples compare equal to plain tuples + assert current_driver.cartesian() == tuple(current_driver.cartesian()) + + +def test_set_single_component(current_driver: AMIModel4303D) -> None: + current_driver.cartesian((0.1, 0.2, 0.3)) + + current_driver.x(0.4) + current_driver.field(0.5) + + assert np.isclose(current_driver.field(), 0.5) + assert np.isclose(current_driver.field_measured(), 0.5) + # setting the field magnitude keeps the direction of (0.4, 0.2, 0.3) + direction = np.array([0.4, 0.2, 0.3]) / np.linalg.norm([0.4, 0.2, 0.3]) + assert np.allclose(current_driver.cartesian(), 0.5 * direction) + + +def _field_units(driver: AMIModel4303D) -> set[str]: + field_parameters = ( + driver.cartesian, + driver.x, + driver.y, + driver.z, + driver.field, + driver.rho, + driver.cartesian_measured, + driver.x_measured, + driver.y_measured, + driver.z_measured, + driver.field_measured, + driver.rho_measured, + ) + return {parameter.unit for parameter in field_parameters} + + +def test_field_units_follow_magnets(current_driver: AMIModel4303D) -> None: + # the field values are in the field units of the magnets, which can be + # tesla or kilogauss + assert _field_units(current_driver) == {"T"} + # the angles are not affected + assert current_driver.phi.unit == "deg" + assert current_driver.theta_measured.unit == "deg" + + +def test_cannot_change_field_units_of_magnet_in_3d_driver( + current_driver: AMIModel4303D, + magnet_axes_instances: tuple[AMIModel430, AMIModel430, AMIModel430], + mocker: MockerFixture, +) -> None: + # the set point and field limit of the 3D driver are in the field units of + # the magnets, so changing them would silently change their meaning + mag_x, _, _ = magnet_axes_instances + current_driver.cartesian((0.1, 0.2, 0.3)) + write = mocker.spy(mag_x, "write") + + with pytest.raises(RuntimeError, match="Cannot change the field units of x"): + mag_x.field_units("kilogauss") + + write.assert_not_called() + assert mag_x.field.unit == "T" + assert _field_units(current_driver) == {"T"} + assert np.allclose(current_driver.cartesian(), [0.1, 0.2, 0.3]) + + # setting the units that are already used is fine + mag_x.field_units("tesla") + + +def test_mixed_field_units_warn( + magnet_axes_instances: tuple[AMIModel430, AMIModel430, AMIModel430], + caplog: LogCaptureFixture, +) -> None: + mag_x, _, _ = magnet_axes_instances + mag_x.field_units("kilogauss") + + with caplog.at_level(logging.WARNING): + driver = AMIModel4303D("AMI430_3D", *magnet_axes_instances, field_limit) + try: + assert _field_units(driver) == {""} + assert ( + "The magnets use different field units (x: kG, y: T, z: T)" in caplog.text + ) + finally: + driver.close() + + +def test_field_units_at_init_follow_magnets( + magnet_axes_instances: tuple[AMIModel430, AMIModel430, AMIModel430], +) -> None: + for magnet in magnet_axes_instances: + magnet.field_units("kilogauss") + + driver = AMIModel4303D("AMI430_3D", *magnet_axes_instances, field_limit) + try: + assert _field_units(driver) == {"kG"} + finally: + driver.close() + + +def test_magnets_work_after_3d_driver_is_closed( + magnet_axes_instances: tuple[AMIModel430, AMIModel430, AMIModel430], +) -> None: + mag_x, _, _ = magnet_axes_instances + driver = AMIModel4303D("AMI430_3D", *magnet_axes_instances, field_limit) + assert mag_x._parent_instrument is driver + + driver.close() + + # the magnets no longer delegate to the closed 3D driver + assert mag_x._parent_instrument is None + mag_x.field_units("kilogauss") + assert mag_x.field.unit == "kG" + mag_x.set_field(0.1) + assert mag_x.field() == pytest.approx(0.1) + mag_x.zero() + + def get_ramp_down_order(messages: list[str]) -> list[str]: order = [] @@ -796,7 +961,8 @@ def test_simultaneous_ramp_mode_does_not_reset_individual_axis_ramp_rates_if_non ami3d._instrument_y.ramp_rate(0.10) ami3d._instrument_z.ramp_rate(0.11) - ami3d.vector_ramp_rate(0.05) + vector_ramp_rate = 0.05 + ami3d.vector_ramp_rate(vector_ramp_rate) with caplog.at_level(logging.DEBUG, logger=LOG_NAME): # Initiate the simultaneous ramp @@ -812,7 +978,7 @@ def test_simultaneous_ramp_mode_does_not_reset_individual_axis_ramp_rates_if_non # set by the simultaneous ramp based on the vector_ramp_rate and the # setpoint magnetic field expected_ramp_rate = pytest.approx( - 0.5 / np.linalg.norm(ami3d.cartesian(), ord=2) * ami3d.vector_ramp_rate() + 0.5 / np.linalg.norm(ami3d.cartesian(), ord=2) * vector_ramp_rate ) assert ami3d._instrument_x.ramp_rate() == expected_ramp_rate assert ami3d._instrument_y.ramp_rate() == expected_ramp_rate @@ -1909,13 +2075,20 @@ def test_calculate_vector_ramp_rate_from_duration() -> None: assert np.isclose(rate, 0.5) # distance=5.0, 5.0/10.0 = 0.5 -def test_raise_if_not_same_field_units(current_driver: AMIModel4303D) -> None: +def test_raise_if_not_same_field_units( + magnet_axes_instances: tuple[AMIModel430, AMIModel430, AMIModel430], +) -> None: """Test that mismatched field_units raises ValueError.""" - current_driver._instrument_x.field_units("kilogauss") - with pytest.raises(ValueError, match="field_units"): - current_driver._raise_if_not_same_field_and_ramp_rate_units() - # Clean up - current_driver._instrument_x.field_units("tesla") + # the field units cannot change while the magnets are part of a 3D + # driver, so set up the mismatch first + mag_x, _, _ = magnet_axes_instances + mag_x.field_units("kilogauss") + driver = AMIModel4303D("AMI430_3D", *magnet_axes_instances, field_limit) + try: + with pytest.raises(ValueError, match="field_units"): + driver._raise_if_not_same_field_and_ramp_rate_units() + finally: + driver.close() def test_raise_if_not_same_ramp_rate_units(current_driver: AMIModel4303D) -> None: diff --git a/tests/test_field_vector.py b/tests/test_field_vector.py index 16cb6f2b3765..2362078a2bcb 100644 --- a/tests/test_field_vector.py +++ b/tests/test_field_vector.py @@ -10,6 +10,11 @@ from hypothesis import given, settings from hypothesis.strategies import floats, tuples +from qcodes.math_utils import ( + CartesianVector, + CylindricalVector, + SphericalVector, +) from qcodes.math_utils.field_vector import FieldVector from qcodes.utils import NumpyJSONEncoder @@ -161,3 +166,21 @@ def test_all_attributes_are_floats() -> None: for attr in FieldVector.attributes: assert isinstance(getattr(fv, attr), float) + + +def test_coordinate_named_tuples() -> None: + vector = FieldVector(x=0.1, y=0.2, z=0.3) + + cartesian = CartesianVector(*vector.get_components("x", "y", "z")) + spherical = SphericalVector(*vector.get_components("r", "theta", "phi")) + cylindrical = CylindricalVector(*vector.get_components("rho", "phi", "z")) + + assert cartesian.x == 0.1 + assert spherical.field == vector.r + assert cylindrical.rho == vector.rho + # they are tuples, so they compare equal to and unpack like plain tuples + assert cartesian == (0.1, 0.2, 0.3) + x, y, z = cartesian + assert (x, y, z) == (0.1, 0.2, 0.3) + # and they are stored as plain lists in JSON, like other tuples + assert json.loads(json.dumps(cartesian, cls=NumpyJSONEncoder)) == [0.1, 0.2, 0.3]