Skip to content

Comments

Znb 20 fixes as requested by t10#623

Closed
jenshnielsen wants to merge 19 commits intomicrosoft:masterfrom
jenshnielsen:znb_20_fixes_t10
Closed

Znb 20 fixes as requested by t10#623
jenshnielsen wants to merge 19 commits intomicrosoft:masterfrom
jenshnielsen:znb_20_fixes_t10

Conversation

@jenshnielsen
Copy link
Collaborator

@jenshnielsen jenshnielsen commented May 19, 2017

  • Read out S11, S12, S22, S21
  • Ability to autoscale
  • Center/Span
  • Display all outputs on request
  • Markers and peak finders
  • Output scale
  • default settings on startup

@jenshnielsen
Copy link
Collaborator Author

@core This should be ready for review. The markers and peak finders are not implemented but that is less de scoped for the moment.

@nataliejpg Would be great if you have a chance to look at the changes. The example notebook is probably the easiest place to start

Copy link
Contributor

@nataliejpg nataliejpg left a comment

Choose a reason for hiding this comment

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

In general looks good.

I can't tell how easy it is to use the instrument in the way it was being used as it was before (ie usually just with 1 S21 trace), is it possible to display just that or do you have to display all 4 or none at all? What happens when you assign 4 traces? Does it take them all? Will it be just as fast if I want to use the instrument for only one like I was doing before?

Is it possible to add channel and sindex to the docstings as I can't tell what they are really?

I had some idea that it would be nice to have the sindex as a parameter which you set (and then influences the code executed in the get call) rather than having a parameter for each sindex because that has the potential to grow rather large if people want more than s21 22 etc. This method just seems somewhat less flexible (how to extend to adding external generator?). Then you could have differnt traces as different channels (when we finally have channels implemented in a meaningful way in qcodes) which to me seems to be a more faithful implementation of what the instrument is doing not to mention my allergy to having numbers in the parameter names. That said it will probably fulfill 99% of user needs and I may well be overthinking it and if I wanted it a different way I should have written it myself

@jenshnielsen
Copy link
Collaborator Author

A new version based on channels is available in https://github.com/jenshnielsen/qcodes/tree/znb_with_channels this requires #568 to be merged along with more modifications in prs for that pr

This is a good deal cleaner but still defaults to setting up 4 channels. In the future we we can expand this to dynamically add channels with parameters (z, y snm n,m>2...) I don't think this is a performance problem as we disable output before capturing a channel output

  • Actually test the new implementation with channels
  • benchmark performance implications of multiple channels

@jenshnielsen
Copy link
Collaborator Author

replaced by #643

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.

2 participants