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

Reading measurement result from an instrument : property vs method #9

Open
MatthieuDartiailh opened this issue Feb 18, 2015 · 37 comments

Comments

@MatthieuDartiailh
Copy link

The question is the following :
To retrieve the value of a physical quantity measured by an instrument which changes spontaneously (let's say the voltage of a multimeter), should we define :

  • a property (Feature)
  • a method (Action)

This is actually related to the problem of caching the Feature value. As if we use a Feature, we must either not cache the value or even if a cache exists query the value.
I am in favor of always returning the cache value if one exists, so we should either tell the Feature not to use the cache (@hgrecco is against this option as he thinks it will be confusing if not all Feature behave the same way with respect to caching), or always use a method and in such a case we may need some automation in writing standard methods.
@hgrecco would prefer to use the cache to avoid useless set and always perform get. This means that checking the instrument state will be an expensive operation even if it is known which is why I am not in favor of this solution.

@hgrecco
Copy link

hgrecco commented Feb 18, 2015

Maybe is good to look it from the user side:

Let's say we have a multimeter and we want to measure the voltage.
Using a Feature

>>> print(inst.voltage)

or using and Action

>>> print(inst.measure_voltage())

(The names are, of course debatable, lets focus on the Feature/Action dichotomy)

If we go for the Feature option, then the getter either:

  1. always return the actual value.
  2. return the actual or cached value depending on a flag in the Feature definition.

I found the second option confusing because in some cases (e.g. for a voltmeter) it will return the actual value while in others (e.g. a power supply) it will return the cached value.

So in my opinion is we should either using an Action or a Feature returning always the actual value.

But in any case, before deciding it might be good to put a few examples (from the user perspective):

  • The frequency of a lockin amplifier which can be get/set using the internal oscillator or just get if you are using an external reference.
  • The frequency of function generator which can operate using the internal oscillator or act as a slave of another.
  • other?

@MatthieuDartiailh
Copy link
Author

Your last examples should indeed make us thing seriously about this issue. As you mention trying to look from the user side, I think we can expect at least two typical kind of use :

  • people wanting to get a kind of distant control panel to control their experiment using their computer, but who will still sometimes directly interact with their instrument. Those ones are not concerned about speed and could just de-activate caching.
  • people automating an acquisition protocol during the user is not meant to play with the instruments. This one is interested in performances and want to take advantage of the cache. The question is does he trust 'us' enough to handle the cache correctly so that no matter whether the value is cached or not he gets the right answer to its query (as long as he does not mess up anything on purpose, and if at some point he suspect he did so he can always clear the cache). If he does (and I think we should work as hard as we need so that he is right to do so), then we can cache some values, not some others and even have dynamic authorization to use caching (for a lock-in oscillator the internal oscillator is used -> use cache, the external ref is used -> don't).

Actually I would prefer to go that way as otherwise we will end up with action for the lock-in oscillator frequency just because sometimes it is meaningless to use the cache. But it still think that for measured values (which are always read-only) we should use Action.

There is actually another kind of dirty solution which would be to use a general method to either force the getting operation (@alexforencich already suggested we might want to have a forced set) or get the cache if one exists. But I think this will be even more confusing for the users as some will always use the method and not the generic getter and this will looks weird.

@alexforencich
Copy link
Member

Something like a power supply will have a voltage feature to set the voltage and a measure_voltage action to read the actual voltage (e.g. when in current limit, output disabled, etc.). Generally I would say that features are for things that do not generally change on their own. I'm not sure what the best setup for a lock-in amplifier would be, then. When running on an external reference, does the lock-in amplifier have a built-in frequency counter to measure the frequency of the reference?

@MatthieuDartiailh
Copy link
Author

I just checked the docs of one lock-in we have. A single command can either return the internal oscillator frequency or the external reference frequency or 0 if the lock in is unlocked. This is non trivial to handle.

@alexforencich
Copy link
Member

Wow. That's incredibly annoying. However, I wonder if a 'better' solution would be to have two different items that end up calling the same instrument command - one feature for setting the internal frequency and one action for reading the frequency. Then just make sure that it's documented that reading the feature won't work if the external reference is selected.

@MatthieuDartiailh
Copy link
Author

@alexforencich I guess you don't like either the idea of messing up at runtime with how is handled the cache. I do agree we could have both feature and action in this case but I fear this might end being a bit confusing for the user (as their is a single frequency for the oscillator and they don't care where it comes from).
However I do agree about the power supply as there is truly a notion of required value vs actual value.

@alexforencich
Copy link
Member

Well, perhaps a better question is how much difference does it make if that particular value is simply never cached? I think we're splitting hairs here with some of these corner cases. A few extra reads when running on the internal oscillator probably isn't that big of a deal. However, another thing to consider is how this is implemented on other devices. Is it an industry standard to have one command to read the internal oscillator setting AND the measured reference frequency? If some units have separate commands, then I say that is a very good argument to have a property and an action. It may make more sense to do it that way anyway as in one instrument state that command actually returns a measurement and not simply a set point. I actually don't think it would be all that confusing for the user, just write to .frequency when setting the internal oscillator frequency and then use .measure_frequency() when you want to know what the frequency is, regardless of which source is active. It's just like the power supply, except it happens to use the result from the same command.

@MatthieuDartiailh
Copy link
Author

I think you do have a point about using feature and action because we might ends up with different commands. We can kind of make it a standard for how we will write driver and if the users always see the same way of doing things they will get used to it.

@hgrecco
Copy link

hgrecco commented Feb 20, 2015

@alexforencich I do not think it is confusing in your example. I do think is confusing if the same name is either cached or not depending on the instrument.

So I think that we agree that the situation is not so trivial and we need to think from the user side.

As I mentioned before, Lantz currently works in the following way:

  • get operations always return the current value
  • set operations are performed only if the new value is different from the cache

(obviously get/set updates the cache)

Optionally you can:

  • return the cached value (a method named recall)
  • set operations can be forced (a method named update)

The details are here
(By the way, recall and update are very useful when dumping the current state to disk and restoring it later as they operate with kwargs. But thats another discussion)

The rationale is that in many instruments setting might be a more expensive operation than getting. For example, setting might involve moving a physical device and it needs to check first where it is.

We also considered our experience building automation programs with plugins. In this case the final program is the execution of multiple plugins but they do not know about each other. Each of these plugins measures something different but they have to be sure that the equipment is configured in certain way. So you need to be sure you are getting the actual value with get, but setting can rely in the cache value. I can give you a more detailed example if unclear.

Another option that we took was to provide a getter method even for write only quantities (i.e. the voltage of a power supply). In this case we return the cached value. The rationale here is making it simple for the user.

We also took a direction similar to what Alex proposed. frequency queries the current frequency (either from the internal or external, depends on what is active) and sets the internal.

@MatthieuDartiailh
Copy link
Author

@hgrecco I do not understand how caching can be considered safe for set but not get in your plugin application. For me the only way to reach that safety is to have a single driver per instrument, which can be done by weak caching based on the connection informations. If you have a multi-process application, then either you de-activate the cache or you go through a central server in charge of the instrument.
I think @alexforencich was rather in favor of having feature (internal) and action (external) as he was concerned that those could in certain cases be queried through different commands.

@MatthieuDartiailh
Copy link
Author

@hgrecco would you mind elaborating on why you think having different caching behaviors for different featuree is an issue.

@hgrecco
Copy link

hgrecco commented Feb 21, 2015

The only True safe way is avoid caching. But caching is useful in many cases. For example I was able to reduce by half the duration of an experiment thanks to caching. And I would like to argue that using the cache to avoid unnecessary set operations is in general more useful and safe enough.

A typical experiment might go like this. The user turns on the equipment and he may or may not use the front panel to adjust different settings. The user then start the program that will get or set different quantities. These can be divided in two:

  • parameters: quantities that the user controls. These are changed by the program so the program always know the actual value and it can use the cached value to prevent unnecessary set (Except at the very beginning which is not a problem because as the cache is empty and the value is set anyway)

Querying the value of a parameter is not such a common operation.

  • measurements: quantities that the user does not control. These are changes by nature. You always want the actual value.

That is why I like getters that are always actual, and setters which use the cache to prevent unnecessary communication.

Regarding why I found it confusing. I do not want to go to the docs to know if a certain feature getter returns the actual value or the cached value.

@crazyfermions
Copy link

I totally agree with @hgrecco that it is very confusing if the default behaviour for Features is not well defined. I also agree fully that there are to types of quantities to be considered: Internal parameters, which are in full control of a single device, and external parameters, which change due to external changes (other devices, temperature, etc.). If you want to use Features for both of them and if you want to maintain the same caching behaviour for all of them, the reasonable way to go is probably the one hgrecco suggested. However, this leads to a very inefficient cache (still better than no cache, though).
First of all: Reading operations are expensive. Sending a command and receiving an answer is typically 50-100 ms, looking up a value in cache is probably ~1 µs.
Second: I disagree that setting operations are much more expensive than reading operations, since this assumes that a) the device does not have a cache on its own and b) that the setting operation is still expensive even if the device does not have to do anything. A combination of both is very unlikely.

I think the simple solution is to drop the first if, meaning: Do not use features for internal and external parameters alike!

Internal Parameters: Use a Feature, always use caching if it is activated.
External Parameters: Use an Action, never use caching.

To come back to the initial posts by @MatthieuDartiailh and hgrecco: Voltage of a multimeter should be an action. Voltage of a power supply -> a feature (hell, these are two completely different types of instruments, I think providing the same API for them is not reasonable at all).

How to solve the Lock-In Oscillator frequency problem:
Lock-In base class should consider an external oscillator -> Action (in fact, there are many lockins which do not have an internal osci). Then define an API extension for lock-ins with internal oscillator, use a (differently named) feature.

By the way, using the front panel should not be considered safe when it comes to caching. When the cache is used, lantz has to be greedy and assume that there is only one true god. ;)

@MatthieuDartiailh
Copy link
Author

@crazyfermions I fully agree with your proposal and I think that we are going back to what @alexforencich proposed some time ago. @hgrecco can you accept @crazyfermions proposal ?

@hgrecco
Copy link

hgrecco commented Feb 21, 2015

Great. We agree that the feature dependent cache on/off is a no go. I agree with @crazyfermions and I think that the proposed behaviour also solves the Feature/Action question that we have posed in the past.

But I would like you to consider one more thing. I am not saying that getters are cheap, I am saying that setters are in many cases more expensive. An most importantly, in most programs you rarely get internal parameters. Most of the times they are fetch once (to store somewhere the current state of the system) and then set to change them. So my proposal is asymmetrical between set and get, because programs are asymmetrical.

If this argument is not convincing you. I am +1 on Feature for internal, Action for external idea.

@MatthieuDartiailh
Copy link
Author

I am still not convinced because if we want to raise an early exception when a user tries to use a feature that cannot be used in the current operation mode of the instrument we need to check the instrument state.
My use case is a dc source that can operate as voltage or current source. I find find more natural to have a voltage Feature and a current Feature rather than simply having a magnitude one as it is more explicit. However in this case it is a good idea to check that the instrument is in the correct mode of operation before doing anything and I can't do a systematic set as it would be likely to destroy the user sample. And I think we can find more cases like this one.

@alexforencich
Copy link
Member

There are also a number of instruments that I have worked with a bit that have commands to set various parameters, but no way to read some of the settings. This is somewhat common for pre-SCPI test equipment. One of the worst offenders that I have seen is an HP power meter. There are also a number of similar issues on HP spectrum analyzers, but these are a bit more subtle - you can set some of the parameters (e.g. resolution bandwidth, video bandwidth, sweep time) to either an explicit value or AUTO and read out what the current value is, but I know of no way to see if a particular parameter is in AUTO mode or not. In AUTO mode, the values are determined automatically by the span and/or other manually selected values. Reading gives you the current value, but no indication if it is manually selected or automatically selected. The only recourse in these situations is to hope the cache is accurate if a getter is called. I'm not sure if there is a better way of handling this sort of issue.

@MatthieuDartiailh
Copy link
Author

Would the automatic invalidation of the cache you suggested when some values are set be sufficient to deal with this ? Lets say you change the sweep time, if the cache of all potentially related values is invalidated (video bandwidth, ....) we should be safe, no ? As given what I understand of your example if no order is sent and the user don't touch the front panel nothing will change (is that true ?).

@alexforencich
Copy link
Member

It's a little more subtle than that. For example resolution_bandwidth, resolution_bandwidth_auto, video_bandwidth, video_bandwidth_auto, etc. would be implemented as features. Setting resolution_bandwidth would invalidate video_bandwidth so that a subsequent read of video_bandwidth would return the new automatically selected value. This is not the issue. The problem is that there is no way to read resolution_bandwidth_auto from the instrument. The command rb sets the resolution bandwith, so rb 1000 sets the resolution bandwidth to 1 kHz and rb auto sets it to automatic. rb? reads the setting, but it will return 1000 whether the setting is in automatic mode or not (presuming auto mode decides to use a 1 kHz RBW).

@alexforencich
Copy link
Member

Basically, we need a decent method to handle poorly-implemented write-only properties.

@MatthieuDartiailh
Copy link
Author

If you cache the set operation of the resolution_bandwidth_auto and invalidate the cache if the user set resolution_bandwidth manually you get a pseudo getter. The issue is the initial case when the cache is empty because the user never set resolution_bandwidth_auto, perhaps we could have a three state Feature returning True, False, Unknown. But I do agree that this is not entirely satisfying.

@iromero91
Copy link

@alexforencich One way i've dealt with that is to have the initialize() method set all parameters into a known state, then track states internally of the write only properties. Of course it only takes a button press to take it out of sync, but you can always warn the lab personnel that finger loss may occur if they touch the instrument panel (Or some instruments have a keypad lock instruction in the protocol, which is handy).

@alexforencich
Copy link
Member

That's how it's done in python-ivi. Set everything to defaults on driver load, track updates, and cross your fingers. I'm just bringing it up as another implementation annoyance to see if anyone has any better ideas on how to deal with it. The main issue is that with this sort of 'write-only' property, you can't use an asymmetric cache where the get is not cached since an uncached get is impossible.

@iromero91
Copy link

I guess at the lowest level it would be better to be "honest" with the abstraction and implement write-only properties as such, and let more complicated state tracking work on a higher level, perhaps on an idealized abstract instrument class.

Say you have an experiment setup that needs a temperature source, a reference thermometer and 20 mA current meter to calibrate/characterize a temperature transmitter. I have examples in my lab where those instruments are either all contained in one single package or need 3 separate bits of equipment. Those "instruments" could be implemented as abstract classes that have all generic methods like measure_resistance() and set_temperature().

The concrete class' initialization method would do the dirty work of setting up the proper instrument modes for it to work as expected, including locking the channel so that you need to stop using the abstract instrument before initializing another one in the same channel. That would allow to use the same experiment code on instruments that drive wildly different but achieve the same results in different ways.

@MatthieuDartiailh
Copy link
Author

I am against setting features at start-up just to know their states as it is likely to mess up the user config (I have one user who need to fine tune the parameters of his spectrum analyser before taking a measurement and he won't be happy if the program reset in a way the instrument settings).
I agree about the simple honesty, we cannot I think fill all holes in instrument communication protocols. We could envisage to use Action instead of Feature for that kind of write only settings.

@iromero91
Copy link

Yes, delegating the state tracking to a higher level class could allow a GUI to ask the user which settings were entered in the device if they can't be read over the wire (to do things like setting up proper unit conversions, bounds checking, etc.), or have the user interface ask the user to perform a sequence of manual keypresses to get the device in the state that is needed by the experiment protocol. That's specially useful for read only multimeters and other simple instruments that only transmit the screen contents continuously with no possibility to send commands to change modes or ranges.

@alexforencich
Copy link
Member

The problem with interchanging feature and action is that this would preclude presenting cross-compatible interfaces for instruments of the same type. If I want to use a spectrum analyzer, anything that isn't a vendor-specific extension should work the same way across all spectrum analyzer drivers.

Now, as far as setting features at startup, I think the idea would be to pre-fill the cache with the some default state information, perhaps marked as 'stale' so it can be read out with the getter but not checked during a set operation. This would not be written to the instrument. I think the user should be responsible for making sure the instrument is configured properly, either by manually configuring it from the front panel or making sure that all of the appropriate parameters are set in the script.

@hgrecco
Copy link

hgrecco commented Feb 23, 2015

I agree with @MatthieuDartiailh that setting default values in initialize is not a good idea. Lot's of user do not configure everything with the computer but set a bunch of things manually and then continue from there. As @iromero91 said, this should be higher order API. In my opinion, in initialize there should be only those commands required to operate remotely.

The problem with interchanging feature and action is that this would preclude presenting cross-compatible interfaces for instruments of the same type.
I agree with @alexforencich that this is important and must be a goal of the project. So we should keep this in mind when designing the drivers.

I think the idea would be to pre-fill the cache with the some default state information, perhaps marked as 'stale' so it can be read out with the getter but not checked during a set operation.
This is exactly who we currently do it and it works quite nice. We have a sentinel object (MISSING) which is the default value in the cache. +1 on this or similar.

@iromero91
Copy link

To implement cross compatibility i think it's better to start from the top, defining an abstract interface for an idealized instrument, then a concrete implementation for each particular one (or a combination! say, implementing an RTD meter with a current source, a voltmeter, and a multiplexer for swapping polarity), but kept separate from driver code which would be done in a style more natural to how the actual instrument actually works. That way you can have nice well behaved instruments for high level setups while allowing people to take advantage of their particular bit of equipment's features/quirks without falling all the way down to dealing with the communication protocol itself.

@hgrecco
Copy link

hgrecco commented Feb 23, 2015

@iromero91 There was a proposal last week to start discussing types of instruments (which Features and Actions they should have). I would say let's start discussing as that will feedback positively in this discussion.

@alexforencich
Copy link
Member

Yeah, that's probably a good idea. I would say that the IVI spec is probably a good starting point for most instruments, though there are a few things that should probably be changed.

@MatthieuDartiailh
Copy link
Author

It would make sense to devote part of the wiki to that. And open specific issues for each kind of instruments. @alexforencich perhaps you could give it a start as you are probably the more familiar with that kind of issues and the IVI specs.

@alexforencich
Copy link
Member

The main thing is there are some consistency issues in the spec, some devices/subsystems are required to use getters/setters or to have a separate property to select the active channel that the other properties would then modify as opposed to just using indexes for everything. There are also some issues with the function generator base class wrt. how modulation is implemented that does not map on to modern generators well. Basically, IVI assumes that there is an amplitude modulation module that is shared across all of the channels, while on most modern multichannel ARBs, the modulation settings are a part of each channel. Another thing it does not do so well is throw a HUGE amount of functionality into the frequency counter base class, precluding its use for things like microwave counters. Reorganizing a bunch of this into separate extension classes might be a better idea.

However, the IVI spec does do a decent job of breaking functionality out into extension classes for most instruments. So starting from there would be reasonable.

@MatthieuDartiailh
Copy link
Author

I am not suggesting we follow ivi in everything (I tend to find it overdivided for example) but as you suggest it as a starting point and you are familiar with it and its limitations I think you are well placed to do some basic proposals.

@alexforencich
Copy link
Member

What do you mean by overdivided? Too many extension classes? Too many strangely named subsystems?

@MatthieuDartiailh
Copy link
Author

Too many strangely named subsystem but I have only looked quickly at python-ivi so perhaps I just need some convincing.

@MatthieuDartiailh
Copy link
Author

The discussion in LabPy/lantz_drivers#3 made this issue surface again. The case is the following :

For some sources the operating mode is determined by the first reach limit between current and voltage limits (Keysight E3631A, ...), for others (Yokogawa7651, ...) the mode must be set. In the last case, the more natural representation is a Feature but it will clash with the first case for which we cannot rely on caching. Once again this precludes the use of a nice standard.
I am still in favor of specifying whether or not to cache features, as the only other solution I see would mean using setters actions and seems terribly inconsistent to me.

Thoughts @hgrecco , @alexforencich, @crazyfermions, @iromero91

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

5 participants