Naming of signals in ophyd-async is different to ophyd.v1

[whs92]

In ophyd v1 the names of signals of components of a device are created by joining names with a _

so a device like:

class Stage(Device):
    x = Cpt(EpicsMotor, 'X')
    y = Cpt(EpicsMotor, 'Y')

class StageOfStage(Device):
    a = Cpt(Stage, 'A')
    b = Cpt(Stage, 'B')

stage_of_stage = StageOfStage('TEST:', name='stage_of_stage'

would have component names like:

stage_a_x
stage_a_y
stage_b_x
stage_b_y

In ophyd-async component names are joined with a - character instead and any _ characters are removed from the child name.

            child_name = f"{self.name}-{child_name.strip('_')}" if self.name else ""

https://github.com/bluesky/ophyd-async/blob/main/src/ophyd_async/core/_device.py#L113

If I include ophyd.v1 and ophyd-async devices in a plan together then the signals in event documents from ophyd-async and ophyd.v1 devices look different.

Why is this useful?

@coretl

The reason it was changed it because given a dash separated name you can split on the dash and walk the tree of devices from the root. With an underscore separated name you cannot.

@DiamondJoseph suggests it's possible to iterate through the descriptor document to walk the tree. (I've not tried this)

Why is this a problem?

1. Inconsistency in the signal names in documents is confusing when using ophyd v1 and ophyd-async in the same place
2. The - can be interpreted as a -Ve sign in some cases e.g if the signal is monitored in a plan and then you try to access the stream from the run using the databroker client.

Proposal

Standardise the signal naming of signals to be the same as in ophyd.v1

[coretl]

@DominicOram @DiamondJoseph @callumforrester will this break anything if we make ophyd-async go back to _ separated names?

@whs92 I can be convinced on changing - to _ but I still think we should strip underscores, as we sometimes have a signal read_ which is named like that because otherwise it would shadow read().

Also, in your example, if x was named _x, would you expect the name to be stage_a__x or stage_a_x?

[callumforrester]

Why did we use - originally? And did we ever write it down?

[coretl]

@coretl

The reason it was changed it because given a dash separated name you can split on the dash and walk the tree of devices from the root. With an underscore separated name you cannot.

[callumforrester]

Sorry, reading failure on my part.

@DiamondJoseph suggests it's possible to iterate through the descriptor document to walk the tree. (I've not tried this)

Neither have I, but it makes sense to me.

Are there any other major disadvantages we can think of for going back to _? If so, could make it configurable and push the tradeoff to the user? If not, I'm happy to just go with this.

[whs92]

Also, in your example, if x was named _x, would you expect the name to be stage_a__x or stage_a_x?

stage_a__x

Making it default to ophyd.v1 while also making it configurable seems like a great compromise

[dperl-dls]

Using a character for separation which is not a part of a valid python identifier was a significant improvement on the naming scheme in Ophyd IMO; I would hate to have to go through all our data handling code and rename everything back (but obviously I would be willing to if it's really important).

Is it just the aesthetics of having the different schemes that bothers you @whs92 or are there also functional problems with this?

I would be okay with making it configurable too, but that has some potential pitfalls - it should at least be one global config singleton so that we don't have some devices accidentally unconfigured - and seems like a lot of work.

[DiamondJoseph]

"parent-child" == parent.child.name - if you're going to interrogate your devices based on the documents, without relying on any information passed in the descriptor, then you can replace - with . (e: Assuming that your children are only named automatically from setting the parent name, and that your parent is a valid Python variable).

I agree that having a character that isn't a valid Python variable name actively prevents confusion and is a design choice we should stick by- if I have a device named mirror_heater with a child mirror_heater_temp, I can't distinguish that from a case of having mirror, with a child mirror_heater.

There must be something that unambiguously cannot be a variable name to allow you to split on it reliably.

[whs92]

All good points, thanks everyone for your input.

@dperl-dls

Is it just the aesthetics

No, when you monitor a signal the resulting stream name takes the name of the signal. Using a - character there means you can't access the stream using run.stream-name_monitor and instead have to use run["stream-name_monitor"]. This isn't a breaking problem, but it was confusing.

I agree with @DiamondJoseph point about the mirror and mirror_heater example. However since all ophyd.v1 users are aware of this standard naming construction, we know not to form this. The heater should be a subcomponent of the mirror that it's heating and part of.

I'd argue that this all might seem minor but trying to reduce differences between ophyd v1 and v2 is helpful to drive adoption. Diamond is maybe the only place where only ophyd async is used. Most places are using and have been using v1 for many years and are now exploring async. We have mixed environments.

If we leave things as they are I have to write docs for my users explaining why their data contains keys which look different. Aesthetics matter.

[dperl-dls]

Yes, sorry, I didn't mean to imply aesthetics don't matter at all, it's definitely still a burden to have to keep the naming convention in mind. But we did run with mixed ophyd and ophyd-async devices for about a year and it wasn't really a problem, and we still have a few ophyd v1 devices kicking around.

But then similarly "we know not to form this" about the above mirror heater example is a significant mental burden too - and we have to onboard tens of new people to writing ophyd devices who will not find it easy to understand why they shouldn't name the child of the mirror "heater" - in fact, knowing when to do this and when not is quite subtle, and not systematic like "one uses dashes and the other underscores", and may also lead to some unaesthetic naming choices.

[whs92]

I would also be happy if we make it possible to specify this in ophyd.v1.

My main problem is the inconsistency. I don't really care which one is used, I just want it to be the same, or be possible to configure it to be the same.

I agree that the names in ophyd-async are easier to seperate and more likely to be unique in the dataset.

[coretl]

I suggest we discuss this in the collab meeting on Monday. @whs92 are you around for this?

[whs92]

Yep. See you there.