inverted Digital outputs

[philipstarkey]

Original report (archived issue) by Jan Werkmann (Bitbucket: PhyNerd, GitHub: PhyNerd).

---

It would be nice to have the posibility to declare a Digitaloutput as inverted. This output would not behave any differently but the ui buttons in blacs would invert(pressed=low, unpressed=high). This would reduce confusion when using shutters that are inverted and ones that aren't at the same time.

[philipstarkey]

Original comment by Philip Starkey (Bitbucket: pstarkey, GitHub: pstarkey).

---

I'm personally in favour of keeping the device tabs actually matching what the output of the device does, rather than something else that may not be immediately obvious if you are trying to directly debug the physical output, rather than looking at the state of a device that may be attached at the end of the cable. In short, the current device tabs are supposed to directly represent the hardware (a good manifestation of this is the fact that channels show up in BLACS even if they aren't in your connection table).

However, I do understand the usefulness of having an inverted digital controls for shutters. It has been a long term aim to create virtual devices; tabs in BLACS that do not correspond to a specific device but instead draw controls from one or more devices. Implementing this, along with the ability to create a control that represents the labscript subclass (aka a Shutter) I think would be the best approach. It maintains a view of the hardware (device tab) if you need that, but also allows you to create customised views that represent something else (like all MOT controls and the digital lines are represented as open/close buttons when they are shutters).

This wouldn't be too hard to implement I think. I already planned for this long ago, so the internal objects that represent each channel actually already support the connection of multiple widgets. We just need an interface for creating virtual device tabs, and a shutter widget that allows for the state to be inverted to that of the internal DigitalOut object in BLACS.

[philipstarkey]

Original comment by Ian B. Spielman (Bitbucket: Ian Spielman).

---

I am not sure if this question was to do with the blacs UI or with the scripting of devices. But except for syntax I TOTALL AGREE. Already analog outs don't display what the hardware is doing, they have scaling and units. So we should just move this infrastructure over to digital outputs.

In the case of scripting see issue "32: Eliminate go_high, go_low syntax" in labscript, I would suggests doing

do_line.constant(t, 1) or do_line.constant(t, True) for example

instead. With this view a Digital output is really a one-bit analog out, and can have the same scaling and units that an analog devices has, of which inverting is the one that makes the most sense.

In this world, gohigh and golow would include the scaling by default so they could have the desired inverted behavior.

Also DO's should have the ability to do ramps too. For example to make pulse trains.

[philipstarkey]

Original comment by Chris Billington (Bitbucket: cbillington, GitHub: chrisjbillington).

---

Somwhat off topic, but I've been thinking about how to make labscript's internal timing model more sensible, and unifying digital and analog outputs is something I thought to do. It makes things simpler if the labscript core code can treat them the same, which would mean you would get digital pulse trains for free, and inversion of a digital output would just be another unit conversion (one with added syntactic sugar so you could turn it on and off with a keyword argument, but internally just another unit conversion).

I suspect all that would be needed to support pulse trains now though is to move the AnalogQuantity.customramp method to the Output class, which both AnalogQuantity and DigitalQuantity subclass. The you could have DigitalQuantity wrap the method under the name pulse_train - or you could rename it in the Output class to something that makes sense for both digital and analog outputs - having AnalogQuantity.customramp wrap it for backward compatibility.

Back on topic, I was agreeing with phil that the interface should show what the hardware is doing, but it's a good point that it already not doing that with analog outputs, since they support unit conversions. Perhaps our digital output widgets need to support "unit conversions" too - with inversion being essentially the only possible conversion even though you could have identity conversion just for the sake of having different names like "shutter open" vs "shutter closed" even for a non-inverted widget. You would definitely want a very obvious visible difference to the widget in BLACS for when it is in "hardware units" vs something else. Like, pink instead of green level of obvious, good taste in colour schemes notwithstanding.

Perhaps if the "virtual devices" idea were implemented, other tabs could go back to not supporting unit conversions, if you wanted them to be pure to the state of the hardware. But I suspect that would be annoying in practice, and that nobody would actually want to get rid of them.

[philipstarkey]

Original comment by Philip Starkey (Bitbucket: pstarkey, GitHub: pstarkey).

---

Ian: On the labscript side, the inverted digital output can already be achieved with the Shutter class via the open_state kwarg. So I believe the query was regarding the BLACS UI. I realise that doesn't solve the issues raised in labscript issue 37 (labscript-suite-temp/labscript#37) that you refer to, but at least it does provide some form of abstraction for the inversion. I've though for a while that most of the functionality in Shutter (like delays) should be moved into the DigitalQuantity class anyway.

For pulse trains, this is currently possible using the do.repeat_pulse_sequence(...) method, although there are several caveats due to the necessity of handling clock quantisation and the possibility of intersecting instructions/ramps on other channels. I believe @cbillington's plan is to handle this better in labscript v3.0 (or at least the planned changes for v3.0 will make this easier to do better).

EDIT: Looks like @cbillington wrote a response while I wrote mine. So mine does not address anything he's written!

EDIT 2: I think Ian updated his response while I was writing mine as well, so my above response possibly makes even less sense now.

[philipstarkey]

Original comment by Chris Billington (Bitbucket: cbillington, GitHub: chrisjbillington).

---

Oh, I didn't realise repeat_pulse_sequence was a wrapper around add_instruction with a function ramp instruction, I thought it was non-looping. Sweet, so that's already there.

[philipstarkey]

Original comment by Philip Starkey (Bitbucket: pstarkey, GitHub: pstarkey).

---

One possibility is that we could make tooltips show what the hardware state actually is. That might be an appropriate compromise (and could apply equally to analog and digital outputs)

[philipstarkey]

Original comment by Chris Billington (Bitbucket: cbillington, GitHub: chrisjbillington).

---

New comment instead of a confusing edit (issue threads don't support nested replies!):

I don't think there are any problems with repeat_pulse_sequence other than those that apply to analog ramps already. It's true that intersecting ramps etc will move the points that the function will be evaluated - which will shift the edges of the pulse train, but that doesn't cause problems if you have a high enough sample rate. The sample rate should be picked based on how accurate you want the edges to be, not on what the period of your pulses actually are. If you really want a low sample rate though, and precise edges, then yes, there's a need for regularly spaced clock ticks that won't be moved by an intersecting ramp, which is related to the "ramp-splitting" pull request and not what labscript currently tries to do. (This problem "exists" for analog outputs in the sense that, if you have a carefully crafted analog function that makes assumptions about exactly what timepoints it will be evaluated at, you're going to have a bad time)

[philipstarkey]

Original comment by Chris Billington (Bitbucket: cbillington, GitHub: chrisjbillington).

---

+1 to tooltips.

[philipstarkey]

Original comment by Ian B. Spielman (Bitbucket: Ian Spielman).

---

Chris's proposal to move all of the ramps and value setting into an ancestor class that both Digital and Analog output classes makes really good sense to me. It makes the language more logical and consistent, the current Digital-only function calls could be retained, but they would just call the associated generic function (which would be the prefered syntax going forward?)

[philipstarkey]

Original comment by Jan Werkmann (Bitbucket: PhyNerd, GitHub: PhyNerd).

---

I like the idea of tooltips and a different color for outputs not displaying what the hardware does. I also think BLACS should display what the hardware does. But I would argue that inverted shutters are even then currently misrepresented as they are inconsistent with non inverted shutters. At least if you consider the Shutter to be the hardware rather than the output.

Unifying wasn't something I was aiming at but still sounds great and as already mentioned would allow for the inversion via unit conversion.

As for totally getting rid of go_high() go_low() I'm not a fan of, as I don't want to rewrite all our scripts. But I would be very okay with them in the background just calling do_line.constant(t, 1).

[philipstarkey]

Original comment by Chris Billington (Bitbucket: cbillington, GitHub: chrisjbillington).

---

My suggestion was that digital outs set to use "units" other than "hardware units" would be a different colour. This would mean that a non-inverted shutter would still have a different colour, if it was set to use "shutter state" units rather than "hardware state" units - even though it was non inverted and and the same as the hardware state.

I'm also in favour of keeping lots of syntactic sugar around, but making the underlying implementation as clean as possible. Not just for backward compatibility - I think it provides good hints for quickly understanding what oft-read code is doing, and is good for top-level, not-very-nested code that the user is currently writing. Code that is read less often though, and that other code is relying on to behave well, should try to be more "kosher".

[philipstarkey]

Original comment by Jan Werkmann (Bitbucket: PhyNerd, GitHub: PhyNerd).

---

As this came up in our lab again today.

Until unification of the output classes or virtual devices are here, I suggest for a quick fix adding inverted to the device properties of the DigitalOutput as this is a minimal change. This would then be added as a kwarg to the constructor of DO. The standart would be for it to be False so no changes occur by accident. If inverted is True DO.set_value() and DO.value() would behave in a inverted fashion. Also I would add a tooltip to the widget that says it's inverted.

Everyone would then be free to add the functionality to their labscript_devices in a away they please or not.

If this should not be satisfactory I'd also be ok with it and just implement it in our lab and replace it when one of the better solutions comes along.

[philipstarkey]

Original comment by Jan Werkmann (Bitbucket: PhyNerd, GitHub: PhyNerd).

---

• changed state from "new" to "resolved"

implemented in ede02b8d06fd0bf5efc9a94d603ed2987a13c04f