Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use events to couple parameters in instrument drivers? #92

Open
alexcjohnson opened this issue Apr 20, 2016 · 10 comments
Open

Use events to couple parameters in instrument drivers? #92

alexcjohnson opened this issue Apr 20, 2016 · 10 comments
Assignees

Comments

@alexcjohnson
Copy link
Contributor

@MerlinSmiles noticed an interesting case while writing an instrument driver: sometimes different parameters are coupled to each other, such that changing one affects the value of the other(s). This means that the snapshot of this instrument will definitely be incorrect right after a set call on one of these parameters.

Sometimes this is just two different ways to call the same hardware command, eg resolution and averaging time... sometimes you want to specify one, sometimes the other, but they both generate identical hardware calls. In that case, we could probably work out a way to determine the value of the other parameter programmatically, but it would be easiest to just ask the instrument.

Other times they are separate parameters that don't have a 1:1 mapping but they impose constraints on each other. In that case, unless we want to figure out the details of these constraints and hard code them into the driver (which sounds horrible!) you need to ask the instrument for the other parameter(s).

It occurred to me the easiest way to build in such couplings would be through events. In this case we would do something like:

def update_res_and_time(self):
    self.resolution.get()
    self.integration_time.get()
    self.NPLC.get()

def __init__(self, ...):
    self.add_parameter('resolution', ...)
    self.add_parameter('integration_time', ...)
    self.add_parameter('NPLC', ...)

    self.on(('resolution_set', 'integration_time_set', 'NPLC_set'), self.update_res_and_time)

Where the parameter would know, because it's attached to this instrument, to emit a '<name>_set' event on setting (and maybe a '<name>_get' event on getting too?) on this instrument's event emitter.

But I haven't used any event systems in Python... anyone? I see pyee, gevent-eventemitter, eventemitter...

Is this the right way to do this or is there something better? Do we need such events to extend beyond the walls of the instrument (which in practice would also mean extending beyond this PROCESS, which would make it quite a bit more involved)? pyee looks nice and lightweight, people seem to like it, so that's what I would go with all else equal.

@MerlinSmiles
Copy link
Contributor

MerlinSmiles commented Apr 20, 2016

Didnt mean to close this, sorry

@spauka
Copy link
Contributor

spauka commented May 3, 2016

Given that all that needs to happen is updating the values of parameters in a snapshot, rather than having to define a new function/events for each set of coupled parameters, could we make it a part of the parameter class?

In addition to the complication of coupled parameters there are those parameters that are only available when another is enabled. Looking at the Keysight 33410A, which is a DMM with a similar set of commands, NPLC and integration time are mutually exclusive parameters controlled by a switch (i.e. you can use NPLC, or an integration time, but only one of these values is valid at a time). We should then include a way of disabling various parameters as well.

I'm imagining a syntax that looks something like:

def __init__(self, ...)
    self.add_parameter('resolution', ...)
    self.add_parameter('integ_enabled', ...)
    self.add_parameter('integration_time', ...)
    self.add_parameter('NPLC', ...)

    self.resolution.depends_on((self.integration_time, self.NPLC))
    self.integration_time.enabled_if(self.integ_enabled, True)
    self.NPLC.enabled_if(self.integ_enabled, False)

A similar issue exists for parameters that may disable others, such as averaging, or whether or not a second source is enabled on a PNA etc. For example, looking at averages, we could have something like:

def __init__(self, ...)
    self.add_parameter('averaging', ...)
    self.add_parameter('average_count', ...)
    self.add_parameter('average_type', ...) # Could be 'sweep' or 'point'

    self.average_count.enabled_if(self.averaging, True)
    self.average_type.enabled_if(self.averaging, True)

Looking at metadata and snapshots #107, this also allows us to disable parameters from being saved in a snapshot if they are not relevant.

@giulioungaretti
Copy link
Contributor

@spauka @MerlinSmiles not sure I can follow this conversation, Alex link is broken.
Could you summarise // point out if it's still relevant ?

@spauka
Copy link
Contributor

spauka commented May 8, 2017

The issue that is being referred to is still outstanding, in that as far as I can tell there is still no way of creating coupled parameters, though the solution referred to in the title, "Use events ...", may no longer be the best solution.

In short, the issue is that some instruments expose certain parameters only sometimes, or that the values/units of other parameters change depending on the selection of a parameter. For example, looking at the SR830 driver, when we change the value of input_config, the units of X,Y and sensitivity can change from voltage(V) to current(I). The way that this is currently handled is that the get/set parser of input_config is overriden to change the value of the units, however arguably this is not the role of the get/set parser function. Other times, certain parameters only take a value if another parameter is enabled, see my previous comment re. averages. I would say these are the two issues noted in Alex's link.

Don't have a strong opinion on the best solution, but moving to a more logical way of handling these parameters should be addressed.

@giulioungaretti
Copy link
Contributor

@spauka yes agree! @WilliamHPNielsen had a lot of the same issues writing drivers (correct?)

@WilliamHPNielsen
Copy link
Contributor

@giulioungaretti Yes, that is correct, we had that a lot with the ZI UHF-LI driver. If you want to convince yourself that Alex and Sebastian are right that hardcoding this stuff into set and get is not the proper way to go about it, have a look at this highly specialised set function: https://github.com/QCoDeS/Qcodes/blob/master/qcodes/instrument_drivers/ZI/ZIUHFLI.py#L1467.

I fully agree that we should solve this. Although the SCPI standard encourages tree-like disparate parameters, most instruments still have them mingled together.

@giulioungaretti
Copy link
Contributor

@WilliamHPNielsen @spauka yeah, suspected so just wanted to make sure.

@nulinspiratie
Copy link
Contributor

Solving coupled parameters with events sounds like a good idea to me. We use the package blinker for events in our qcodes extension, and it works quite nicely. It's quite fast, and allows both named and anonymous signals. A big advantage of blinker is that it allows to subscribe to specific senders.

As for implementation, one possibility is to attach an event object to each parameter. Simple get/set signals can then be hardcoded into the get/set parameters, for instance in decorators suggested in #600. @WilliamHPNielsen signals can also be used for more complicated cases such as the one you mentioned, by sending signals that don't have get or set as a kwarg, but other specific ones. Would this be a viable solution?

@giulioungaretti
Copy link
Contributor

@nulinspiratie does it work across processes?

@nulinspiratie
Copy link
Contributor

@giulioungaretti No it doesn't, I'm not aware of any event handler that also transfers signals across processes. A wrapper could probably be programmed around the package that could handle this, but I'm not sure how much overhead this would give

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

No branches or pull requests

6 participants