Standford Research SR860 lock-in amplifier driver improvements #1160

sohailc · 2018-06-25T18:45:39Z

An update for the driver for Standford Research lock-in amplifiers SR86x and SR860 in particular. The work is done together with @astafan8.

BUG FIXES:

Fix maximum frequency for SRS860 - it is 500 kHz, not 500 Hz
get_capture_data uses the above functions; it does not have the 64kB limit anymore, instead it's limit is the current capture length (via _get_raw_capture_data function)
sine_outdc can set the value below 1 mV, the instrument can handle that

NEW DRIVER METHODS:

add caprture_one_sample_per_trigger that captures a given number of samples per each trigger signal; a callable argument starts the trigger pulses train
add start_capture_at_trigger that captures a given number of samples once it receives a trigger signal; a callable argument initiates sending a trigger signal
add set_capture_length_to_fit_samples to conveniently set the capture length for the number of sample (not kilobytes)
add wait_until_samples_captured to conveniently wait until the given number of samples is captured

NEW DRIVER PARAMETERS:

add parameter for the source of the trigger signal
add parameter for the source of the reference signal
_get_raw_capture_data_block implements the direct low-level call to CAPTUREGET command of the instrument
_get_raw_capture_data allows to get data from the beginning of the buffer avoiding the instrument's 64kB-reading-per-time limit

EXAMPLE NOTEBOOK UPDATED

more descriptive
covers capture modes

Code improvements related to capturing data via the buffer

add min_capture_length_in_kb as a property
add max_capture_length_in_kb as a property
make _set_capture_len_parser object method from static method
make _set_capture_len_parser use new capture-length-related properties
make _calc_capture_size_in_kb reuse _set_capture_len_parser
add convenient set_capture_rate_to_maximum method
add convenient _get_list_of_capture_variable_names and reuse it
add convenient _get_number_of_capture_variables and reuse it

@jenshnielsen @WilliamHPNielsen @Dominik-Vogel

… samples per each trigger signal; a callable argument starts the trigger pulses train - add start_capture_at_trigger that captures a given number of samples once it receives a trigger signal - add parameter for the source of trigger signal - add parameter for the source of reference signal

… well

- capture only the requested number of samples in start_capture_at_trigger, not the full buffer - add min_capture_length_in_kb property - add max_capture_length_in_kb property - make _set_capture_len_parser object method from static method - make _set_capture_len_parser use new capture-length-related properties - make _calc_capture_size_in_kb reuse _set_capture_len_parser - add convenient set_capture_rate_to_maximum method - add convenient _get_list_of_capture_variable_names and reuse it - add convenient _get_number_of_capture_variables and reuse it - remove capture_samples method because it does not make much sense

codecov · 2018-06-25T19:00:54Z

Codecov Report

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

@@           Coverage Diff           @@
##           master    #1160   +/-   ##
=======================================
  Coverage   79.67%   79.67%           
=======================================
  Files          47       47           
  Lines        6661     6661           
=======================================
  Hits         5307     5307           
  Misses       1354     1354

WilliamHPNielsen · 2018-06-26T07:50:02Z

Not-so-useful comment: It would make my editor happy if we could stick to a maximum line length of 79.

WilliamHPNielsen · 2018-06-26T07:53:35Z

qcodes/instrument_drivers/stanford_research/SR86x.py

+        argument to True allows this method to be used as well for set commands of the instrument parameters.
+
+        Args:
+            capture_length_in_kb (int): The desired capture length in kB.


I think it is superfluous to write the type down here again. (same for subsequent docstrings)

agree. i will remove it.

WilliamHPNielsen · 2018-06-26T07:56:28Z

qcodes/instrument_drivers/stanford_research/SR86x.py

+            capture_len_in_kb (int)
+        """
+        if capture_length_in_kb % 2:
+            capture_length_in_kb += 1


I don't think it is a good idea to give the user anything else than either exactly what they ask for or a big fat exception. I realise that this is a private method, but I think that principle is still valid.

yes, i will make it an exception. users will have to be explicit and the driver will be straightforward.

Just for the record, the lock-in itself when given an odd number can convert it to even internally.

WilliamHPNielsen · 2018-06-26T07:59:22Z

I left a few minor comments. Else it looks sensible. Have you tested it on real hardware, @sohailc and @astafan8?

sohailc · 2018-06-26T08:09:46Z

qcodes/instrument_drivers/stanford_research/SR86x.py

+        """
+        return self._parse_capture_length(capture_len_in_kb, issue_warning=True)
+
+    def _parse_capture_length(self, capture_length_in_kb: int, issue_warning: bool=False) -> int:


I think this function has a wrong name; it does not really parse anything. Additionally, I think maybe William is right when he says that the function should return exactly what the user asked (see his comment below). In this case, I think the function should be "validate_capture_length" and should not return anything

@WilliamHPNielsen Yes, this has all been tested on real hardware

- _get_raw_capture_data_block implements the direct low-level call to CAPTUREGET command of the instrument - _get_raw_capture_data allows to get data from the beginning of the buffer avoiding the instrument's 64kB-reading-per-time limit - get_capture_data uses the above functions; it does not have the 64kB limit anymore, instead it's limit is the current capture length (via _get_raw_capture_data function)

astafan8

Rigorously tested by @astafan8 on the device

- set_capture_length_to_fit_samples - wait_until_samples_captured

… sr860_capture_n_at_trigger

Dominik-Vogel

Looks good to me.
@sohailc or @astafan8 could you add an example for set_capture_length_to_fit_samples to the example notebook notebook?

It has been accidentally removed, but now it is back and implemented in a better way rather than "sleeping" for "capture_rate*sample_count" amount of time.

This is more consistent with the names of other methods. It is also a more correct name since the capture is not only started but also stopped within this method.

Before it was not, which resulted in a bug for cases where the capture config had more than one variable, that the function would return before the requested number of samples is acquired.

In "continuous" capture mode, this function does not work at high capture rates because the buffer starts getting overwritten *before* wait_until_samples_captured realizes that enough samples have been captured. This occurs due to the fact that VISA call to capture_bytes in the while in wait_until_samples_captured cannot keep up with the lock-in acquisition speed at high capture rates, hence the data gets "wrapped" (in other words, the buffer gets overwritten) which results in the value returned by capture_bytes to be small again, thus the while loop is not escaped.

It is now up to date with the driver implementation, and covers three popular capture modes.

updated the notebook

jenshnielsen · 2018-07-05T13:02:36Z

qcodes/instrument_drivers/stanford_research/SR86x.py

+        name
+        instrument
+            This argument is unused, but needed because the add_parameter
+            method of the Instrument base class adds this as a kwarg.


I don't understand why the instrument is just not passed to the super method so that the paramter is bound to the instrument. (I realize this is nothing new)

indeed. smth to fix later

The device can actually handle that, so now specifying 1e-6, 5e-9, etc. works without any problems.

Merge: 2162c5e 6d3a49d Author: Mikhail Astafev <astafan8@gmail.com> Merge pull request #1160 from sohailc/sr860_capture_n_at_trigger

sohailc added 6 commits June 14, 2018 15:28

Fix maximum frequency for SRS860 - it is 500 kHz

3e22e21

add start_triggers_pulsetrain callable to start_capture_at_trigger as…

aa55bd2

… well

Remove unnecessary empty lines

2e5d393

Give better name for the reference signal source parameter

e5528c1

sohailc added bug driver labels Jun 25, 2018

sohailc assigned sohailc and astafan8 Jun 25, 2018

sohailc requested review from jenshnielsen, astafan8, WilliamHPNielsen and Dominik-Vogel June 25, 2018 18:45

Fix bug due to property name in format string

2365639

Fix bug

06447a9

sohailc force-pushed the sr860_capture_n_at_trigger branch from 78ee2cb to 06447a9 Compare June 25, 2018 19:09

WilliamHPNielsen reviewed Jun 26, 2018

View reviewed changes

sohailc commented Jun 26, 2018

View reviewed changes

jenshnielsen mentioned this pull request Jun 27, 2018

Fix maximum frequency for SRS860 - it is 500 kHz #1164

Merged

sohailc added 4 commits June 27, 2018 15:03

Substitute warning with excpetion in set capture length parser

44fdc8c

Improve code style of the driver (PEP8)

442d241

Merge branch 'master' into sr860_capture_n_at_trigger

5507c2f

astafan8 approved these changes Jun 27, 2018

View reviewed changes

Merge branch 'master' into sr860_capture_n_at_trigger

07c1f7b

sohailc added 2 commits June 28, 2018 13:59

Extract duplicate code into convenient functions

11dbe5d

- set_capture_length_to_fit_samples - wait_until_samples_captured

Merge remote-tracking branch 'sohail/sr860_capture_n_at_trigger' into…

3cca41b

… sr860_capture_n_at_trigger

Dominik-Vogel previously requested changes Jul 2, 2018

View reviewed changes

sohailc and others added 8 commits July 2, 2018 15:54

Correct parameter name, add docstring for clarity

bc0c36e

Add parameter for input resistance of external reference

4f60e3d

Add docstring for reference source parameter

0580345

Bring back capture_samples (captures immediately and continuously)

ad62cc8

It has been accidentally removed, but now it is back and implemented in a better way rather than "sleeping" for "capture_rate*sample_count" amount of time.

Rename start_capture_at_trigger to capture_samples_after_trigger

4eb2915

This is more consistent with the names of other methods. It is also a more correct name since the capture is not only started but also stopped within this method.

wait_until_samples_captured now is aware of number of variables

99b63ac

Before it was not, which resulted in a bug for cases where the capture config had more than one variable, that the function would return before the requested number of samples is acquired.

Update the example notebook for SR86x driver

5d9ff61

It is now up to date with the driver implementation, and covers three popular capture modes.

jenshnielsen reviewed Jul 5, 2018

View reviewed changes

astafan8 added 2 commits July 5, 2018 15:23

Fix sine out dc level being not able to set values below 1 mV

2528698

The device can actually handle that, so now specifying 1e-6, 5e-9, etc. works without any problems.

Merge branch 'master' into sr860_capture_n_at_trigger

6d3a49d

astafan8 merged commit 0d85481 into microsoft:master Jul 5, 2018

sohailc deleted the sr860_capture_n_at_trigger branch July 5, 2018 13:42

giulioungaretti pushed a commit that referenced this pull request Jul 5, 2018

Generated gh-pages for commit 0d85481

00d1090

Merge: 2162c5e 6d3a49d Author: Mikhail Astafev <astafan8@gmail.com> Merge pull request #1160 from sohailc/sr860_capture_n_at_trigger

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Standford Research SR860 lock-in amplifier driver improvements #1160

Standford Research SR860 lock-in amplifier driver improvements #1160

sohailc commented Jun 25, 2018 •

edited by astafan8

codecov bot commented Jun 25, 2018 •

edited

WilliamHPNielsen commented Jun 26, 2018

WilliamHPNielsen Jun 26, 2018

sohailc Jun 26, 2018

WilliamHPNielsen Jun 26, 2018

sohailc Jun 26, 2018

WilliamHPNielsen commented Jun 26, 2018

sohailc Jun 26, 2018

sohailc Jun 26, 2018

astafan8 left a comment

Dominik-Vogel left a comment

jenshnielsen Jul 5, 2018

astafan8 Jul 5, 2018

Standford Research SR860 lock-in amplifier driver improvements #1160

Standford Research SR860 lock-in amplifier driver improvements #1160

Conversation

sohailc commented Jun 25, 2018 • edited by astafan8

BUG FIXES:

NEW DRIVER METHODS:

NEW DRIVER PARAMETERS:

EXAMPLE NOTEBOOK UPDATED

Code improvements related to capturing data via the buffer

codecov bot commented Jun 25, 2018 • edited

Codecov Report

WilliamHPNielsen commented Jun 26, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WilliamHPNielsen commented Jun 26, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

astafan8 left a comment

Choose a reason for hiding this comment

Dominik-Vogel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sohailc commented Jun 25, 2018 •

edited by astafan8

codecov bot commented Jun 25, 2018 •

edited