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

Znb read from existing channels #1111

Merged
merged 19 commits into from
Apr 3, 2019

Conversation

jenshnielsen
Copy link
Collaborator

@QCoDeS/core This enables reading channels that are already mapped on the ZNB from a manual user config. This is sort of a stop gap solution because we cant fully setup everything with the driver yet.

For use by @GrigoryanRuben

@codecov
Copy link

codecov bot commented May 28, 2018

Codecov Report

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

@@           Coverage Diff           @@
##           master    #1111   +/-   ##
=======================================
  Coverage   70.66%   70.66%           
=======================================
  Files         102      102           
  Lines       11585    11585           
=======================================
  Hits         8186     8186           
  Misses       3399     3399

@jenshnielsen jenshnielsen changed the title [WIP] Znb read from existing channels Znb read from existing channels Apr 3, 2019
@jenshnielsen
Copy link
Collaborator Author

So I would much prefer to have this done in a proper way but I think this is good enough to be useful. And I have a hart time seeing how we would get enough bandwidth to implement all the functionality needed to setup more advanced channels on the ZNB without relying on using the front panel. @QCoDeS/core What do you think?

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 incremental approach here due to the complexity of the instrument.

@astafan8
Copy link
Contributor

astafan8 commented Apr 3, 2019

Big thanks for reworking the notebook!!!

Minor comments on the notebook:

  • perhaps it can be executed with a new db file so that there is no long list of measurements at the top of the notebook))
  • in17 - not full_trace but trace_mag_phase if understand it correctly?
  • in40 - is the warning there by intention? it does not seem like so.

Copy link
Contributor

@Dominik-Vogel Dominik-Vogel left a comment

Choose a reason for hiding this comment

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

I think that looks very sensible. If you feel in the mood for it you could add an example for the trace_mag_phase parameter.

@jenshnielsen
Copy link
Collaborator Author

@astafan8

  • Yes, fixed
  • Yes that parameter changed name at some point and the notebook was not updated
  • Added the rf on and cleared the warning. I don't have the instrument any more. So I did not rerun

@Dominik-Vogel There is an example of that in [17] as far as I can see

@jenshnielsen jenshnielsen merged commit 5d7e975 into microsoft:master Apr 3, 2019
@jenshnielsen jenshnielsen deleted the znb_existing_channels branch April 3, 2019 13:05
giulioungaretti pushed a commit that referenced this pull request Apr 3, 2019
Merge: fa0b8d2 57d0cf6
Author: Jens Hedegaard Nielsen <[email protected]>

    Merge pull request #1111 from jenshnielsen/znb_existing_channels
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.

3 participants