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

Alazar: refactor DLL API related things #1471

Merged
merged 196 commits into from
Mar 18, 2019

Conversation

astafan8
Copy link
Contributor

@astafan8 astafan8 commented Feb 13, 2019

Main point of the PR

This PR extracts all C-level calls to the Alazar SDK into its own class to make it easier for that class to manage the use of background threads (and other stuff, if needed, like asyncio), and remove all these low-level details out of the driver class to make the latter manageable.

This is done by:

  • Make the driver use a dedicated AlazarAPIAPI class that is based on WrappedDll class and DllWrapperMeta metaclass.
    • DllWrapperMeta ensures that there is one instance of the class per given DLL path (kind of singleton).
    • WrappedDll uses DllWrapperMeta class:
      • creates lock and thread pool executor for the DLL calls
      • assigns ctypes to DLL function arguments and returns (specified in signatures attribute)
      • WrappedDll allows AlazarATSAPI methods (that represent Alazar ATS API functions) to accept qcodes Parameters, and makes them aware of TraceParameters of the alazar driver.
    • AlazarATSAPI class defines ctypes signatures for relevant DLL functions, as well as bound methods which represent them; the class also implements convenience methods which make work with the API functions more convenient from the driver class point of view.

Note that the new DLL wrapper class here is somewhat coupled to the Alazar usage, generalizing it to "any DLL" does not bring much value now.

Thanks to @cgranade for the initial bulk of the work in #1379 PR. However, asyncio-related parts were not taken into this PR.

Other changes in this PR:

  • move many convenience methods from alazar ats driver class to alazar api class
  • add minor tests for alazar api class
  • add minor tests for alazar dma buffer class
  • create a module with alazar api constants, populate it with ones that are used in the driver
  • attempt to cleanup docstrings in the driver
  • refactor get_idn of the alazar driver
  • refactor everything related to querying capabilities from alazar driver class into CapabilityHelper class that the driver uses
  • Fix get_num_channels alazar driver method for 4 and 8 channel configurations
  • Make the Alazar example notebook runnable without any additional hardware
  • use instance logger in alazar

To-Do:

  • squash when merge
  • (thanks @jenshnielsen) run test acquisitions to see if the driver hasn't been broken :)
  • (thanks @jenshnielsen) run test acquisitions to see if the changes don't make it slower :)

Epilogue:

From Alazar API header files:

#define MEDIMUM_EXTERNAL_CLOCK	0x00000003UL //TYPO
#define MEDIUM_EXTERNAL_CLOCK	0x00000003UL

@codecov
Copy link

codecov bot commented Mar 14, 2019

Codecov Report

Merging #1471 into master will decrease coverage by 3.26%.
The diff coverage is 65.07%.

@@            Coverage Diff             @@
##           master    #1471      +/-   ##
==========================================
- Coverage   73.84%   70.57%   -3.27%     
==========================================
  Files          92      102      +10     
  Lines       10459    11546    +1087     
==========================================
+ Hits         7723     8149     +426     
- Misses       2736     3397     +661

Copy link
Collaborator

@jenshnielsen jenshnielsen left a comment

Choose a reason for hiding this comment

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

This looks good. Only a few very minor comments. Let me know if I should test it on a 9360 card

Now that we have tests for the Alazar it can be removed from the excluded here https://github.com/QCoDeS/Qcodes/blob/master/qcodes/.coveragerc

The trailing whitespaces that codacy found should also be fixed

qcodes/instrument_drivers/AlazarTech/ATS.py Show resolved Hide resolved
qcodes/instrument_drivers/AlazarTech/ATS.py Outdated Show resolved Hide resolved
qcodes/instrument_drivers/AlazarTech/utils.py Outdated Show resolved Hide resolved
@astafan8
Copy link
Contributor Author

@jenshnielsen If you have time, i'd appreciate if you test this PR on your setup. I am also going to test it with my 9360 card. In particular, it would be great if you have some tests at hand which can show that the overall performance did not degrade.

@astafan8
Copy link
Contributor Author

After CI is done, i'll squash-merge it (rebasing ~2000 commits was not successful at all).

@astafan8 astafan8 merged commit 60fbd5e into microsoft:master Mar 18, 2019
@astafan8 astafan8 deleted the alazar/refactor/dll-api branch March 18, 2019 15:15
giulioungaretti pushed a commit that referenced this pull request Mar 18, 2019
Author: Mikhail Astafev <astafan8@gmail.com>

    Alazar: refactor DLL API related things (#1471)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants