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

Refactor doNds #193

Merged
merged 15 commits into from
Nov 28, 2019
Merged

Refactor doNds #193

merged 15 commits into from
Nov 28, 2019

Conversation

Dominik-Vogel
Copy link
Contributor

Lets create some tests to see if I broke anything ;-)

@Dominik-Vogel
Copy link
Contributor Author

@GateBuilder would you be interested in writing the tests?

@Dominik-Vogel
Copy link
Contributor Author

Here is already one I had used in another context:

def test_do0d():
    from qdev_wrappers.dataset.doNd import do0d, do1d
    from qcodes.instrument.parameter import Parameter
    from qcodes import config
    from qcodes.utils import validators
    config.user.mainfolder = "output"  # set ouput folder for doNd's

    s = Parameter('setter', set_cmd=None, get_cmd=None)
    p = Parameter(
        'test',
        set_cmd=None,
        get_cmd=lambda: 1+1j,
        vals=validators.ComplexNumbers())
    do1d(s, 0, 1, 1, 0, p)
    do0d(p)

) -> None:
for parameter in param_meas:
if isinstance(parameter, _BaseParameter):
meas.register_parameter(parameter,
Copy link
Collaborator

@ThorvaldLarsen ThorvaldLarsen Nov 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Dominik-Vogel is it possible that we can handle array parameters separately to ensure they get saved as blobs in the database. This was the main reason this PR #166 exists and why most transmon people have been working out of that branch for almost a year.

The doNd functions in this form are completely unusable when doing anything with an arrayparameter (the overhead for saving is way too large).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point! Thank you for notifying us!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will consider these in a new pr, to keep this one pure refactoring.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@GateBuilder can you work on a test for this?

@GateBuilder GateBuilder reopened this Nov 28, 2019
Copy link
Contributor

@GateBuilder GateBuilder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good thanks!

@GateBuilder GateBuilder merged commit 4fe8388 into qdev-dk:master Nov 28, 2019
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

Successfully merging this pull request may close these issues.

3 participants