Node-tree based parameter structure in instruments

@jenshnielsen @WilliamHPNielsen As discussed with @alan-geller and @sohailc . Any work on this requires #600 to be merged (@nulinspiratie) . Can be seen as a generalization of channel parameters (#640 #641 ) @spauka . Relevant for future DCL QuTech drivers @nyxus .

I would like to propose supporting a node/tree based structure for parameters in an instrument. This is motivated by the need to group certain parameters on a more generic level than channels.

Motivation

A concrete example would an instrument like the AWG5014 that has a parameter ch1_m1_high (channel 1 marker 1 high). It would be convenient to group parameters in a node structure. This would mean that instead of using underscores in the name of the parameter the instrument would have nodes with attributes, so ch1_m1_high would become ch1.m1.high.

Such a grouping of parameters would have several advantages.

Autocomplete of large instruments would be more managable and would immediately show the relevant groups of parameters to a user.
Possible to natively match structure that exists in many instruments (see e.g., colon separated commands in VISA protocols and the Zurich Instruments python API).
Possible to add documentation to nodes or groups of parameters.
Provides structure to datafiles (where snapshots are saved).
Provides structure that can be used in instrument monitoring capabilities.
I think this is simpler to understand than the Channel parameter (that does not work as per comment in #600 ). This argument requires reading the section below.

Proposed implementation/required changes

To make the node based parameters work changes are required to the following methods

add_parameter
snapshot
print_readable_snapshot
datasaving/loading -> especially urgent as that is currently being redesigned.
A new parameter_node object.
Instrument.get and Instrument.set

I will now go over these points individually.

add_parameter

The Instrument.add_parameter method would need to support adding parameters to nodes in a simple way. I would propose only changing the way the name argument of the add_parameter method works. The name would be split based on the period . character (as conventional in python), the last part of the name would be the name of the parameter. Anything before that corresponds to a node. The example of pseudo code below shows how it should work

ins.add_parameter(name='ch1.m1.high', **kwargs) 
 -> separate string based on period "." character

 -> check if ch1 exists as a parameter in the instrument. 
           -> if ch1 does not exist, create a "node" 
           -> if ch1 is a regular parameter raise exception 
           -> if ch1 is a node parameter continue 
           -> recursively call this "add_node" part until you get to the `parameter` "high", there add the parameter as it is done now.

snapshot

Support a nested structure for the dictionary that contains the parameter nodes with parameters in them.

print_readable_snapshot

Preserve the nesting in a readable way. To give an example I give below (part of) a snapshot of our VectorSwitchMatrix and how it could look using a nested structure.

VSM:
    parameter      value
--------------------------------------------------------------------------------
IDN             :   {'driver': "<class 'pycqed.instrument_drivers.physical_inst...
in1_out1_att    :   14
in1_out1_phase  :   0
in1_out1_switch :   EXT
in1_out2_att    :   11
in1_out2_phase  :   90
in1_out2_switch :   EXT
in2_out1_att    :   11
in2_out1_phase  :   0
... # cut short for example
mode            :   raw

Example using the node-tree structure (I couldn't get the alignment to work on github but you get the idea).

VSM:
    parameter      value
--------------------------------------------------------------------------------
IDN                  :  {'driver': "<class 'pycqed.instrument_drivers.physical_inst...
in1
    out1
         att         :  14
         phase      :   0
         switch     :   EXT
    out2
          att       :   11
          phase     :   90
          switch    :   EXT
in2
    out1
        att          :  11
        phase       :   0
... # cut short for example
mode                 :  raw

Note that the indents used here to indicate the groups are also natural folding levels in any kind of interactive monitor.

datasaving/loading

Datasaving should already support saving of snapshots as a JSON like dict (or in hdf5 as we are using). Extending this to nested dicts should not be a problem. Ofcourse this should also be taken into consideration for the future database formats but I don't see fundamental problems here.

ParameterNode object

A new node class has to be made that can be added to an instrument as a parameter but can contain parameters itself. This object should not be very difficult to make but should support the following methods.

snapshot -> returning the snapshot of the nodes and parameters it contains (this ensures that the higher level snapshots remain working).
get/set -> see below for how instrument get/set should work. I'm not sure if a node is more like an instrument (something that can contain parameters) or like a parameter (it can be get/set and is added to an instrument).

Instrument get/set

The get/set methods should support nested extraction of values. To give an example. VSM.get('in1.out1.att) should return the nested value.

Fancy extensions to get/set

Above I have described the basic idea and functionality that I would like. When working this out it is only natural to think of how the get/set could be made more powerful. I consider this beyond the scope of an initial implementation but as we ( @alan-geller , @sohailc ) discussed them and they could be quite powerful I think it is worth mentioning.

allow asterix * character in get/set commands, setting a value to all parameters that match the description e.g., VSM.set('in*.out*.switch, 'OFF') to set all switches to off. Similarly use it to get all values that match the description. I would say that the return format should then be a dict containing the full parameter names (including parent nodes) as keys and the return values as values.
Allow setting using a list or dict of values. An example would be `VSM.set('in1.out*.att', [0, 2]) in which case the 0 and 2 values are mapped to out1 and out2 respectively (sorted alphabetically). Note that it is important ot use a list type here as the parameters are not guaranteed to be single valued.

You are aware that we already have channels that can be nested arbitrarily deep? They are already used in several instruments

Sorry the above is bit short I’m away from my computer with only a phone but you can have a look at the qdac, keysight waveform generator, or keithley source meter drivers as examples. Any feedback on what that interface is missing is very wellcome

@jenshnielsen as far as I can judge it as a newbie in qcodes is this node feature by @AdriaanRol complementary to the existing channels in the sense that while channels are great for homogeneous parameter structures, nodes are great for inhomogeneous parameters. By this I mean that a channel offers a lot of comfort for e.g. an oscilloscope with four channels that all behave in the same way. For the implementation of an instrument with "channels" that share no structure one would have to chose between creating specialized channel classes or to have all the parameters living at the root level. I think the nodes concept could offer a great third option, that allows for the structuring of the parameters without the need for creating all the nested channel objects.

Hey, so I think some of what you are proposing to add is possible, though with a slightly different implementation. What I implemented for channels was this idea of submodules, which were just snapshottable (Metadatable) objects that could be referenced as part of an instrument. ChannelLists fit this definition. Since they only need to be snapshottable, they can be used to implement this nested structure.

As an example of what is currently possible, a skeleton for an SMU with source and measure broken out would look like:

class SourceUnit(InstrumentChannel):
   def __init__(self, parent, name):
      super().__init__(parent, name)
      self.add_parameter("voltage",...)
      self.add_parameter("range",...)
      ...

class MeasureUnit(InstrumentChannel):
   def __init__(self, parent, name):
      super().__init__(parent, name)
      self.add_parameter("voltage",...)
      self.add_parameter("resistance",...)
      ...

class SourceMeasureUnit(VisaInstrument):
   def __init__(self, name, address, **kwargs):
      super().__init__(name, address, **kwargs)
      source = SourceUnit(self, "source")
      measure = MeasureUnit(self, "measure")
      self.add_submodule('source', source)
      self.add_submodule('measure', measure)

which can be accessed like:

>>> smu = SourceMeasureUnit("smu",...)
>>> smu.source.voltage.set(1)
>>> smu.measure.voltage.get()
1.001

As I read it this is how a ParameterNode might work? It may make sense to only do things this way, rather than extend add_parameter, just as a way of tidying up the __init__ function and enforcing logical groupings of parameters in code.

For an example in the wild, I've got an extended Yoko GS200 driver which I have been meaning to pull in here: https://github.com/spauka/Qcodes/blob/device-dev/qcodes/instrument_drivers/yokogawa/GS200.py

I'm definitely in favour of a way of grouping together parameters, and the ParameterNode looks like a good implementation. Also when combining this with monitoring tools etc, it will be very useful to have a the parameters structured, instead of a giant list.

The functionality of Channel/MultiChannelParameter/ChannelList seems to have a lot of overlap with the proposed ParameterNode, they both seem to be a container for parameters. In this sense, I like the name ParameterNode better, especially since parameters may want to be grouped in a way unrelated to a channel. At the moment I can't think of a situation where a ParameterNode could not be used instead. @Dominik-Vogel You mentioned channels being more suitable as homogeneous parameters, could you give an example of it's advantage over a ParameterNode?

@AdriaanRol enhancing the get/set functionality of instruments also sounds good. I'm thinking this could also be a feature of the ParameterNode. In this case, the ParameterNode actually also has a lot of overlap with the CombinedParameter (which is currently in need of improvements). I'm wondering if the ParameterNode would be able to take over its functionality entirely. The asterisk symbol could then act as a flattening of a ParameterNode, returning a flattened ParameterNode. For example, say we have:

ch1 (ParameterNode):
    in1 (ParameterNode):
        trigger_threshold (Parameter)
    in2 (ParameterNode):
        trigger_threshold (Parameter)

By then specifying ch1['in*] (or via .get or some other method), we would get a flattened ParameterNode:

ch1_flattened (ParameterNode):
    in1_trigger_threshold (Parameter)
    in2_trigger_threshold (Parameter)

We could then get/set its values as we would for a CombinedParameter (ch1['in*'].set([1, 1.5]))

I'm not sure if defining these nodes implicitly via add_parameter is the best solution, it might be worthwhile to explicitly create nodes. In this case, it would be possible later on to remove the add_parameter, and instead directly set parameter attributes (e.g. instr.node.param_name = Parameter())

@spauka @jenshnielsen it does indeed appear that the InstrumentChannel already behaves mostly like I would intend a parameter node to behave. Especially the example by @spauka was very helpful. Playing around with that example and looking at the code for channels did raise a whole bunch of other questions though. I'll first show the example on how I would like it to work, then I'll address the changes that would be needed and last I will ask some questions on the way channels work.

Examples of intended behaviour

I have put together 3 examples of code that (should) function idential. The first two already do, the third would contain the two lines of example two merged into the add_parameter.

rewrote example by @spauka to be self contained.

class SourceUnit(InstrumentChannel):
    def __init__(self, parent, name):
        super().__init__(parent, name)
        self.add_parameter("voltage", parameter_class=ManualParameter, unit='V')
        self.add_parameter("range", parameter_class=ManualParameter, unit='V')

class MeasureUnit(InstrumentChannel):
    def __init__(self, parent, name):
        super().__init__(parent, name)
        self.add_parameter("voltage", parameter_class=ManualParameter, unit='V')
        self.add_parameter("resistance", parameter_class=ManualParameter, unit='Ohm')

class SourceMeasureUnit(Instrument):
     def __init__(self, name, **kwargs):
        super().__init__(name, **kwargs)
        source = SourceUnit(self, "source")
        measure = MeasureUnit(self, "measure")
        self.add_submodule('source', source)
        self.add_submodule('measure', measure)

Same as above but without extra classes

class SourceMeasureUnit_no_extra_classes(Instrument):
    def __init__(self, name, **kwargs):
        super().__init__(name, **kwargs)
        # I propose these two lines be merged into the add_parameter when there is nodes 
        source = InstrumentChannel(self, "source")
        self.add_submodule('source', source)
        source.add_parameter("voltage", parameter_class=ManualParameter, unit='V')
        source.add_parameter("range", parameter_class=ManualParameter, unit='V')

        measure = InstrumentChannel(self, "measure")
        self.add_submodule('measure', measure)

        measure.add_parameter("voltage", parameter_class=ManualParameter, unit='V')
        measure.add_parameter("resistance", parameter_class=ManualParameter, unit='Ohm')

Proposed equivalent

class SourceMeasureUnit_intended_nesting(Instrument):
    def __init__(self, name, **kwargs):
        super().__init__(name, **kwargs)
        # here the source-node parameters get added 
        self.add_parameter("source.voltage", parameter_class=ManualParameter, unit='V')
        self.add_parameter("source.range", parameter_class=ManualParameter, unit='V')
        # here the measure-node parameters get added 
        self.add_parameter("measure.voltage", parameter_class=ManualParameter, unit='V')
        self.add_parameter("measure.resistance", parameter_class=ManualParameter, unit='Ohm')

Snippet I use to test the channels

# instantiating the instrument 
mySMU = SourceMeasureUnit('mySMU')

# Setting some random values for the example
mySMU.measure.resistance(50)
mySMU.measure.voltage(1.2)

mySMU.source.voltage(1.03)
mySMU.source.range(2)

# her I would like to also list the Nodes/Submodules/InstrumentChannels
mySMU.parameters                             # currently only returns the IDN parameter
mySMU.print_readable_snapshot()     # shows that the snapshot works but is not optimal yet. 

mySMU.get('source.range')    # should return "2" but not implemented yet 
mySMU.set('source.range', 5) # similar as above but also not implemented yet

Required changes.

It seems that the InstrumentChannel is almost exactly what a ParameterNode would be so that is good news 👍 . However there are several things that would need to be changed in the way it behaves.

Rename the InstrumentChannel to ParameterNode as I think the name is more general
Add submodules to the list of parameters so that Instr.parameters also lists the nodes (in fact I would propose to get rid of the distinction completely)
Support fancier get/set methods (as in example above)
Update snapshot and readable snapshot (mmh while writing this it seems most of this has already been written in my initial post).

Questions on channels

I know understand what an InstrumentChannel is, what is the difference between a submodule and an InstrumentChannel? What are the intended use cases of all the other classes in the Channels module? (MultiChannelInstrumentParameter and ChannelList)?

@nulinspiratie , I was writing while you posted your last update. The description you give on the fancier get/set is kind of what I'm trying to get at. In my understanding the ParameterNode is a concept that could indeed, as you describe, describe all the use cases that are now covered with the variety of Channel MultiChannelParameter and ChannelList. Additionally I am of the opinion that it is a good thing if I can kill of 3 concepts and replace them by 1 😉 .

I am a big proponent of the add_parameter method though mostly as this does a bit more than just Instr.parname = par. It also checks for existing definitions and adds them to the list of Instr.parameters which in turn is used in the snapshotting. It also helps a lot with code generated drivers (e.g., for ch in range(10): add_parameter('base_name{}'.format(ch), **kw)).

A reason I am a proponent of hiding the node creation in the add parameter method is twofold. First of all I think of it a bit like creating files in a folder struture. I like my makefile/open command to create the higher level folders if that is required for convenience reasons. Secondly I like to create drivers based on JSON files or manuals. They contain a dictionary of parameter names (which we can create using . separators. It is then very convenient to create the required nodes and only complain if an identical parameter already exists.

@AdriaanRol all of the extra features can be handled quite easily using the Instrument.__setattr__:

def __setattr__(self, attr, val):
    if isinstance(val, Parameter):
        if hasattr(self, attr):
            raise Exception()
        else:
            self.parameters[attr] = val
    super().__setattr__(attr, val)

I think in this case setting the parameter would be explicit: instr.param_name=Parameter(). I think it makes it clearer, since you don't need to pass the parameter class as a kwarg. Also, your example could be replaced by for ch in range(10): setattr(instr, base_name{}'.format(ch), Parameter())

As for implicit or explicit node creation, it doesn't matter too much except that implicit node creation doesn't work well with setattr. Also, implicit node creation doesn't allow to use a subclassed node. This would still have to be done explicitly. That being said, I do see the appeal in creating implicit nodes

@nulinspiratie I was basically thinking of the differences in the intended uses between nodes and channels. Where channels are currently typically used through subclassing, nodes were intended to be created by adding parameters with an extended add_parameter method. Adding, what I called "homogeneous" channels/nodes, can of course also be achieved by looping over a group of add_parameter calls and is only a question of personal preference. There are some drivers that have channels with parameter-getters/setters that rely on channel attributes (e.g. ZNB, Decadac)

@AdriaanRol I was also wondering about the intended use of add_submodule. In the main code base it is exclusively used for adding channels. Possibly this can be added to the list of concepts that can be replaced by a ParameterNode?

@Dominik-Vogel perhaps it would be useful to explain my reasoning behind using add_submodule as opposed to extending add_parameter. Given that multi-threading has been taken out of qcodes for now, the only reason that add_parameter/add_function even needs to be used (as opposed to something like self.param = Parameter("bla", ...)) was to ensure snapshotting works. However the way that objects are accessed from the parameters dictionary meant that it wasn't really logical to store more general objects in this dictionary. The submodules dictionary was defined to store ANY instance of a Metadatable object.

It's used in a couple of places for more than just channels, see the yokogawa driver linked above for a more general usage example. Or, in the Lakeshore.Model_336 driver, add_submodule is used to allow access to the temperature channels either using the channel list OR using the channel letters, i.e. the following two methods of accessing the channel are equivalent:

temp = Model_336("temp", "GPIB_ADDRESS")
temp.channels[0].temperature.get()
temp.A.temperature.get()

@nulinspiratie Renaming InstrumentChannel to ParameterNode makes sense I think? There's nothing in there that makes it uniquely channel-like...

@AdriaanRol Actually, rather interestingly, there seems to be a conflict in the way we both access parameters, which may decide how we move forward. We normally pull parameters out of Instruments and call get/set on these. You seem to call get/set on the instrument with a parameter name as an argument? The ChannelList/InstrumentChannel divide was to support the former method of access, and we can write down a mapping between all your examples, and how I would access them currently:

Parameter Based Access	Instrument Based Access
`mySMU.source.range.set(5)`	`mySMU.set('source.range', 5)`
`VSM.in[0].out[0].att.get()`	`VSM.get('in1.out1.att)`
`VSM.in.out.switch.set('OFF')`	`VSM.set('in.out.switch, 'OFF')`
`VSM.in[0].out[0:2].set([0, 2])`	`VSM.set('in1.out*.att', [0, 2])`
`VSM.in[0].out[2:4].set([0, 2])`	`?`

To answer @AdriaanRol's question then:

InstrumentChannel is an instrument-like object that points all ask/write requests to the parent instrument.
- ChannelList is a list-like object that implements slicing logic for channel lists (i.e. makes dac.channels[0:20] possible)
- MultiChannelInstrumentParamter is a parameter-like object that contains multiple instrument channels (i.e. the result of dac.channels[0:20].voltage is a MultiChannelInstrumentParamter pointing to the voltage parameter of the first 20 dac channels. It is like a CombinedParameter that points to the first 20 channels of the dac.

My goal with this was to get rid of strings as a method of getting access to channels, and to allow list-like access to channels. In addition, to allow a more structured definition of large instrument drivers by breaking out sections as logical. Code generated parameters are still very much possible (see Decadac Driver), although this may require more work for JSON code generation.

@spauka Thank you for explaining the intention behind the submodules. I didn't want to evoke the impression that we can get rid of it. I only meant to say that it is so far only used in the context that was at discussion, which are Channels and ChannelLists, which includes your examples of the Yokogawa(Channel) and 336 (ChannelList).

@spauka , thanks for explaining that. I'll just quickly write my thoughts below, a more in depth reply will follow later.

Given that multi-threading has been taken out of qcodes for now, the only reason that add_parameter/add_function even needs to be used (as opposed to something like self.param = Parameter("bla", ...)) was to ensure snapshotting works.

Agree, it also provides a neat hook in if we ever want to reintroduce such functionality. I also think it has minor advantages in readability for code generated parameter names if only because string formatting an attribute name can be annoying.

However the way that objects are accessed from the parameters dictionary meant that it wasn't really logical to store more general objects in this dictionary. The submodules dictionary was defined to store ANY instance of a Metadatable object.

I would like to get rid of the distinction and allow the parameters dictionary to allow for storing general snapshotable object in general. I think getting rid is advantageous because one has less concepts to worry about and the user can quickly get an overview of all the relevant "parameter" he/she can interact with.

either using the channel list OR using the channel letters

This would be kind of what I am going for, what I am thinking is that the different ways of accessing should not be done by using multiple parameter classes but rather by improving the way the get and set commands work, including potential slicing access to nodes. Adding those kind of features would be a second step after implementing the basic nodes.

@nulinspiratie Renaming InstrumentChannel to ParameterNode makes sense I think? There's nothing in there that makes it uniquely channel-like...

This would make me very happy :+1:

@AdriaanRol Actually, rather interestingly, there seems to be a conflict in the way we both access parameters, which may decide how we move forward. We normally pull parameters out of Instruments and call get/set on these. You seem to call get/set on the instrument with a parameter name as an argument?

We actually use both. I generally prefer the attribute based access as it has nice autocomplete features and is quite readable. However, when code generating loops over specific parameters (or using some parameter to designate the name of another parameter I want to access) I prefer the instr.get('parname') syntax. I think the second method is very explicit and flexibile (using string formatters).

The only real conflict lies in the following example

VSM.in[0].out[2:4].set([0, 2]) = ?

This would be easily addressed if we allow regular expressions, which I personally detest but are a widespread and powerful standard.

My goal with this was to get rid of strings as a method of getting access to channels, and to allow list-like access to channels. In addition, to allow a more structured definition of large instrument drivers by breaking out sections as logical.

I think then our goals are very much aligned. To these goals I would like to add that I want to have a way providing this structure to more general objects within a driver while simultaneously standardizing the way these are accessed (i.e., getting rid of the different classes that achieve this).

All sounds great 😄 As long as parameter based access is kept in mind, I think it's definitely a feature worth adding.

microsoft / Qcodes