-
Notifications
You must be signed in to change notification settings - Fork 301
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
Keithley 2600: 4 probe current sweep support #3023
Keithley 2600: 4 probe current sweep support #3023
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3023 +/- ##
==========================================
- Coverage 65.55% 65.52% -0.03%
==========================================
Files 213 213
Lines 28441 28454 +13
==========================================
Hits 18644 18644
- Misses 9797 9810 +13 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i know this is a bit out of scope of the PR, but can the "fastsweep" parameter be explicitly mentioned in the notebook? :) please? :) otherwise the reader thinks that doFastSweep is the only way to run the measuyrement (resulting in a legacy qcodes dataset!), while it is not, because "fastsweep" is an array parameter and can be used with the Measurement class as usual.
self.setpoint_names = ('Current',) | ||
self.setpoint_units = ('A',) | ||
self.label = 'voltage' | ||
self._short_name = 'vi_sweep_four' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for clarity/expressiveness?
self._short_name = 'vi_sweep_four' | |
self._short_name = 'vi_sweep_four_probe' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree :-)
@@ -62,6 +62,13 @@ def prepareSweep(self, start: float, stop: float, steps: int, | |||
self.label = 'voltage' | |||
self._short_name = 'vi_sweep' | |||
|
|||
if mode == 'VIfour': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.. or is this too verbose?
if mode == 'VIfour': | |
if mode == 'VIfourprobe': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree :-)
@@ -62,6 +62,13 @@ def prepareSweep(self, start: float, stop: float, steps: int, | |||
self.label = 'voltage' | |||
self._short_name = 'vi_sweep' | |||
|
|||
if mode == 'VIfour': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be also added to the docstring, not only here but also in other methods in this file ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree :-)
Agree with @astafan8 that we should not promote the |
@astafan8 I have updated the notebook to use fastsweep in combination with a d0d. We could also make a new function(do_faste_sweep) in the driver wrapping the d0d. So performing the fastsweep stays a one-liner? |
perfect!!! thank you!
no, that would be mixing concerns for negligible benefit -- do*d is already a one-liner-ish :) and the fact that doFastSweep exists as a method is just historical and shouldn't have been added in the first place. |
This PR adds a new mode to the doFastSweep function of the of Keithley 26xx
The new mode allows for 4 probe measurements
@jenshnielsen