microsoft / Qcodes

Modular data acquisition framework
http://microsoft.github.io/Qcodes/
MIT License
328 stars 312 forks source link

Feature / Let measurement class register a ParamSpec #1220

Open WilliamHPNielsen opened 6 years ago

WilliamHPNielsen commented 6 years ago

There is no way (exposed by the API) to register a ParamSpec directly with a Measurement object. That's a petty, since doing that is necessary in many situations.

Let's fix that! I suppose the only question is: do we extend register_custom_parameter or make a new register_paramspec?

@QCoDeS/core @cgranade

astafan8 commented 6 years ago

I vote for register_paramspec. and the expected argument is a ParamSpec object. Also, register_parameter and register_custom_parameter should be re-implemented to call register_paramspec at the end, since this is what they are doing already.

The only difficulty (or smth else?) that i see is that ParamSpec includes the inferred_from and depends_on parts which the user now have to take care about, and register_paramspec has to somehow also call _registration_validation so that we know that registration is correct.

also, what are those "many situations"? from programming point of view, I completely agree, and I do not need examples of situations :) But from usage point of view I am concerned, since I feel that the qcodes Measurement API is attempted to be used to store stuff that is not "simple/usual" experimental data. If this is so, I wonder if our Measurement API is appropriate. In other words, I wouldn't want us to hack the Measurement API, but rather extend/improve if possible.

nataliejpg commented 6 years ago

I'd be happy with using register_parameter or register_paramspec but either way it would be really great to have this. From what I understand the Measurement object is the recommended way to save data but at the moment we can't use it for any homemade sweeps as they inevitably involve some analysis data which we store as Paramspec values. @astafan8 I am confused by your comment because my understanding was that the Measurement is exactly meant to be used for "simple/usual" experimental data? If we want to slightly extend some data taking from the ultimate "simple/usual" to the slightly more sophisticated it seems bad to have to change the API to stop using the Measurement, otherwise I would maybe just never use it rather than having to switch between two API's especially when the measurements I am running will only be a tiny bit different and will require the same 'set up' and 'tear down' actions.

astafan8 commented 6 years ago

@nataliejpg I should have been more careful when saying "simple/usual". What I meant was the following: to my understanding, Measurement's and DataSet's primary usage is to save data (99.99% of the cases, basically, numbers) which are associated with qcodes Parameters (!); and under the hood, Paramspec is used (basically, Paramspec instances are created based on qcodes Parameters that are registered). So, I just wondered what is that the researchers would like to do, that led them to the conclusion that Paramspec is a good mean of doing it. Moreover, I mean, it could very well be that I am just not aware of some information, and people like @WilliamHPNielsen can help me with that :)

nataliejpg commented 6 years ago

@astafan8 I have been using it to manually save analysis data which I would previously have used a ManualParameter for combined with some logic in the get_cmd to do analysis. At the moment I am also using it to save all my alazar data because I can't save values of MultiParameter class in the database any other way but that's another story which I think we covered in #1200

WilliamHPNielsen commented 6 years ago

I would tend to agree with @astafan8 that using a ParamSpec instead of a Parameter seems wrong (since the ParamSpec was born as a very SQLite specific adapter) and should at least not be any easier than using Parameters for everything. But I think this (again) is a hint to us that there should be a simpler Parameter class to hold things that do not come directly out of instruments. And perhaps that simpler Parameter is indeed the ParamSpec right now...

But as for allowing ParamSpecs to be registered directly, I would indeed see that as an extension rather than a hack.

nataliejpg commented 6 years ago

Interesting. So what was ParamSpec meant to be used for? To be honest I always found ManualParameter quite unweildly and used to feel like I was forcing something into being a parameter which did't need to be one (usually things which would make no sense in the snapshot, don't belong to an instrument, can't actually be swept). Just not really clear on what the concept is for when to use ManualParametes and when to use Paramspec beyond the obvious case when Parameters represent physical settings on an instrument.

WilliamHPNielsen commented 6 years ago

Initially, ParamSpec was only meant to be the very thin adapter between the python API and the database. Basically just a class for saying "there will be a column called blah with values of type bluh". But as the database became aware of more things (dependencies), the ParamSpec also grew and is now perhaps a good representation of a "Parameter" (as in: something we record and possibly vary, not limited to an instrument setting).

The ManualParameter was improved a lot with the last restructure of parameters. The name ManualParameter is now just a shim for a special case of Parameter. So perhaps they've got a bit less unwieldy? I would - like Mikhail - hope that you never have to use anything else than a Parameter to cover your needs.

In any case, I maintain that ParamSpecs should be allowed to be directly registered. Further discussion of Parameter improvements probably belongs in a dedicated discussion issue. I would be interested to hear use cases about ParamSpec versus Parameter.

nataliejpg commented 6 years ago

@WilliamHPNielsen Thanks that was helpful. I guess I would like to work out the 'best' way to save analysis data in the database but that discussion can very much be held elsewhere at another time even. It is entirely possible that I feel Paramspec is more suitable just because there was a better example notebook for it. In the meantime I agree that you should be able to register a Paramspec

nataliejpg commented 6 years ago

I would be interested to hear use cases about ParamSpec versus Parameter.

@WilliamHPNielsen I found a use case:

I want to use the measurement object to save data but one of the 'parameters' I want to save is just a counter.

depends_on_parameter = ParamSpec("depends_on", "numeric")
analysis_meas = Measurement("analysis")
analysis_meas.register_parameter(depends_on_parameter)
analysis_meas.register_parameter(
        real_instrument_parameter, setpoints=[depends_on_parameter])
analysis_meas.write_period = 1

with analysis_meas.run() as datasaver:
    for i, pulse_dur in np.arange(10):
        some_action(pulse_dur)
        datasaver.add_result(
            (real_instrument_parameter.name, real_instrument_parameter()),
            ('depends_on', i))

If I have depends_on as a Parameter rather than a ParamSpec I then have to also set it within my for loop which feels really unnatural. If I don't set it then I end up saving values which are out of sync with the values of the parameter I have created which also feels wrong.

astafan8 commented 6 years ago

@nataliejpg , I understand your example, but the intended way forward is indeed to use Parameter. You can easily create one without it being a part of an Instrument (e.g. through Parameter or ManualParameter).

The importance of your example also depends on the meaning of the "counter": could you describe a real-world example? What would the counter mean?

nataliejpg commented 6 years ago

@astafan8 hmmm ok, just doesn't seem like a very elegant solution to have to keep setting this parameter which you have only created to be saved to the dataset or to let it get out of sync. In real life counter is actually the row number of the measurement in the dataset which I am analysing so the example is very much contrived and would be solved if add_result could take the id of another measurement or dataset. As in #1286

WilliamHPNielsen commented 6 years ago

@nataliejpg, I don't understand your example. What is wrong with doing the following?

dummy = DummyInstrument('dummy', gates=['real_instrument_parameter'])

depends_on_parameter = Parameter('depends_on', set_cmd=None)
analysis_meas = Measurement()
analysis_meas.register_parameter(depends_on_parameter)
analysis_meas.register_parameter(
        dummy.real_instrument_parameter, setpoints=[depends_on_parameter])
analysis_meas.write_period = 1

with analysis_meas.run() as datasaver:
    for i in np.arange(10):
        datasaver.add_result(
            (dummy.real_instrument_parameter, dummy.real_instrument_parameter()),
            (depends_on_parameter, i))

If I have depends_on as a Parameter rather than a ParamSpec I then have to also set it within my for loop

As the snippet shows, that's simply not true.

1286 is a different story, but I think we all agree that that is very much a do-want.

nataliejpg commented 6 years ago

@WilliamHPNielsen It's true. It is possible. It just seemed like bad form to save values in the dataset which don't match the saved value of the parameter. I should have written 'I have to also set it within my loop unless I want the parameter value to be out of sync'. If that's the 'recommended solution' then I will use it, it is just a situation that I came across that I didn't feel very comfortable in and I thought might interest you as you were looking for use cases where directly using ParamSpec would be preferable. Does that make it more clear?

astafan8 commented 6 years ago

@nataliejpg , I agree with @WilliamHPNielsen here. Moreover, so far, I haven't really seen a use case where "directly using ParamSpec would be preferable" - also because ParamSpec is not intended for direct use by users :)

WilliamHPNielsen commented 6 years ago

@nataliejpg, now I see your point, thanks for clarifying. When we originally designed the context manager, it was actually a design criterion for us that the user be able to save values not synced with the state of the corresponding parameter. The idea was that the user could know more than QCoDeS (like there being some non-linear offset, etc.) and should be able to easily write that down in the data. I never thought about it this way around before.

Having now done that for five minutes, I still don't think it's an issue that your dataset contains values that the Parameter never actually had. The Parameter is there to serve the dataset, not the other way around.

nataliejpg commented 6 years ago

@WilliamHPNielsen ok thanks for the explanation. So long as it's clear that it's meant to be ok to do that then I'm all good.