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

SR830: Add increment/decrement sensitivity functions. #1380

Merged
merged 3 commits into from
Nov 12, 2018

Conversation

spxtr
Copy link
Contributor

@spxtr spxtr commented Nov 9, 2018

I often need to auto range lock-ins mid-sweep. I think it's usually best for instrument drivers to only act as instrument drivers, so I went with as small a change as possible to let me implement autoranging outside of the driver. It would have also been possible to add autoranging behavior into the driver itself, and if y'all prefer that then I'd be happy to update this PR.

The two functions in this change make it possible to do things like the following in my measurement sweeps:

def autorange(srs, max_changes=1):
    def autorange_once():
        r = srs.R.get()
        sens = srs.sensitivity.get()
        if r > 0.9 * sens:
            return srs.increment_sensitivity()
        elif r < 0.1 * sens:
            return srs.decrement_sensitivity()
        return False

    sets = 0
    while autorange_once() and sets < max_changes:
        sets += 1
        time.sleep(srs.time_constant.get())

@StefanD986 @astafan8

@codecov
Copy link

codecov bot commented Nov 9, 2018

Codecov Report

Merging #1380 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1380   +/-   ##
=======================================
  Coverage   73.22%   73.22%           
=======================================
  Files          79       79           
  Lines        9188     9188           
=======================================
  Hits         6728     6728           
  Misses       2460     2460

@jenshnielsen
Copy link
Collaborator

@spxtr I think your idea is good and it makes a lot of sense to implement this and then make it possible for the user to implement the autorange. Would you be willing to document the feature in the example notebook here https://github.com/QCoDeS/Qcodes/blob/master/docs/examples/driver_examples/Qcodes%20example%20with%20Stanford%20SR830.ipynb

Copy link
Contributor

@astafan8 astafan8 left a comment

Choose a reason for hiding this comment

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

I agree with the approach described in the description of the PR. Thanks! And it would be indeed great if you could document this feature and how to use it in the driver's example notebook (basically just copy pasting the PR's description with some extra explanatory text would suffice).

return self._change_sensitivity(-1)

def _change_sensitivity(self, dn):
n = int(self.ask('SENS?'))
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd like to suggest reusing the sensitivity parameter, so that duplicated bare SCPI commands are not scattered around the driver (unless really needed).

So, in order to do that, what about the following:

self.sensitivity.get()  # or self.sensitivity()
n = self.sensitivity.raw_value

and for setting the new sensitivity value below, just use the n_to:

self.sensitivity.set(n_to(n + dn))  # or self.sensitivity(n_to(n + dn))

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. This does make sense, I didn't know the raw value was accessible in this way.

@jenshnielsen jenshnielsen merged commit 4dd73ba into microsoft:master Nov 12, 2018
giulioungaretti pushed a commit that referenced this pull request Nov 12, 2018
Merge: 4c6581e 6485d9d
Author: Jens Hedegaard Nielsen <jenshnielsen@gmail.com>

    Merge pull request #1380 from spxtr/change_sensitivity
@spxtr spxtr deleted the change_sensitivity branch December 3, 2018 07:32
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

3 participants