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

feature: Add voltage divider device #492

Merged
merged 4 commits into from
Mar 1, 2017
Merged

Conversation

giulioungaretti
Copy link
Contributor

No description provided.

"""
Returns:
number: value at which the attached paraemter is (i.e. does not
account for the scaling)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should un-indent to the level of number to render correctly

"""
Resitive voltage divider

To be used when you use a voltage divider to measure a parameter.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are you measuring the parameter. The example sounds more like you are setting an output?

Output of dac is X -> Voltage divider -> x/10 to actual Experiment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's fine like this, you can also get values.

@giulioungaretti
Copy link
Contributor Author

@QCoDeS/core moar opinions, else I will just merge and let people try ?

@jenshnielsen
Copy link
Collaborator

👍 examples of using it both in input and output would probably make it more clear how it works but that can wait

@MerlinSmiles
Copy link
Contributor

Just want to throw two things in here:
https://github.com/QCoDeS/Qcodes/blob/master/qcodes/instrument_drivers/stanford_research/SR560.py
and #57

Maybe its more general then adding devices for all kinds of conversions...?

@jenshnielsen
Copy link
Collaborator

I think it makes sense to move the VoltageParameter and also the CurrentParameter from https://github.com/QCoDeS/Qcodes/blob/master/qcodes/instrument_drivers/ithaco/Ithaco_1211.py#L7 here to this location they dont really seem to belong in a specific instrument driver

@giulioungaretti
Copy link
Contributor Author

@jenshnielsen doc wise, I am not sure I understand. there are both sets and gets in the examples :D
@MerlinSmiles @jenshnielsen yeah it may make sense, but let's postpone the dicussion untill we have a working dataset. It's a lot better to repeat some code than to make the wrong abstractions IMHO 🦄


def set(self, value: Union[int, float]) -> None:
instrument_value = value * self.division_value
self._save_val(value)
Copy link
Collaborator

Choose a reason for hiding this comment

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

what happens if instrument does not have a set function

Copy link
Contributor Author

Choose a reason for hiding this comment

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

well it's going to raise an exception :D as expected :D

Copy link
Contributor

Choose a reason for hiding this comment

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

How about setting the parameter's has_set to equal that of the original parameter?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nulinspiratie

a = MockParabola("yolo")
attn_dac0 = qc.VoltageDivider(a.parabola, 10, "combained", "atteunauted dac")

the error:

>>> attn_dac0(0)
/src/Qcodes/qcodes/instrument/parameter.py in no_setter(*args, **kwargs)
    676
    677 def no_setter(*args, **kwargs):
--> 678     raise NotImplementedError('This Parameter has no setter defined.')


NotImplementedError: ('This Parameter has no setter defined.', 'setting yolo_parabola to 0')

The difference why using has_set is that one gets the real error i.e. that the underlying parameter has no set :D

Does that make sense ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah yeah that would give a clearer error message

@giulioungaretti
Copy link
Contributor Author

@jenshnielsen more feedback, I would merge other wise :D

@jenshnielsen
Copy link
Collaborator

IMHO the current documentation is still not very clear in the distinction between using it as an input or output device. I had to read it several times to understand how it was ment to be used in the two modes and what refers to what but perhaps it's more clear to others. Otherwise 👍

@giulioungaretti giulioungaretti merged commit 50388b1 into master Mar 1, 2017
@giulioungaretti giulioungaretti deleted the feature/voltagedivider branch March 1, 2017 11:59
giulioungaretti pushed a commit that referenced this pull request Mar 1, 2017
Author: Giulio Ungaretti <giulio.ungaretti@gmail.com>

    feature: Add voltage divider device (#492)
giulioungaretti pushed a commit that referenced this pull request Mar 2, 2017
Author: Giulio Ungaretti <giulio.ungaretti@gmail.com>

    feature: Add voltage divider device (#492)
giulioungaretti pushed a commit that referenced this pull request Mar 2, 2017
Author: Giulio Ungaretti <giulio.ungaretti@gmail.com>

    feature: Add voltage divider device (#492)
giulioungaretti added a commit that referenced this pull request Mar 2, 2017
* feature: Add voltage divider device

* fix: Rst polish and save metadata

* fix: Save value also when getting

* docs: Better docstring
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.

None yet

4 participants