diff --git a/pyaml/bpm/bpm.py b/pyaml/bpm/bpm.py index 1c68560d..acc35e75 100644 --- a/pyaml/bpm/bpm.py +++ b/pyaml/bpm/bpm.py @@ -29,8 +29,6 @@ def __init__(self, cfg: ConfigModel): ---------- name : str Element name - hardware : DeviceAccess - Direct access to a hardware (bypass the BPM model) model : BPMModel BPM model in charge of computing beam position """ diff --git a/pyaml/bpm/bpm_model.py b/pyaml/bpm/bpm_model.py index 33e35962..19fd21d3 100644 --- a/pyaml/bpm/bpm_model.py +++ b/pyaml/bpm/bpm_model.py @@ -20,7 +20,7 @@ def get_pos_devices(self) -> list[DeviceAccess | None]: Returns ------- list[DeviceAccess] - Array of DeviceAcess + h and v position devices """ pass @@ -32,7 +32,7 @@ def get_tilt_device(self) -> DeviceAccess | None: Returns ------- list[DeviceAccess] - Array of DeviceAcess + tilt device """ pass @@ -44,6 +44,80 @@ def get_offset_devices(self) -> list[DeviceAccess | None]: Returns ------- list[DeviceAccess] - Array of DeviceAcess + h and v offset devices """ pass + + def x_pos_index(self) -> int | None: + """ + Returns the index of the horizontal position in + an array, otherwise a scalar value is expected from the + corresponding DeviceAccess + + Returns + ------- + int + Index in the array, None for a scalar value + """ + return None + + def y_pos_index(self) -> int | None: + """ + Returns the index of the veritcal position in + an array, otherwise a scalar value is expected from the + corresponding DeviceAccess + + Returns + ------- + int + Index in the array, None for a scalar value + """ + return None + + def is_pos_indexed(self) -> bool: + return self.x_pos_index() is not None and self.y_pos_index() is not None + + def tilt_index(self) -> int | None: + """ + Returns the index of the tilt angle in + an array, otherwise a scalar value is expected from the + corresponding DeviceAccess + + Returns + ------- + int + Index in the array, None for a scalar value + """ + return None + + def is_tilt_indexed(self) -> bool: + return self.tilt_index() is not None + + def x_offset_index(self) -> int | None: + """ + Returns the index of the horizontal offset in + an array, otherwise a scalar value is expected from the + corresponding DeviceAccess + + Returns + ------- + int + Index in the array, None for a scalar value + """ + return None + + def y_offset_index(self) -> int | None: + """ + Returns the index of the veritcal offset in + an array, otherwise a scalar value is expected from the + corresponding DeviceAccess + + Returns + ------- + int + Index in the array, None for a scalar value + """ + return None + + def is_offset_indexed(self) -> bool: + return self.x_offset_index() is not None and self.y_offset_index() is not None diff --git a/pyaml/bpm/bpm_simple_model.py b/pyaml/bpm/bpm_simple_model.py index dd8ae74f..582a6025 100644 --- a/pyaml/bpm/bpm_simple_model.py +++ b/pyaml/bpm/bpm_simple_model.py @@ -21,12 +21,20 @@ class ConfigModel(BaseModel): Horizontal position device y_pos : DeviceAccess, optional Vertical position device + x_pos_index : int, optional + Index in the array when specified, otherwise scalar + value is expected + y_pos_index : int, optional + Index in the array when specified, otherwise scalar + value is expected """ model_config = ConfigDict(arbitrary_types_allowed=True, extra="forbid") x_pos: DeviceAccess | None y_pos: DeviceAccess | None + x_pos_index: int | None = None + y_pos_index: int | None = None class BPMSimpleModel(BPMModel): @@ -73,5 +81,31 @@ def get_offset_devices(self) -> list[DeviceAccess | None]: """ return [None, None] + def x_pos_index(self) -> int | None: + """ + Returns the index of the horizontal position in + an array, otherwise a scalar value is expected from the + corresponding DeviceAccess + + Returns + ------- + int + Index in the array, None for a scalar value + """ + return self._cfg.x_pos_index + + def y_pos_index(self) -> int | None: + """ + Returns the index of the veritcal position in + an array, otherwise a scalar value is expected from the + corresponding DeviceAccess + + Returns + ------- + int + Index in the array, None for a scalar value + """ + return self._cfg.y_pos_index + def __repr__(self): return __pyaml_repr__(self) diff --git a/pyaml/bpm/bpm_tiltoffset_model.py b/pyaml/bpm/bpm_tiltoffset_model.py index 91079e19..6bccbd8c 100644 --- a/pyaml/bpm/bpm_tiltoffset_model.py +++ b/pyaml/bpm/bpm_tiltoffset_model.py @@ -11,6 +11,8 @@ # Define the main class name for this module PYAMLCLASS = "BPMTiltOffsetModel" +# TODO: Implepement indexed offset and tilt + class ConfigModel(BaseModel): """ @@ -34,6 +36,8 @@ class ConfigModel(BaseModel): x_pos: DeviceAccess | None y_pos: DeviceAccess | None + x_pos_index: int | None = None + y_pos_index: int | None = None x_offset: DeviceAccess | None y_offset: DeviceAccess | None tilt: DeviceAccess | None diff --git a/pyaml/control/abstract_impl.py b/pyaml/control/abstract_impl.py index 783e534b..87831ebc 100644 --- a/pyaml/control/abstract_impl.py +++ b/pyaml/control/abstract_impl.py @@ -138,6 +138,36 @@ def unit(self) -> str: # ------------------------------------------------------------------------------ +class CSBPMArrayMapper(CSScalarAggregator): + """ + Wrapper to a native CS aggregator for BPM + """ + + def __init__(self, devs: list[DeviceAccess], indices: list[list[int]]): + self._indices = indices + self._devs = devs + + def set(self, value: NDArray[np.float64]): + raise Exception("BPM are not writable") + + def get(self) -> NDArray[np.float64]: + # TODO read using DeviceAccessList + allValues = [] + for i, d in enumerate(self._devs): + v = d.get() + allValues.extend(v[self._indices[i]]) + return np.array(allValues) + + def readback(self) -> np.array: + return self.get() + + def unit(self) -> str: + return self._dev.unit() + + +# ------------------------------------------------------------------------------ + + class RWHardwareScalar(abstract.ReadWriteFloatScalar): """ Class providing read write access to a magnet @@ -267,20 +297,35 @@ def unit(self) -> list[str]: class RBpmArray(abstract.ReadFloatArray): """ - Class providing read access to a BPM array of a control system + Class providing read access to a BPM position [x,y] of a control system """ - def __init__(self, model: BPMModel, devs: list[DeviceAccess]): - self.__model = model - self.__devs = devs + def __init__(self, model: BPMModel, hDev: DeviceAccess, vDev: DeviceAccess): + self._model = model + self._hDev = hDev + self._vDev = vDev + self._hIdx = self._model.x_pos_index() + self._vIdx = self._model.y_pos_index() # Gets the values def get(self) -> np.array: - return np.array([self.__devs[0].get(), self.__devs[1].get()]) - - # Gets the unit of the value Assume that x and y has the same unit + if self._hDev != self._vDev: + allhVal = self._hDev.get() + allvVal = self._vDev.get() + hVal = allhVal if self._hIdx is None else allhVal[self._hIdx] + vVal = allvVal if self._vIdx is None else allvVal[self._vIdx] + else: + # When h and v devices are identical, indexed + # values are expected + allVal = self._hDev.get() + hVal = allVal[self._hIdx] + vVal = allVal[self._vIdx] + return np.array([hVal, vVal]) + + # Gets the unit of the value Assume that x and y, offsets and positions + # have the same unit def unit(self) -> str: - return self.__model.get_pos_devices()[0].unit() + return self._model.get_pos_devices()[0].unit() # ------------------------------------------------------------------------------ @@ -292,22 +337,27 @@ class RWBpmTiltScalar(abstract.ReadFloatScalar): """ def __init__(self, model: BPMModel, dev: DeviceAccess): - self.__model = model - self.__dev = dev + self._model = model + self._dev = dev + self._idx = model.tilt_index() # Gets the value def get(self) -> float: - return self.__dev.get() + allTilt = self._dev.get() + if self._idx is not None: + return allTilt[self._idx] + else: + return allTilt def set(self, value: float): - self.__dev.set(value) + self._dev.set(value) def set_and_wait(self, value: NDArray[np.float64]): raise NotImplementedError("Not implemented yet.") # Gets the unit of the value def unit(self) -> str: - return self.__model.get_tilt_device().unit() + return self._model.get_tilt_device().unit() # ------------------------------------------------------------------------------ @@ -315,28 +365,51 @@ def unit(self) -> str: class RWBpmOffsetArray(abstract.ReadWriteFloatArray): """ - Class providing read write access to a BPM offset of a control system + Class providing read write access to a BPM offset [x,y] of a control system """ - def __init__(self, model: BPMModel, devs: list[DeviceAccess]): - self.__model = model - self.__devs = devs - - # Gets the value - def get(self) -> NDArray[np.float64]: - return np.array([self.__devs[0].get(), self.__devs[1].get()]) + def __init__(self, model: BPMModel, hDev: DeviceAccess, vDev: DeviceAccess): + self._model = model + self._hDev = hDev + self._vDev = vDev + self._hIdx = self._model.x_pos_index() + self._vIdx = self._model.y_pos_index() - # Sets the value + # Gets the values + def get(self) -> np.array: + if self._hDev != self._vDev: + allhVal = self._hDev.get() + allvVal = self._vDev.get() + hVal = allhVal if self._hIdx is None else allhVal[self._hIdx] + vVal = allvVal if self._vIdx is None else allvVal[self._vIdx] + else: + # When h and v devices are identical, indexed + # values are expected + allVal = self._hDev.get() + hVal = allVal[self._hIdx] + vVal = allVal[self._vIdx] + return np.array([hVal, vVal]) + + # Sets the values def set(self, value: NDArray[np.float64]): - self.__devs[0].set(value[0]) - self.__devs[1].set(value[1]) + if self._hDev != self._vDev: + self._hDev.set(value[0]) + self._vDev.set(value[1]) + else: + # When h and v devices are identical, indexed + # values are expected + newValue = self._hDev.get() + newValue[self._hIdx] = value[0] + newValue[self._vIdx] = value[1] + self._hDev.set(newValue) def set_and_wait(self, value: NDArray[np.float64]): raise NotImplementedError("Not implemented yet.") - # Gets the unit of the value + # Gets the unit of the value Assume that x and y, offsets and positions + # have the same unit def unit(self) -> str: - return self.__model.get_offset_devices()[0].unit() + return self._model.get_pos_devices()[0].unit() # ------------------------------------------------------------------------------ diff --git a/pyaml/control/controlsystem.py b/pyaml/control/controlsystem.py index 44d465a0..2c3f296c 100644 --- a/pyaml/control/controlsystem.py +++ b/pyaml/control/controlsystem.py @@ -1,12 +1,16 @@ from abc import ABCMeta, abstractmethod +from typing import Tuple from ..bpm.bpm import BPM +from ..bpm.bpm_model import BPMModel from ..common.abstract import RWMapper from ..common.abstract_aggregator import ScalarAggregator from ..common.element import Element from ..common.element_holder import ElementHolder +from ..common.exception import PyAMLException from ..configuration.factory import Factory from ..control.abstract_impl import ( + CSBPMArrayMapper, CSScalarAggregator, CSStrengthScalarAggregator, RBetatronTuneArray, @@ -46,6 +50,12 @@ def attach(self, dev: list[DeviceAccess]) -> list[DeviceAccess]: coming from configuration attached to this CS""" pass + @abstractmethod + def attach_array(self, dev: list[DeviceAccess]) -> list[DeviceAccess]: + """Return new instances of DeviceAccess objects + coming from configuration attached to this CS""" + pass + @abstractmethod def name(self) -> str: """Return control system name (i.e. live)""" @@ -61,6 +71,12 @@ def vector_aggregator(self) -> str | None: """Returns the module name used for handling aggregator of DeviceVectorAccess""" return None + def attach_indexed(self, dev: DeviceAccess, idx: int | None) -> DeviceAccess: + if idx is not None: + return self.attach_array([dev])[0] + else: + return self.attach([dev])[0] + def create_scalar_aggregator(self) -> ScalarAggregator: mod = self.scalar_aggregator() agg = Factory.build_object({"type": mod}) if mod is not None else None @@ -90,15 +106,70 @@ def create_magnet_hardware_aggregator( return agg def create_bpm_aggregators(self, bpms: list[BPM]) -> list[ScalarAggregator]: - agg = self.create_scalar_aggregator() - aggh = self.create_scalar_aggregator() - aggv = self.create_scalar_aggregator() - for b in bpms: - devs = self.attach(b.model.get_pos_devices()) - agg.add_devices(devs) - aggh.add_devices(devs[0]) - aggv.add_devices(devs[1]) - return [agg, aggh, aggv] + # return [None,None,None] + + if any([not b.model.is_pos_indexed() for b in bpms]): + # Aggregator for single BPM (all values are scalar) + agg = self.create_scalar_aggregator() + aggh = self.create_scalar_aggregator() + aggv = self.create_scalar_aggregator() + for b in bpms: + devs = self.attach(b.model.get_pos_devices()) + agg.add_devices(devs) + aggh.add_devices(devs[0]) + aggv.add_devices(devs[1]) + return [agg, aggh, aggv] + + elif any([b.model.is_pos_indexed() for b in bpms]): + # Aggregator for indexed BPMs + allH = [] + hIdx = [] + allV = [] + vIdx = [] + allHV = [] + for b in bpms: + devs = self.attach_array(b.model.get_pos_devices()) + devH = devs[0] + devV = devs[1] + if devH not in allH: + allH.append(devH) + if devH not in allHV: + allHV.append(devH) + if devV not in allV: + allV.append(devV) + if devV not in allHV: + allHV.append(devV) + hIdx.append(b.model.x_pos_index()) + vIdx.append(b.model.y_pos_index()) + + if len(allH) > 1 or len(allV) > 1: + # Does not support aggregator for individual BPM that + # returns an array of [x,y] + print( + "Warning, Individual BPM that returns [x,y]" + + " are not read in parralell" + ) + # Default to serialized readding + return [None, None, None] + + if devH == devV: + # [x0,y0,x1,y0,....] + idx = [] + for b in bpms: + idx.append(b.model.x_pos_index()) + idx.append(b.model.y_pos_index()) + hvIdx = [idx] + else: + hvIdx = [hIdx, vIdx] + + agg = CSBPMArrayMapper(allHV, hvIdx) + aggh = CSBPMArrayMapper(allH, [hIdx]) + aggv = CSBPMArrayMapper(allV, [vIdx]) + return [agg, aggh, aggv] + else: + raise PyAMLException( + "Indexed BPM and scalar values cannot be mixed in the same array" + ) def set_energy(self, E: float): """ @@ -152,7 +223,8 @@ def fill_device(self, elements: list[Element]): devs = self.attach(e.model.get_devices()) currents = [] strengths = [] - # Create unique refs the series and each of its function for this control system + # Create unique refs the series and each of its function for this + # control system for i in range(e.get_nb_magnets()): current = ( RWHardwareScalar(e.model.get_sub_model(i), devs[i]) @@ -172,12 +244,19 @@ def fill_device(self, elements: list[Element]): self.add_magnet(m) elif isinstance(e, BPM): - tiltDev = self.attach([e.model.get_tilt_device()])[0] - offsetsDevs = self.attach(e.model.get_offset_devices()) - posDevs = self.attach(e.model.get_pos_devices()) - tilt = RWBpmTiltScalar(e.model, tiltDev) - offsets = RWBpmOffsetArray(e.model, offsetsDevs) - positions = RBpmArray(e.model, posDevs) + hDev = e.model.get_pos_devices()[0] + vDev = e.model.get_pos_devices()[1] + tiltDev = e.model.get_tilt_device() + hOffsetDev = e.model.get_offset_devices()[0] + vOffsetDev = e.model.get_offset_devices()[1] + ahDev = self.attach_indexed(hDev, e.model.x_pos_index()) + avDev = self.attach_indexed(vDev, e.model.y_pos_index()) + atiltDev = self.attach_indexed(tiltDev, e.model.tilt_index()) + ahOffsetDev = self.attach_indexed(hOffsetDev, e.model.x_offset_index()) + avOffsetDev = self.attach_indexed(vOffsetDev, e.model.y_offset_index()) + positions = RBpmArray(e.model, ahDev, avDev) + tilt = RWBpmTiltScalar(e.model, atiltDev) + offsets = RWBpmOffsetArray(e.model, ahOffsetDev, avOffsetDev) e = e.attach(self, positions, offsets, tilt) self.add_bpm(e) diff --git a/pyaml/control/deviceaccess.py b/pyaml/control/deviceaccess.py index 41218c68..e6dd0cb6 100644 --- a/pyaml/control/deviceaccess.py +++ b/pyaml/control/deviceaccess.py @@ -1,14 +1,11 @@ from abc import ABCMeta, abstractmethod -from typing import Union -import numpy.typing as npt - -from .readback_value import Value +# TODO: correctly type value class DeviceAccess(metaclass=ABCMeta): """ - Abstract class providing access to a control system float variable + Abstract class providing access to a control system variable """ @abstractmethod @@ -22,22 +19,22 @@ def measure_name(self) -> str: pass @abstractmethod - def set(self, value: float): + def set(self, value): """Write a control system device variable (i.e. a power supply current)""" pass @abstractmethod - def set_and_wait(self, value: float): + def set_and_wait(self, value): """Write a control system device variable (i.e. a power supply current)""" pass @abstractmethod - def get(self) -> float: - """Return the setpoint of a control system device variable""" + def get(self): + """Return the setpoint(s) of a control system device variable""" pass @abstractmethod - def readback(self) -> Union[Value, npt.NDArray[Value]]: + def readback(self): """Return the measured variable""" pass diff --git a/tests/config/bpms.yaml b/tests/config/bpms.yaml index 14ec1a84..e0efafec 100644 --- a/tests/config/bpms.yaml +++ b/tests/config/bpms.yaml @@ -60,6 +60,20 @@ devices: type: tango.pyaml.attribute_read_only attribute: srdiag/bpm/c01-03/SA_VPosition unit: mm +- type: pyaml.bpm.bpm + name: BPM_C01-04 + model: + type: pyaml.bpm.bpm_simple_model + x_pos_index: 0 + y_pos_index: 1 + x_pos: + type: tango.pyaml.attribute_read_only + attribute: srdiag/bpm/c01-04/Position + unit: mm + y_pos: + type: tango.pyaml.attribute_read_only + attribute: srdiag/bpm/c01-04/Position + unit: mm - type: pyaml.magnet.cfm_magnet name: SH1A-C01 #Name of the element in the lattice model mapping: diff --git a/tests/dummy_cs/tango-pyaml/tango/pyaml/attribute.py b/tests/dummy_cs/tango-pyaml/tango/pyaml/attribute.py index 23447c77..f5108a5d 100644 --- a/tests/dummy_cs/tango-pyaml/tango/pyaml/attribute.py +++ b/tests/dummy_cs/tango-pyaml/tango/pyaml/attribute.py @@ -19,13 +19,16 @@ class Attribute(DeviceAccess): values (Debugging purpose) """ - def __init__(self, cfg: ConfigModel): + def __init__(self, cfg: ConfigModel, is_array=False): super().__init__() self._cfg = cfg self._setpoint = cfg.attribute self._readback = cfg.attribute self._unit = cfg.unit - self._cache = 0.0 + + def set_array(self, is_array: bool): + self._is_array = is_array + self._cache = 0.0 if not is_array else [0.0, 1.0] def name(self) -> str: return self._setpoint @@ -33,18 +36,21 @@ def name(self) -> str: def measure_name(self) -> str: return self._readback - def set(self, value: float): + def set(self, value): print(f"{self._cfg.attribute}:{value}") self._cache = value - def set_and_wait(self, value: float): + def set_and_wait(self, value): self.set(value) - def get(self) -> float: + def get(self): return self._cache - def readback(self) -> Value: - return Value(self._cache) + def readback(self): + if self._is_array: + return [Value(v) for v in self._cache] + else: + return Value(self._cache) def unit(self) -> str: return self._unit diff --git a/tests/dummy_cs/tango-pyaml/tango/pyaml/attribute_read_only.py b/tests/dummy_cs/tango-pyaml/tango/pyaml/attribute_read_only.py index 7be8de4e..5dca8688 100644 --- a/tests/dummy_cs/tango-pyaml/tango/pyaml/attribute_read_only.py +++ b/tests/dummy_cs/tango-pyaml/tango/pyaml/attribute_read_only.py @@ -22,29 +22,12 @@ class AttributeReadOnly(Attribute): def __init__(self, cfg: ConfigModel): super().__init__(cfg) - self._cfg = cfg - self._setpoint = cfg.attribute - self._readback = cfg.attribute - self._unit = cfg.unit - self._cache = 0.0 - def name(self) -> str: - return self._setpoint - - def measure_name(self) -> str: - return self._readback - - def set(self, value: float): + def set(self, value): raise Exception(f"{self._cfg.attribute} is read only attribute") - def set_and_wait(self, value: float): + def set_and_wait(self, value): raise Exception(f"{self._cfg.attribute} is read only attribute") - def get(self) -> float: - return self._cache - - def readback(self) -> Value: - return Value(self._cache) - def unit(self) -> str: return self._unit diff --git a/tests/dummy_cs/tango-pyaml/tango/pyaml/controlsystem.py b/tests/dummy_cs/tango-pyaml/tango/pyaml/controlsystem.py index d1d62202..68fee777 100644 --- a/tests/dummy_cs/tango-pyaml/tango/pyaml/controlsystem.py +++ b/tests/dummy_cs/tango-pyaml/tango/pyaml/controlsystem.py @@ -24,7 +24,13 @@ def __init__(self, cfg: ConfigModel): print(f"Creating dummy TangoControlSystem: {cfg.name}") self.__DEVICES = {} + def attach_array(self, devs: list[DeviceAccess]) -> list[DeviceAccess]: + return self._attach(devs, True) + def attach(self, devs: list[DeviceAccess]) -> list[DeviceAccess]: + return self._attach(devs, False) + + def _attach(self, devs: list[DeviceAccess], is_array: bool) -> list[DeviceAccess]: newDevs = [] for d in devs: if d is not None: @@ -36,6 +42,7 @@ def attach(self, devs: list[DeviceAccess]) -> list[DeviceAccess]: # to allow a new attribute name newDev._cfg = copy.copy(d._cfg) newDev._cfg.attribute = full_name + newDev.set_array(is_array) self.__DEVICES[full_name] = newDev newDevs.append(self.__DEVICES[full_name]) else: diff --git a/tests/test_bpm_controlsystem.py b/tests/test_bpm_controlsystem.py index dcc7764f..3e5ed269 100644 --- a/tests/test_bpm_controlsystem.py +++ b/tests/test_bpm_controlsystem.py @@ -54,4 +54,16 @@ def test_controlsystem_bpm_position(install_test_package): assert np.allclose(bpm.positions.get(), np.array([0.0, 0.0])) assert np.allclose(bpm_simple.positions.get(), np.array([0.0, 0.0])) + +@pytest.mark.parametrize( + "install_test_package", + [{"name": "tango-pyaml", "path": "tests/dummy_cs/tango-pyaml"}], + indirect=True, +) +def test_controlsystem_bpm_position_indexed(install_test_package): + sr: Accelerator = Accelerator.load("tests/config/bpms.yaml") + bpm = sr.live.get_bpm("BPM_C01-04") + + assert np.allclose(bpm.positions.get(), np.array([0.0, 1.0])) + Factory.clear()