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

Documentation of breaking changes of MFLIs could include more details #41

Open
jenshnielsen opened this issue May 20, 2022 · 5 comments

Comments

@jenshnielsen
Copy link
Collaborator

The documentation here mentions a list of nodes that have changed but I don't see that anywhere in that page. The page here does not mention changes to sigout and demods

Specifically I don't see the following documented

  • sigout[0].amplitudes0...3 have changed into a ChannelList with a single parameter value sigout[0].amplitudes[0...3].value
  • Same with enables
  • demods[0].sample now returns a dict rather than a complex number.

The first 2 changes are clearly improvements but it would be nice if they had been documented.

The 3th one is more problematic since it now breaks our ability to directly measure parameters using qcodes.

e.g. the following no longer works

output_no_sync = do1d(freqparam, 50, 300, 10, 2, mfli2.demods[0].sample)

For now we have our resorted to work around this with a complex_sample parameter that retains the old behavior added in a subclass

@tobiasah
Copy link
Member

I See that the MFLI refactoring page is rather empty. If you like you can contribute to the documentation with your discovered changes. Since the value thing is a general change we will update our documentation in that way (would be great if QCoDeS would support having parameter lists and we would not have the need for the value hack. Or is this maybe even possible?).

About the third problem. I agree that this is a breaking change but just returning a complex value was a obvious mistake of the old driver. On looses important information about DIOs, Trigger ...

Ignoring the fact the mfli2.demods[0].sample() is not the correct way of getting demod samples from our devices (please use subscribe and poll instead!) and is far away from being able to deliver the sampling rate our devices are capable of, I don't see way how we could improve this or make it easier for you.

What would you like us to provide in this case?

Maybe a simple lambda function solves this problem?
sample_to_complex = lambda sample: complex(sample["x"],sample["x"])

@simonzihlmann
Copy link

I strongly agree with @jenshnielsen for the third point. The beauty of having a qcodes.Parameter is that it can be measured easily and that many handy things come with it.

I see your point @tobiasah that mfli2.demods[0].sample does not include information about DIO, Trigger and so forth. Also the timing is not properly defined and I also see that your instruments can deliver samples at a much higher rate. However, sometimes this is not needed, especially for low frequency transport measurements where timing is not so critical. There it would be really nice to have a qcodes.Parameter that returns the complex response as a complex number (as it used to be the case).

@tobiasah I guess uhfli.demods[0].sample or hf2li.demods[0].sample do also return a dict now and no longer a complex number, is this right?

Am I right that the information about DIOs, Trigger, ... would be contained in the snapshot of the device? Could yo confirm this @jenshnielsen?

@jenshnielsen is the complex_parameter in a sublcass somewhere in qcodes? Can I find this somewhere? Because we are using this functionality right now and updating zhinst-qcodes would break our experiments.

@jenshnielsen
Copy link
Collaborator Author

@simonzihlmann thats something that we put in an internal helper function. I am happy to share it with you thou. I have been thinking about implementing a Parameter Class in QCoDeS that can handle this type of Parameter (returning a dict) in a consistent way that allows users to sub select elements. I am however rather busy with other stuff but I will try to get the proposal up in qcodes asap

@simonzihlmann
Copy link

@simonzihlmann thats something that we put in an internal helper function. I am happy to share it with you thou. I have been thinking about implementing a Parameter Class in QCoDeS that can handle this type of Parameter (returning a dict) in a consistent way that allows users to sub select elements. I am however rather busy with other stuff but I will try to get the proposal up in qcodes asap

@jenshnielsen that would be great! I guess everyone using qcodes would be very happy to have such a class.

@jenshnielsen
Copy link
Collaborator Author

Forgot to post out work around implementation but it is currently

class ComplexSampleParameter(Parameter):
    def __init__(
        self, *args: Any, dict_parameter: Optional[Parameter] = None, **kwargs: Any
    ):
        super().__init__(*args, **kwargs)
        if dict_parameter is None:
            raise TypeError("ComplexCampleParameter requires a dict_parameter")
        self._dict_parameter = dict_parameter

    def get_raw(self) -> ParamRawDataType:
        values_dict = self._dict_parameter.get()
        return complex(values_dict["x"], values_dict["y"])


class MFLIWithComeplexSample(zhinst.qcodes.MFLI):
    """
    This wrapper adds back a "complex sample" parameter to the demodulators such that
    we can use them in the way that we have done with "sample" parameter
    in version 0.2 of ZHINST-qcodes
    """

    def __init__(self, name: str, serial: str, **kwargs: Any):
        super().__init__(
            name=name, serial=serial, **kwargs
        )
        for demod in self.demods:
            demod.add_parameter(
                "complex_sample",
                label="Vrms",
                vals=vals.ComplexNumbers(),
                parameter_class=ComplexSampleParameter,
                dict_parameter=demod.sample,
                snapshot_value=False,
            )

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

3 participants