Separation or renaming of ConfigModels

[gubaidulinvadim]

Description, motivation and use case
Right now, the ConfigModel is kept in the same file as the actual class with implementation. This is nice to keep everything in the same place. However, when writing the script without a config file, there is a small issue when ConfigModels are imported. One needs to do

from something.something.bpm import ConfigModel as BPMConfigModel
from something.something.quadrupole import Config Model as QuadrupoleConfigModel

Is it required for the Factory to have for everyclass a ConfigModel and not a SomethingConfigModel? I imagine it is easier with ConfigModel for everything in the same file. But it should be possible to modify it.

Proposed solution
Either we need to rename All ConfigModels or put the configuration in a separate place

Example
Something like below should be possible

from something.something import BPMConfigModel, QuadrupoleConfigModel

This technically only reduces the lines of code by one. But I find it to be much better personally.

Additional context
@TeresiaOlsson was mentioning this same issue some time ago.

Checklist

• I've assigned this issue to a project
• I've @-mentioned relevant people
• I've checked, there are no duplicate issues

[TeresiaOlsson]

I'm in favour of unique names. The documentation also becomes complicated to understand if there are several classes with the same name.

Personally I also don't like that the config model is used to initialize a device object. That also makes the documentation difficult to understand because you need to look at a different class to understand what the input should be. I have an idea in mind where every class would explicitly define which input it needs and the config model is just used to validate the data. So validation is separated from object creation.

I have an idea for this that I'm working on but I didn't have time to finish it because of workshop preparations. I want to refactor the factory because I would like at some point in the future to be able to use the same config to initialize devices for our IDs and connect the work our ID group is doing on measuring feedforward tables to pyAML. But that development is bluesky using ophyd devices which means the factory needs to be sufficently general to also create objects from external packages.

Next week I will get back to working on the idea I have for how it could be done.

[JeanLucPons]

Nothing prevent the factory to create any object.
If you want to create a class inheriting from StandardReadable that also extends pyaml.diagnostics.BetatronTuneMonitor is it feasible. You can then keep the AT access and just override the CS part. This can be done during the attachment process.
Then you BetatronTuneMonitor will be an OphydDevice accesible from PyAML tuning tools.

[JeanLucPons]

Description, motivation and use case Right now, the ConfigModel is kept in the same file as the actual class with implementation. This is nice to keep everything in the same place. However, when writing the script without a config file, there is a small issue when ConfigModels are imported. One needs to do

The idea there was rather to create a dynamic API. It was already discussed.

[gubaidulinvadim]

@JeanLucPons Could you give a link to the discussion if it was on GitHub?

[JeanLucPons]

We have already several discussions around this topics with mainly @TeresiaOlsson.
I will try to find links.
Basically the idea is to create a dynamic generated API (using current rules defined for PYAMLCLASS and introspection) to allow simple writing for object creation. No need of from something.something.bpm import ConfigModel as BPMConfigModel, you will just have to import something like below:

import pyaml
import tango.pyaml
qAtt = Attribute(attribute="srmag/vps-qf1/c01-e/current", unit="A"))
myQuad = Quadrupole(name="QF1E-C04", model=LinearMagnetModel(calibration_factor=0.99123,powerconverter=qAtt))

Above is without the device_catalog, but using it you should have:

import pyaml
# powerconverter definition for QF1E-C04 will be in the catalog.
myQuad = Quadrupole(name="QF1E-C04", model=LinearMagnetModel(calibration_factor=0.99123))

This is a bit out of spec however, i don't remember that we have something on this in the specification document.
I already discussed about this topics with @simoneliuzzo and he seems a bit against dynamic creation (no yaml usage), but I may misunderstood him.

[simoneliuzzo]

@gubaidulinvadim, @TeresiaOlsson @JeanLucPons ,

I do not understand clearly the discussion and the topic of this pull request.

I am not against reorganizing ConfigModel as the issue topic.

if you need to import ConfigModel is because you want to manually change the configuration. Should this be possible/easy? if you configuration is well done in the yaml file, why would you need this?

best regards
simone

[gubaidulinvadim]

@JeanLucPons, either way is fine for me. I just proposed the simplest solution of renaming things. Writing BPM(BPMConfigModel(...)) to create an object does not bother me too much. I don't really follow why config based on class rather than module is important in #107 . If it bothers someone to have BPM(BPMConfigModel), maybe the simplest solution is to:

1. Unique names for ConfigModel
2. fill_device() and equivalents will take ConfigModel and create a corresponding object under the hood.

@simoneliuzzo Yes, this small issue is with creating objects manually. Even during the workshop, people had some issues with writing yaml files, and some things are more practical to do via coding (defining an array/family) and then defining the measurement tool. At least for now, when we are in the development. It is already possible to create things manually and I would not say that it is too complicated.

[TeresiaOlsson]

For us it is very important to be able to configure devices that aren't pyAML devices or we can never connect the development our ID group is doing to pyAML. They are using ophyd devices and bluesky to measure feed forward tables and won't replace the work they have spent a lot of time on and likes by reimplementing the same functionality in pyAML. We also have other things started before the pyAML project where we already have devices and applications which we might want to integrate at some point rather than rewriting everything from scratch.

I think as long as a device follows the interface required by pyAML, e.g. set, get etc it should be possible to configure it and use it in measurements as if it was a pyAML device. But that requires a device factory which is sufficiently general to build objects from external classes without putting constraints on how the module needs to have been implemented. For example, requiring a field PYAMLCLASS in the module won't work. I have an idea for this that I'm working on and I hope I will have time to finish soon.

The other part is the documentation. For example, look at the current documentation for a combined function magnet https://pyaml.readthedocs.io/en/latest/api/pyaml.magnet.cfm_magnet.html#pyaml.magnet.cfm_magnet.CombinedFunctionMagnet. It is very difficult from that to understand which input parameters are needed. You need to understand that you need to scroll down and first create an object of ConfigModel that you then send into the CombinedFunctionMagnet. I think it would be better to not have the pydantic basemodels as input for creating the devices but only use them for validation. So you read in the yaml file, the data is validated by creating the ConfigModel and then you unwrap that into a dict and send it as input for creating the device. It also makes the code less dependent on pydantic since pydantic is just used for validation and nothing else.

[JeanLucPons]

I agree that the doc need a lot of work and that is a pain to write it (including code snippet, config example, etc)...

[JeanLucPons]

I think it would be better to not have the pydantic basemodels as input for creating the devices but only use them for validation. So you read in the yaml file, the data is validated by creating the ConfigModel and then you unwrap that into a dict and send it as input for creating the device. It also makes the code less dependent on pydantic since pydantic is just used for validation and nothing else.

This is a choice we can make as we are going to refurbish and break compatibility.
Note that a class and validation_class property has been added so ConfigModel and PYAMLCLASS are no longer mandatory.
However the BaseModel is still passed as one single arg but it can be unpacked.

[TeresiaOlsson]

I think it would be better to not have the pydantic basemodels as input for creating the devices but only use them for validation. So you read in the yaml file, the data is validated by creating the ConfigModel and then you unwrap that into a dict and send it as input for creating the device. It also makes the code less dependent on pydantic since pydantic is just used for validation and nothing else.

This is a choice we can make as we are going to refurbish and break compatibility. Note that a class and validation_class property has been added so ConfigModel and PYAMLCLASS are no longer mandatory. However the BaseModel is still passed as one single arg but it can be unpacked.

I agree. I have been thinking about it again now since it's the only solution I have managed to come up with for getting out of the things I'm stuck with in PR #214. I'm also working on an external package to provide a HAPPI inspired device database for the EPICS labs so we can have a better starting point for generating the pyAML input since if feels like the TANGO labs have an easier start due to the TANGO database. I'm currently having the same issues there when trying to figure out how to add the information needed for the pyAML devices and directly generating the yaml file or even better building the Accelerator from the database.

I'm now thinking something with three layers:

1. Models for validating the yaml file: this will be a generic model built from the existing ConfigModel + the extra fields that we have in the yaml file to be able to know which object to build (type/class/module etc). These models are only for validating the input data and will not be propagated further into the code.
2. The existing ConfigModel but potentially with unique names to solve the issue @gubaidulinvadim described. These models are for describing which data a certain business logic class needs and validate it during both object creation and dynamic changes to an existing object. Hopefully the validator decorator can be used to simplify the implementation of that.
3. Then business logic classes which are independent from pydantic. They just take simple attributes as input and are only connected to pydantic through an optional validation_class/model.

I will start writing a PR for the first step I think is needed to separate the pydantic basemodels out so you can see what I have in mind since it's difficult to explain in text.

[JeanLucPons]

Point 1 is ok for me (simple solution with unpacking).
Point 2: Using unique name is not convenient or we have to find a strict rule to avoid to have to fill the validation_class field.
Point 3: Ok similar to 1

[TeresiaOlsson]

Point 1 is ok for me (simple solution with unpacking). Point 2: Using unique name is not convenient or we have to find a strict rule to avoid to have to fill the validation_class field. Point 3: Ok similar to 1

I agree with 2 but I'm hoping the validator decorator will fix it by providing a quick way to fill the validation_class. I will see how well it works when I come to that part. Could be that it doesn't work out.