Skip to content

Conversation

@gupichon
Copy link
Contributor

@gupichon gupichon commented Jan 16, 2026

Description

DeviceAccess can provide a range of acceptable values.

Related Issue

Closes #106

Features/issues described there are:

  • DeviceAccess can provide a range of acceptable values.
  • Write on devices on the control system will now check the range if it is provided.

@gupichon gupichon requested a review from JeanLucPons January 16, 2026 16:55
@gupichon gupichon linked an issue Jan 16, 2026 that may be closed by this pull request
@gupichon gupichon marked this pull request as draft January 16, 2026 17:01
@gupichon gupichon marked this pull request as ready for review January 21, 2026 14:02
@gupichon-soleil
Copy link
Contributor

gupichon-soleil commented Jan 21, 2026

@JeanLucPons , it's ready for review.

@JeanLucPons
Copy link
Contributor

OK Thanks.

  • I'm not really in favor of the check_device_availability() which is useless for combined function magnets that use RWMapper as a read is done before.
  • Aggregator are not checked. Would you like to do the aggregator later in an other PR ?

@JeanLucPons
Copy link
Contributor

JeanLucPons commented Jan 21, 2026

It behaves very strangely ?

import time
from pyaml.accelerator import Accelerator
sr = Accelerator.load("tests/config/EBSTune.yaml")
mag = sr.live.get_magnet("QD2E-C04").strength
print( mag.get() )
print( mag._RWStrengthScalar__dev )
mag.set(100)
time.sleep(10)
print( mag.get() )
[ubuntu20acu.pons] > python tests/jlp_test.py 
21 Jan% 2026, 15:53:04 | WARNING | PyAML Tango control system binding (0.3.2) initialized with name 'live' and TANGO_HOST=ebs-simu-2:10000
-0.6178546595342231
Attribute(attribute='//ebs-simu-2:10000/srmag/vps-qd2/c04-e/current', unit='A', range=None)
0.0

No excpetion and the beam is lost !?

@JeanLucPons
Copy link
Contributor

JeanLucPons commented Jan 21, 2026

Addind a range in the yaml seems to work but the tango range seems to be ignored

pyaml.common.exception.PyAMLException: The values are out of range: 110.0, [10.0, 109.0] for device Attribute(attribute='//ebs-simu-2:10000/srmag/vps-qd2/c04-e/current', unit='A', range=(10.0, 109.0))
image

@gupichon-soleil
Copy link
Contributor

Addind a range in the yaml seems to work but the tango range seems to be ignored

pyaml.common.exception.PyAMLException: The values are out of range: 110.0, [10.0, 109.0] for device Attribute(attribute='//ebs-simu-2:10000/srmag/vps-qd2/c04-e/current', unit='A', range=(10.0, 109.0))
image

This needs to be fixed in tango-pyaml. I tested it with a mocker, but I may have done it wrong. I will check it.

@gupichon-soleil
Copy link
Contributor

OK Thanks.

* I'm not really in favor of the `check_device_availability()` which is useless for combined function magnets that use `RWMapper` as a read is done before.

* Aggregator are not checked. Would you like to do the aggregator later in an other PR ?

Ok, I will remove the availability check and add the range check to the aggregators.

@gupichon-soleil
Copy link
Contributor

That's strange, it works for me. With tango-pyaml only...

image

@JeanLucPons
Copy link
Contributor

OK I will double check, i might be confused by the fact the the range is "lazily" initialized.

@JeanLucPons
Copy link
Contributor

OK it works (I definitely forgot something) but I still do not know why it claims that the range is None ?
But as you said, it is link to tango-pyaml.

[ubuntu20acu.pons] > python tests/jlp_test.py 
22 Jan% 2026, 11:12:09 | WARNING | PyAML Tango control system binding (0.3.2) initialized with name 'live' and TANGO_HOST=ebs-simu-2:10000
-0.6178546595342231
Attribute(attribute='//ebs-simu-2:10000/srmag/vps-qd2/c04-e/current', unit='A', range=None)
Traceback (most recent call last):
  File "/home/esrf/pons/pyaml/tests/jlp_test.py", line 9, in <module>
    mag.set(-100)
  File "/operation/common/miniconda/envs/jlp-py312/lib/python3.12/site-packages/pyaml/control/abstract_impl.py", line 299, in set
    raise PyAMLException(f"The values are out of range: {current}, {dev_range} for device {self.__dev}")
pyaml.common.exception.PyAMLException: The values are out of range: 110.0, [0.0, 108.0] for device Attribute(attribute='//ebs-simu-2:10000/srmag/vps-qd2/c04-e/current', unit='A', range=None)

@gupichon-soleil
Copy link
Contributor

That range is the one defined in the YAML file. If you want it to be automatically filled, it would require fetching it from the device. Due to lazy instantiation, I don't think this is a good idea...

@JeanLucPons
Copy link
Contributor

OK. I think, you can use the _cfg field of the attached device (if not already filled by the conf) instead of creating a new range attribute but we can discuss this later in tango-pyaml.

@gupichon
Copy link
Contributor Author

It should be ok now @JeanLucPons

@gupichon
Copy link
Contributor Author

You have to test it with the matching tango-pyaml branch.

@gupichon
Copy link
Contributor Author

The developpement for pyaml-cs-oa must be done once we agree on the final implementation.

@JeanLucPons
Copy link
Contributor

JeanLucPons commented Jan 22, 2026

I'm sorry but i still do not manage to make it work :(

from pyaml.accelerator import Accelerator

sr = Accelerator.load("tests/config/EBSTune-range.yaml")
mag = sr.live.get_magnets("QForTune").strengths
mag.set( mag.get()*1000.0 )

Failed with:

[ubuntu20acu.pons] > python tests/jlp_test.py 
22 Jan% 2026, 15:39:21 | WARNING | Tango control system binding for PyAML initialized with name 'live' and TANGO_HOST=ebs-simu-2:10000
Traceback (most recent call last):
  File "/home/esrf/pons/pyaml/tests/jlp_test.py", line 5, in <module>
    mag.set( mag.get()*1000.0 )
  File "/operation/common/miniconda/envs/jlp-py312/lib/python3.12/site-packages/pyaml/arrays/magnet_array.py", line 30, in set
    self.__aggregator.set(nvalue)
  File "/operation/common/miniconda/envs/jlp-py312/lib/python3.12/site-packages/pyaml/control/abstract_impl.py", line 173, in set
    self._devs.set(newHardwareValues)
  File "/operation/common/miniconda/envs/jlp-py312/lib/python3.12/site-packages/tango/pyaml/multi_attribute.py", line 85, in set
    self[index]._attribute_dev.write_attribute_reply(call_id, timeout)
  File "/operation/common/miniconda/envs/jlp-py312/lib/python3.12/site-packages/tango/green.py", line 231, in greener
    return executor.run(fn, args, kwargs, wait=wait, timeout=timeout)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/operation/common/miniconda/envs/jlp-py312/lib/python3.12/site-packages/tango/green.py", line 121, in run
    return fn(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^
  File "/operation/common/miniconda/envs/jlp-py312/lib/python3.12/site-packages/tango/device_proxy.py", line 1042, in __DeviceProxy__write_attribute_reply
    return __write_attributes_reply__(self, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/operation/common/miniconda/envs/jlp-py312/lib/python3.12/site-packages/tango/device_proxy.py", line 953, in __write_attributes_reply__
    return self.__write_attributes_reply(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
tango._tango.DevFailed: DevFailed[
    DevError[
        desc = Failed to execute DeviceProxy::write_attributes_reply() on device srmag/vps-qd2/c04-e, object(s) Current
        origin = DeviceProxy::write_attributes_reply()
        reason = API_AttributeFailed
        severity = ERR
    ]
]

which is the buggy error we get when the Tango PS return an error, see here

Here is my tune file:

type: pyaml.accelerator
facility: ESRF
machine: sr
energy: 6e9
simulators:
  - type: pyaml.lattice.simulator
    lattice: sr/lattices/ebs.mat
    name: design
controls:
  - type: tango.pyaml.controlsystem
    tango_host: ebs-simu-2:10000
    name: live
data_folder: /data/store
arrays:
  - type: pyaml.arrays.magnet
    name: QForTune
    elements:
      - QD2E-C04
      - QD2A-C05
      - QD2E-C05
      - QD2A-C06
      - QD2E-C06
      - ...
devices:
- type: pyaml.magnet.quadrupole
  name: QF1E-C04
  model:
    type: pyaml.magnet.linear_model
    calibration_factor: 1.00054
    crosstalk: 1.0
    curve:
      type: pyaml.configuration.csvcurve
      file: sr/magnet_models/QF1_strength.csv
    unit: 1/m
    powerconverter:
      type: tango.pyaml.attribute
      attribute: srmag/vps-qf1/c04-e/current
      range: [10,109]
      unit: A
- type: pyaml.magnet.quadrupole
  name: QF1A-C05
  model:
    type: pyaml.magnet.linear_model
    calibration_factor: 0.996841
    crosstalk: 1.0
    curve:
      type: pyaml.configuration.csvcurve
      file: sr/magnet_models/QF1_strength.csv
    unit: 1/m
    powerconverter:
      type: tango.pyaml.attribute
      attribute: srmag/vps-qf1/c05-a/current
      range: [10,109]
      unit: A
...

@JeanLucPons
Copy link
Contributor

In control/abstract_impl.py :

        dev_range = self._devs.get_range()
        print(newHardwareValues)
        print(dev_range)
        if not check_range(newHardwareValues, dev_range):
            raise PyAMLException(f"The values are out of range: {newHardwareValues}, {dev_range} for device {self._devs}")

outputs

[110. 110. 110. 110. 110. 110. 110. 110. 110. 110. 110. 110. 110. 110.
110. 110. 110. 110. 110. 110. 110. 110. 110. 110. 110. 110. 110. 110.
110. 110. 110. 110. 110. 110. 110. 110. 110. 110. 110. 110. 110. 110.
110. 110. 110. 110. 110. 110. 110. 110. 110. 110. 110. 110. 110. 110.
110. 110. 110. 110. 110. 110. 110. 110. 110. 110. 110. 110. 110. 110.
110. 110. 110. 110. 110. 110. 110. 110. 110. 110. 110. 110. 110. 110.
110. 110. 110. 110. 110. 110. 110. 110. 110. 110. 110. 110. 110. 110.
110. 110. 110. 110. 110. 110. 110. 110. 110. 110. 110. 110. 110. 110.
110. 110. 110. 110. 110. 110. 110. 110. 110. 110. 110. 110.]
None

hardware setpoints are the one expected.
range is None ?
Is there something wrong in my config ?

@gupichon
Copy link
Contributor Author

I've turn your case into a test. It works. I've done a minor change due to the improvement in the dummy CS but nothing critical.

@gupichon
Copy link
Contributor Author

The exception (simplified), before I handle it as a valid result for the test:

@pytest.mark.parametrize(
"install_test_package",
[{"name": "tango-pyaml", "path": "tests/dummy_cs/tango-pyaml"}],
indirect=True,
)
def test_rf_multi_notrans(install_test_package):
sr: Accelerator = Accelerator.load("tests/config/EBSTune-range.yaml")
mag = sr.live.get_magnets("QForTune").strengths
mag.set(mag.get() * 1000.0)

tests/test_range_array.py:15

Traceback (most recent call last):
File "tests/test_range_array.py", line 15, in test_rf_multi_notrans
mag.set(mag.get() * 1000.0)
File "pyaml/arrays/magnet_array.py", line 30, in set
self.__aggregator.set(nvalue)
File "pyaml/control/abstract_impl.py", in CSStrengthScalarAggregator.set
raise PyAMLException(
pyaml.common.exception.PyAMLException: The values are out of range.

newHardwareValues:
[0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0.,
0., 0., 0., 0., 0., 0., 0., 0., 0., ... 0., 0., 0.]

dev_range (flat list of bounds, min/max pairs):
[None, None, None, None, ...,
10.0, 109.0,
None, None, None, ...]

device list (DeviceAccessList):
[
Attribute(attribute='//ebs-simu-3:10000/srmag/vps-qd2/c04-e/current', unit='A', range=None),
Attribute(attribute='//ebs-simu-3:10000/srmag/vps-qd2/c05-a/current', unit='A', range=None),
...
Attribute(attribute='//ebs-simu-3:10000/srmag/vps-qf1/c05-a/current', unit='A', range=(10.0, 109.0)),
...
]

@JeanLucPons
Copy link
Contributor

Still failing.

[ubuntu20acu.pons] > python tests/jlp_test.py 
22 Jan% 2026, 16:51:51 | WARNING | Tango control system binding for PyAML initialized with name 'live' and TANGO_HOST=ebs-simu-3:10000
Traceback (most recent call last):
  File "/home/esrf/pons/pyaml/tests/jlp_test.py", line 5, in <module>
    mag.set( mag.get()*1000.0 )
  File "/operation/common/miniconda/envs/jlp-py312/lib/python3.12/site-packages/pyaml/arrays/magnet_array.py", line 30, in set
    self.__aggregator.set(nvalue)
  File "/operation/common/miniconda/envs/jlp-py312/lib/python3.12/site-packages/pyaml/control/abstract_impl.py", line 168, in set
    if not check_range(newHardwareValues, dev_range):
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/operation/common/miniconda/envs/jlp-py312/lib/python3.12/site-packages/pyaml/control/abstract_impl.py", line 48, in check_range
    raise ValueError(f"dev_range must have an even length, got {r.size}")
ValueError: dev_range must have an even length, got 1

@gupichon
Copy link
Contributor Author

gupichon commented Jan 22, 2026

Ok, it's strange. The error is in the implementation. get_range() returns None instead of an array. Even an array of None is ok but it must be an array with a correct number of elements.
Are you sure you have the good version of tango-pyaml (branch 28. pull request: python-accelerator-middle-layer/tango-pyaml#28) ?

@JeanLucPons
Copy link
Contributor

OK it is better i was in branch 25.
So tomorrow I update pyaml-cs-oa.

Still this ?

    def set(self, value: float):
        available = self.__dev.check_device_availability()
        if not available:

@gupichon
Copy link
Contributor Author

Still this ?

    def set(self, value: float):
        available = self.__dev.check_device_availability()
        if not available:

I missed this one.

@gupichon
Copy link
Contributor Author

It should be ok now.

@JeanLucPons
Copy link
Contributor

For me it is fine for now on the pyaml side.
However, some work needed to improve the error message.

tango-pyaml

The values are out of range: [110. .... 110.], [0.0, 108.0, .. 0.0, 120.0] for device MultiAttribute(attributes=[], name='', unit='', range=None)

pyaml-cs-oa

The values are out of range: [110. ... 110.], [0.0, 108.0 ... 0.0, 120.0] for device OAScalarAggregator(
TangoRW(attribute='//ebs-simu-2:10000/srmag/vps-qd2/c04-e/current', timeout_ms=3000, range=[10.0, 109.0], unit='A')
TangoRW(attribute='//ebs-simu-2:10000/srmag/vps-qd2/c05-a/current', timeout_ms=3000, range=[10.0, 109.0], unit='A')
TangoRW(attribute='//ebs-simu-2:10000/srmag/vps-qd2/c05-e/current', timeout_ms=3000, range=[10.0, 109.0], unit='A')
...

It would be good to list out of range values according to their respective range and device.

i.e.

Values out of range:
110 A, '//ebs-simu-2:10000/srmag/vps-qd2/c04-e/current' [10.0, 109.0]
110 A, '//ebs-simu-2:10000/srmag/vps-qd2/c05-e/current' [10.0, 109.0]
...

JeanLucPons
JeanLucPons previously approved these changes Jan 23, 2026
@gupichon
Copy link
Contributor Author

I agree, it requires another function to build a beautiful error message. I will do it now; it won’t take long, and the required tests are already there.

@gupichon
Copy link
Contributor Author

I let you do a last review and it should be ok for merge.

@JeanLucPons
Copy link
Contributor

OK it is fine. Thanks.
I just updated wrong OphydAsynch test files.
@simoneliuzzo , it is ok for you ?

@gupichon gupichon requested a review from simoneliuzzo January 23, 2026 10:10
@gupichon gupichon self-assigned this Jan 23, 2026
@JeanLucPons
Copy link
Contributor

May be you can update the version number as well.

@gupichon
Copy link
Contributor Author

gupichon commented Jan 23, 2026

May be you can update the version number as well.

Ok, has there is no interface breaks, i'll update only the patch number.

@JeanLucPons JeanLucPons self-requested a review January 23, 2026 10:33
@gupichon gupichon merged commit 56bfb70 into main Jan 23, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature: Add setpoint check before applying strengths

4 participants