From 0e43749ee80cd2bbf0f06395648aefbdbb14a130 Mon Sep 17 00:00:00 2001 From: PONS Date: Mon, 12 Jan 2026 19:19:18 +0100 Subject: [PATCH 1/7] Support for native BPM CS aggregator --- pyaml/bpm/bpm.py | 9 ++-- pyaml/bpm/bpm_array_model.py | 61 ++++++++++++++++++++++++ pyaml/control/abstract_impl.py | 53 ++++++++++++++++++++- pyaml/control/controlsystem.py | 84 ++++++++++++++++++++++++++-------- pyaml/control/deviceaccess.py | 17 +++---- 5 files changed, 189 insertions(+), 35 deletions(-) create mode 100644 pyaml/bpm/bpm_array_model.py diff --git a/pyaml/bpm/bpm.py b/pyaml/bpm/bpm.py index 1c68560d..f290d345 100644 --- a/pyaml/bpm/bpm.py +++ b/pyaml/bpm/bpm.py @@ -1,3 +1,4 @@ +from ..bpm.bpm_array_model import BPMArrayModel from ..bpm.bpm_model import BPMModel from ..common.element import Element, ElementConfigModel from ..common.exception import PyAMLException @@ -12,7 +13,7 @@ class ConfigModel(ElementConfigModel): - model: BPMModel | None = None + model: BPMModel | BPMArrayModel | None = None """Object in charge of BPM modeling""" @@ -29,10 +30,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 """ super().__init__(cfg.name) @@ -44,7 +41,7 @@ def __init__(self, cfg: ConfigModel): self.__tilt = None @property - def model(self) -> BPMModel: + def model(self) -> BPMModel | BPMArrayModel: return self.__model @property diff --git a/pyaml/bpm/bpm_array_model.py b/pyaml/bpm/bpm_array_model.py new file mode 100644 index 00000000..f55cee33 --- /dev/null +++ b/pyaml/bpm/bpm_array_model.py @@ -0,0 +1,61 @@ +import numpy as np +from numpy.typing import NDArray +from pydantic import BaseModel, ConfigDict + +from ..common.element import __pyaml_repr__ +from ..control.deviceaccess import DeviceAccess + +# Define the main class name for this module +PYAMLCLASS = "BPMArrayModel" + +# TODO: Handle tilt and offset + + +class ConfigModel(BaseModel): + """ + Configuration model for BPM array + + Parameters + ---------- + pos : DeviceAccess, optional + Orbit device name + h_index : int, optional + Index in the array + v_index : int, optional + Index in the array + """ + + model_config = ConfigDict(arbitrary_types_allowed=True, extra="forbid") + + pos: DeviceAccess | None + h_index: int = 0 + v_index: int = 0 + + +class BPMArrayModel: + """ + Class that map an orbit device to individual BPM + """ + + def __init__(self, cfg: ConfigModel): + self._cfg = cfg + + def get_pos_device(self) -> DeviceAccess | None: + """ + Get device handle used for position reading + + Returns + ------- + DeviceAccess + Orbit DeviceAcess + """ + return self._cfg.pos + + def get_h_index(self) -> int: + return self._cfg.h_index + + def get_v_index(self) -> int: + return self._cfg.v_index + + def __repr__(self): + return __pyaml_repr__(self) diff --git a/pyaml/control/abstract_impl.py b/pyaml/control/abstract_impl.py index 783e534b..c117b6bb 100644 --- a/pyaml/control/abstract_impl.py +++ b/pyaml/control/abstract_impl.py @@ -2,6 +2,7 @@ from numpy import double from numpy.typing import NDArray +from ..bpm.bpm_array_model import BPMArrayModel from ..bpm.bpm_model import BPMModel from ..common import abstract from ..common.abstract_aggregator import ScalarAggregator @@ -138,6 +139,32 @@ def unit(self) -> str: # ------------------------------------------------------------------------------ +class CSBPMArrayMapper(CSScalarAggregator): + """ + Wrapper to a native CS aggregator for BPM + """ + + def __init__(self, dev: DeviceAccess, _indices: list[int]): + self._indices = _indices + self._dev = dev + + def set(self, value: NDArray[np.float64]): + raise Exception("BPM are not writable") + + def get(self) -> NDArray[np.float64]: + allValues: np.array = self._dev.get() + return allValues[self._indices] + + 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,7 +294,7 @@ 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]): @@ -286,6 +313,30 @@ def unit(self) -> str: # ------------------------------------------------------------------------------ +class RBpmsArray(abstract.ReadFloatArray): + """ + Class providing read access to a BPM positions array of a control system + """ + + def __init__(self, model: BPMArrayModel, dev: DeviceAccess): + self._model = model + self._h_idx = model.get_h_index() + self._v_idx = model.get_v_index() + self.__dev = dev + + # Gets the values + def get(self) -> np.array: + allPos = self.__dev.get() + return np.array([allPos[self._h_idx], allPos[self._v_idx]]) + + # Gets the unit of the value Assume that x and y has the same unit + def unit(self) -> str: + return self.__model.get_pos_device().unit() + + +# ------------------------------------------------------------------------------ + + class RWBpmTiltScalar(abstract.ReadFloatScalar): """ Class providing read access to a BPM tilt of a control system diff --git a/pyaml/control/controlsystem.py b/pyaml/control/controlsystem.py index 44d465a0..92518f46 100644 --- a/pyaml/control/controlsystem.py +++ b/pyaml/control/controlsystem.py @@ -1,16 +1,21 @@ from abc import ABCMeta, abstractmethod from ..bpm.bpm import BPM +from ..bpm.bpm_array_model import BPMArrayModel +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, RBpmArray, + RBpmsArray, RWBpmOffsetArray, RWBpmTiltScalar, RWHardwareArray, @@ -46,6 +51,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)""" @@ -90,15 +101,44 @@ 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] + if len(bpms) == 0: + return None + + if any([isinstance(b.model, BPMModel) for b in bpms]): + # simple bpm aggregator + 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([isinstance(b.model, BPMArrayModel) for b in bpms]): + # Mapper to a native CS aggregator + dev = self.attach_array([bpms[0].model.get_pos_device()])[0] + if any( + [self.attach_array([b.model.get_pos_device()])[0] != dev for b in bpms] + ): + raise PyAMLException("BPMArrayModel must share the same CS device") + # CS BPM aggregator + hv_idx = [] + h_idx = [] + v_idx = [] + for b in bpms: + hv_idx.append(b.model.get_h_index()) + hv_idx.append(b.model.get_v_index()) + h_idx.append(b.model.get_h_index()) + v_idx.append(b.model.get_v_index()) + agg = CSBPMArrayMapper(dev, hv_idx) + aggh = CSBPMArrayMapper(dev, h_idx) + aggv = CSBPMArrayMapper(dev, v_idx) + return [agg, aggh, aggv] + else: + raise PyAMLException( + "BPMArrayModel and BPMModel cannot be mixed in the same array" + ) def set_energy(self, E: float): """ @@ -152,7 +192,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,14 +213,21 @@ 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) - e = e.attach(self, positions, offsets, tilt) - self.add_bpm(e) + if isinstance(e.model, BPMArrayModel): + # TODO: Handle tilt and offset + posDev = self.attach_array([e.model.get_pos_device()])[0] + positions = RBpmsArray(e.model, posDev) + e = e.attach(self, positions, None, None) + self.add_bpm(e) + else: + 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) + e = e.attach(self, positions, offsets, tilt) + self.add_bpm(e) elif isinstance(e, RFPlant): attachedTrans: list[RFTransmitter] = [] 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 From 18045df95c1f7afd431649ea23a142a9461e3979 Mon Sep 17 00:00:00 2001 From: PONS Date: Wed, 14 Jan 2026 15:24:31 +0100 Subject: [PATCH 2/7] Updated BPM aggregator logic --- pyaml/bpm/bpm.py | 5 +- pyaml/bpm/bpm_array_model.py | 61 ------------------ pyaml/bpm/bpm_model.py | 80 ++++++++++++++++++++++- pyaml/bpm/bpm_simple_model.py | 34 ++++++++++ pyaml/control/abstract_impl.py | 112 ++++++++++++--------------------- pyaml/control/controlsystem.py | 112 +++++++++++++++++++++------------ 6 files changed, 225 insertions(+), 179 deletions(-) delete mode 100644 pyaml/bpm/bpm_array_model.py diff --git a/pyaml/bpm/bpm.py b/pyaml/bpm/bpm.py index f290d345..fb8e4cc9 100644 --- a/pyaml/bpm/bpm.py +++ b/pyaml/bpm/bpm.py @@ -1,4 +1,3 @@ -from ..bpm.bpm_array_model import BPMArrayModel from ..bpm.bpm_model import BPMModel from ..common.element import Element, ElementConfigModel from ..common.exception import PyAMLException @@ -13,7 +12,7 @@ class ConfigModel(ElementConfigModel): - model: BPMModel | BPMArrayModel | None = None + model: BPMModel | None = None """Object in charge of BPM modeling""" @@ -41,7 +40,7 @@ def __init__(self, cfg: ConfigModel): self.__tilt = None @property - def model(self) -> BPMModel | BPMArrayModel: + def model(self) -> BPMModel: return self.__model @property diff --git a/pyaml/bpm/bpm_array_model.py b/pyaml/bpm/bpm_array_model.py deleted file mode 100644 index f55cee33..00000000 --- a/pyaml/bpm/bpm_array_model.py +++ /dev/null @@ -1,61 +0,0 @@ -import numpy as np -from numpy.typing import NDArray -from pydantic import BaseModel, ConfigDict - -from ..common.element import __pyaml_repr__ -from ..control.deviceaccess import DeviceAccess - -# Define the main class name for this module -PYAMLCLASS = "BPMArrayModel" - -# TODO: Handle tilt and offset - - -class ConfigModel(BaseModel): - """ - Configuration model for BPM array - - Parameters - ---------- - pos : DeviceAccess, optional - Orbit device name - h_index : int, optional - Index in the array - v_index : int, optional - Index in the array - """ - - model_config = ConfigDict(arbitrary_types_allowed=True, extra="forbid") - - pos: DeviceAccess | None - h_index: int = 0 - v_index: int = 0 - - -class BPMArrayModel: - """ - Class that map an orbit device to individual BPM - """ - - def __init__(self, cfg: ConfigModel): - self._cfg = cfg - - def get_pos_device(self) -> DeviceAccess | None: - """ - Get device handle used for position reading - - Returns - ------- - DeviceAccess - Orbit DeviceAcess - """ - return self._cfg.pos - - def get_h_index(self) -> int: - return self._cfg.h_index - - def get_v_index(self) -> int: - return self._cfg.v_index - - def __repr__(self): - return __pyaml_repr__(self) diff --git a/pyaml/bpm/bpm_model.py b/pyaml/bpm/bpm_model.py index 33e35962..02b324ae 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_titl_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..729c4af2 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 + y_pos_index: int | 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/control/abstract_impl.py b/pyaml/control/abstract_impl.py index c117b6bb..c07ce51f 100644 --- a/pyaml/control/abstract_impl.py +++ b/pyaml/control/abstract_impl.py @@ -2,7 +2,6 @@ from numpy import double from numpy.typing import NDArray -from ..bpm.bpm_array_model import BPMArrayModel from ..bpm.bpm_model import BPMModel from ..common import abstract from ..common.abstract_aggregator import ScalarAggregator @@ -144,16 +143,20 @@ class CSBPMArrayMapper(CSScalarAggregator): Wrapper to a native CS aggregator for BPM """ - def __init__(self, dev: DeviceAccess, _indices: list[int]): - self._indices = _indices - self._dev = dev + 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]: - allValues: np.array = self._dev.get() - return allValues[self._indices] + # 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() @@ -294,44 +297,35 @@ def unit(self) -> list[str]: class RBpmArray(abstract.ReadFloatArray): """ - Class providing read access to a BPM position (x,y) of a control system + Class providing read access to a BPM position or offset [x,y] of a control system """ - def __init__(self, model: BPMModel, devs: list[DeviceAccess]): - self.__model = model - self.__devs = devs - - # 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 - def unit(self) -> str: - return self.__model.get_pos_devices()[0].unit() - - -# ------------------------------------------------------------------------------ - - -class RBpmsArray(abstract.ReadFloatArray): - """ - Class providing read access to a BPM positions array of a control system - """ - - def __init__(self, model: BPMArrayModel, dev: DeviceAccess): + def __init__(self, model: BPMModel, hDev: DeviceAccess, vDev: DeviceAccess): self._model = model - self._h_idx = model.get_h_index() - self._v_idx = model.get_v_index() - self.__dev = dev + 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: - allPos = self.__dev.get() - return np.array([allPos[self._h_idx], allPos[self._v_idx]]) - - # 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_device().unit() + return self._model.get_pos_devices()[0].unit() # ------------------------------------------------------------------------------ @@ -343,51 +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) - - 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() - - -# ------------------------------------------------------------------------------ - - -class RWBpmOffsetArray(abstract.ReadWriteFloatArray): - """ - Class providing read write access to a BPM offset 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()]) - - # Sets the value - def set(self, value: NDArray[np.float64]): - self.__devs[0].set(value[0]) - self.__devs[1].set(value[1]) + 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_offset_devices()[0].unit() + return self._model.get_tilt_device().unit() # ------------------------------------------------------------------------------ diff --git a/pyaml/control/controlsystem.py b/pyaml/control/controlsystem.py index 92518f46..45a461f2 100644 --- a/pyaml/control/controlsystem.py +++ b/pyaml/control/controlsystem.py @@ -1,7 +1,7 @@ from abc import ABCMeta, abstractmethod +from typing import Tuple from ..bpm.bpm import BPM -from ..bpm.bpm_array_model import BPMArrayModel from ..bpm.bpm_model import BPMModel from ..common.abstract import RWMapper from ..common.abstract_aggregator import ScalarAggregator @@ -15,8 +15,6 @@ CSStrengthScalarAggregator, RBetatronTuneArray, RBpmArray, - RBpmsArray, - RWBpmOffsetArray, RWBpmTiltScalar, RWHardwareArray, RWHardwareScalar, @@ -72,6 +70,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 @@ -101,11 +105,10 @@ def create_magnet_hardware_aggregator( return agg def create_bpm_aggregators(self, bpms: list[BPM]) -> list[ScalarAggregator]: - if len(bpms) == 0: - return None + # return [None,None,None] - if any([isinstance(b.model, BPMModel) for b in bpms]): - # simple bpm aggregator + 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() @@ -115,29 +118,56 @@ def create_bpm_aggregators(self, bpms: list[BPM]) -> list[ScalarAggregator]: aggh.add_devices(devs[0]) aggv.add_devices(devs[1]) return [agg, aggh, aggv] - elif any([isinstance(b.model, BPMArrayModel) for b in bpms]): - # Mapper to a native CS aggregator - dev = self.attach_array([bpms[0].model.get_pos_device()])[0] - if any( - [self.attach_array([b.model.get_pos_device()])[0] != dev for b in bpms] - ): - raise PyAMLException("BPMArrayModel must share the same CS device") - # CS BPM aggregator - hv_idx = [] - h_idx = [] - v_idx = [] + + elif any([b.model.is_pos_indexed() for b in bpms]): + # Aggregator for indexed BPMs + allH = [] + hIdx = [] + allV = [] + vIdx = [] + allHV = [] for b in bpms: - hv_idx.append(b.model.get_h_index()) - hv_idx.append(b.model.get_v_index()) - h_idx.append(b.model.get_h_index()) - v_idx.append(b.model.get_v_index()) - agg = CSBPMArrayMapper(dev, hv_idx) - aggh = CSBPMArrayMapper(dev, h_idx) - aggv = CSBPMArrayMapper(dev, v_idx) + 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( - "BPMArrayModel and BPMModel cannot be mixed in the same array" + "Indexed BPM and scalar values cannot be mixed in the same array" ) def set_energy(self, E: float): @@ -213,21 +243,21 @@ def fill_device(self, elements: list[Element]): self.add_magnet(m) elif isinstance(e, BPM): - if isinstance(e.model, BPMArrayModel): - # TODO: Handle tilt and offset - posDev = self.attach_array([e.model.get_pos_device()])[0] - positions = RBpmsArray(e.model, posDev) - e = e.attach(self, positions, None, None) - self.add_bpm(e) - else: - 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) - e = e.attach(self, positions, offsets, tilt) - self.add_bpm(e) + 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 = RBpmArray(e.model, ahOffsetDev, ahOffsetDev) + e = e.attach(self, positions, offsets, tilt) + self.add_bpm(e) elif isinstance(e, RFPlant): attachedTrans: list[RFTransmitter] = [] From a26d42c12d9d0f8eaa2447940e4e059c63b4ee49 Mon Sep 17 00:00:00 2001 From: PONS Date: Wed, 14 Jan 2026 15:54:57 +0100 Subject: [PATCH 3/7] Various fix --- pyaml/bpm/bpm_simple_model.py | 4 +- pyaml/bpm/bpm_tiltoffset_model.py | 4 ++ pyaml/control/abstract_impl.py | 54 ++++++++++++++++++- pyaml/control/controlsystem.py | 3 +- .../tango-pyaml/tango/pyaml/controlsystem.py | 3 ++ 5 files changed, 64 insertions(+), 4 deletions(-) diff --git a/pyaml/bpm/bpm_simple_model.py b/pyaml/bpm/bpm_simple_model.py index 729c4af2..582a6025 100644 --- a/pyaml/bpm/bpm_simple_model.py +++ b/pyaml/bpm/bpm_simple_model.py @@ -33,8 +33,8 @@ class ConfigModel(BaseModel): x_pos: DeviceAccess | None y_pos: DeviceAccess | None - x_pos_index: int | None - y_pos_index: int | None + x_pos_index: int | None = None + y_pos_index: int | None = None class BPMSimpleModel(BPMModel): 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 c07ce51f..87831ebc 100644 --- a/pyaml/control/abstract_impl.py +++ b/pyaml/control/abstract_impl.py @@ -297,7 +297,7 @@ def unit(self) -> list[str]: class RBpmArray(abstract.ReadFloatArray): """ - Class providing read access to a BPM position or offset [x,y] of a control system + Class providing read access to a BPM position [x,y] of a control system """ def __init__(self, model: BPMModel, hDev: DeviceAccess, vDev: DeviceAccess): @@ -363,6 +363,58 @@ def unit(self) -> str: # ------------------------------------------------------------------------------ +class RWBpmOffsetArray(abstract.ReadWriteFloatArray): + """ + Class providing read write access to a BPM offset [x,y] of a control system + """ + + 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: + 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]): + 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 Assume that x and y, offsets and positions + # have the same unit + def unit(self) -> str: + return self._model.get_pos_devices()[0].unit() + + +# ------------------------------------------------------------------------------ + + class RWRFVoltageScalar(abstract.ReadWriteFloatScalar): """ Class providing read write access to cavity voltage diff --git a/pyaml/control/controlsystem.py b/pyaml/control/controlsystem.py index 45a461f2..2c3f296c 100644 --- a/pyaml/control/controlsystem.py +++ b/pyaml/control/controlsystem.py @@ -15,6 +15,7 @@ CSStrengthScalarAggregator, RBetatronTuneArray, RBpmArray, + RWBpmOffsetArray, RWBpmTiltScalar, RWHardwareArray, RWHardwareScalar, @@ -255,7 +256,7 @@ def fill_device(self, elements: list[Element]): avOffsetDev = self.attach_indexed(vOffsetDev, e.model.y_offset_index()) positions = RBpmArray(e.model, ahDev, avDev) tilt = RWBpmTiltScalar(e.model, atiltDev) - offsets = RBpmArray(e.model, ahOffsetDev, ahOffsetDev) + offsets = RWBpmOffsetArray(e.model, ahOffsetDev, avOffsetDev) e = e.attach(self, positions, offsets, tilt) self.add_bpm(e) diff --git a/tests/dummy_cs/tango-pyaml/tango/pyaml/controlsystem.py b/tests/dummy_cs/tango-pyaml/tango/pyaml/controlsystem.py index d1d62202..7921000a 100644 --- a/tests/dummy_cs/tango-pyaml/tango/pyaml/controlsystem.py +++ b/tests/dummy_cs/tango-pyaml/tango/pyaml/controlsystem.py @@ -24,6 +24,9 @@ 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) + def attach(self, devs: list[DeviceAccess]) -> list[DeviceAccess]: newDevs = [] for d in devs: From d6bd4f3c26e59244fd6c4dd98dbc69c0208993ab Mon Sep 17 00:00:00 2001 From: PONS Date: Wed, 14 Jan 2026 15:58:58 +0100 Subject: [PATCH 4/7] Restaured comment --- pyaml/bpm/bpm.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pyaml/bpm/bpm.py b/pyaml/bpm/bpm.py index fb8e4cc9..acc35e75 100644 --- a/pyaml/bpm/bpm.py +++ b/pyaml/bpm/bpm.py @@ -29,6 +29,8 @@ def __init__(self, cfg: ConfigModel): ---------- name : str Element name + model : BPMModel + BPM model in charge of computing beam position """ super().__init__(cfg.name) From 694cb1a7402601e7a4a907fa09a95b433be3ba68 Mon Sep 17 00:00:00 2001 From: PONS Date: Thu, 15 Jan 2026 08:49:44 +0100 Subject: [PATCH 5/7] Typo --- pyaml/bpm/bpm_model.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyaml/bpm/bpm_model.py b/pyaml/bpm/bpm_model.py index 02b324ae..19fd21d3 100644 --- a/pyaml/bpm/bpm_model.py +++ b/pyaml/bpm/bpm_model.py @@ -90,7 +90,7 @@ def tilt_index(self) -> int | None: """ return None - def is_titl_indexed(self) -> bool: + def is_tilt_indexed(self) -> bool: return self.tilt_index() is not None def x_offset_index(self) -> int | None: From 666ef0666da93106d7a63565c20e2045fc77909e Mon Sep 17 00:00:00 2001 From: PONS Date: Thu, 15 Jan 2026 08:53:30 +0100 Subject: [PATCH 6/7] Added unit test --- tests/config/bpms.yaml | 14 +++++++++++++ .../tango-pyaml/tango/pyaml/attribute.py | 20 +++++++++++------- .../tango/pyaml/attribute_read_only.py | 21 ++----------------- .../tango-pyaml/tango/pyaml/controlsystem.py | 6 +++++- tests/test_bpm_controlsystem.py | 12 +++++++++++ 5 files changed, 46 insertions(+), 27 deletions(-) diff --git a/tests/config/bpms.yaml b/tests/config/bpms.yaml index 14ec1a84..b5a16bb3 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-04Position + 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 7921000a..68fee777 100644 --- a/tests/dummy_cs/tango-pyaml/tango/pyaml/controlsystem.py +++ b/tests/dummy_cs/tango-pyaml/tango/pyaml/controlsystem.py @@ -25,9 +25,12 @@ def __init__(self, cfg: ConfigModel): self.__DEVICES = {} def attach_array(self, devs: list[DeviceAccess]) -> list[DeviceAccess]: - return self.attach(devs) + 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: @@ -39,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() From e7ab7f9c3e6e8954fcadc7a021e7b9c4c2aa8bbe Mon Sep 17 00:00:00 2001 From: PONS Date: Thu, 15 Jan 2026 15:37:26 +0100 Subject: [PATCH 7/7] Typo --- tests/config/bpms.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/config/bpms.yaml b/tests/config/bpms.yaml index b5a16bb3..e0efafec 100644 --- a/tests/config/bpms.yaml +++ b/tests/config/bpms.yaml @@ -68,7 +68,7 @@ devices: y_pos_index: 1 x_pos: type: tango.pyaml.attribute_read_only - attribute: srdiag/bpm/c01-04Position + attribute: srdiag/bpm/c01-04/Position unit: mm y_pos: type: tango.pyaml.attribute_read_only