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

QickSweep can now calclate the actual rounded sweep points #276

Merged
merged 4 commits into from
Aug 12, 2024

Conversation

yoshi74ls181
Copy link
Contributor

Added QickSweep.get_actual_values, which calculates the actual sweep points after rounding to ASM units. There are also some unrelated fixes:

  • Documented the keyword argument ro_ch for QickProgramV2.add_pulse
  • Fixed a typo in pyro.make_proxy

@meeg meeg merged commit fdc2cde into openquantumhardware:main Aug 12, 2024
1 check passed
@meeg
Copy link
Collaborator

meeg commented Aug 12, 2024

Looks good, thanks!

I am a little worried that QickSweep.get_actual_values() is largely redundant with QickProgramV2.get_pulse_param() but they both work, I don't want to get in your way, and your solution might be better. I can think about it later, and for now I am happy to merge this.

@yoshi74ls181
Copy link
Contributor Author

Thank you very much for the quick merge!

I am a little worried that QickSweep.get_actual_values() is largely redundant with QickProgramV2.get_pulse_param()

The advantage of the new QickSweep.get_actual_values() is that it works for a Delay as well as for a Pulse. A disadvantage might be that its implementation involves recursions and closures. If that is not a big concern, maybe it's a good idea to rewrite QickProgramV2.get_pulse_param() to use the new QickSweep.get_actual_values(). The former will still be useful because the user doesn't need to pass to it the loop order and numbers of iterations.

@meeg
Copy link
Collaborator

meeg commented Aug 14, 2024

Yes, agree that it's important to be able to get rounded delays. My idea for that was that you can specify an arbitrary "tag" string as a parameter to a timed instruction (pulse, trigger, delay, wait), and then you can do something like QickProgramV2.get_time_param(tag). Does that sound good?

If get_pulse_param() and get_time_param() are sufficient I would want to make those the preferred user interface, because they work with scalar values as well as sweeps. And I agree, probably both should use get_actual_values() internally, I think it is a better design than what I had.

@yoshi74ls181
Copy link
Contributor Author

Your idea for get_time_param(tag) sounds good! get_pulse_param() and get_time_param() can be the preferred user interface when using qick_lib directly, but for writing our QCoDeS driver QickSweep.get_actual_values() makes more sense because it works in the same way for pulses and delays.

@meeg meeg mentioned this pull request Aug 25, 2024
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