-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Better qubit and measurement lo interface #2832
Comments
I remember why we didn't take the approach of
is separation of My another concern is naming issue ( |
The issue is naming. Right now it appears there are three ways to set the schedule lo
I don't really see the need for the users to ever use this class. They should be able to just provided a dictionary of channel:frequencies. Perhaps we could hide it? |
The only reason to have LoConfig class is self validation of frequency range. If we create pulse job validator, we don't need the class. |
I don't think the above addresses my concerns. |
I understand your point. I agree. We need to simplify the configuration of los. As far as I understand,
now we have too many options to set lo configuration, and we should simplify this. As (3) can cover (2), we might be able to deprecate And I don't like the name |
Yes, I agree. The other subtlety is that if we only allow |
I agree that @nkanazawa1989 thank you for laying out a detailed map of all the current possibilities :) |
Schedule_lo right now is essentially a dictionary that is a mapping of {channel: frequency}. schedule_los can be a list of these. Right now if you do not specify all Drive and Measure channels, it will default |
We are also able to complement I think we have three approaches (examples are how to overwrite default qubit drive[1] and apply frequency sweep of qubit drive[0]). 1. Rename
|
@lcapelluto I agree that |
In general, this problem would be overcome if for each pulse channel we could specify the frame and sideband with which it should be applied. Then we could track these frames and sidebands separately (as we do already internally). |
This PR adds more informative errors of qubit_lo_freq and meas_lo_freq are missing. A set of tests are added other assemble_schedule optional kwargs to ensure the desired behaviour. As a follow-up #2832 has been raised to discuss a better lo interface. Closes #2547 * Add tests for kwargs in assemble schedules. Make qubit_lo_freq and meas_lo_freq raise errors if not supplied. * Error is now raised for missing qubit and measurement los. * Added additional missing assemble kwarg tests. * Change test assertion. * Update changelog. * linting. * Fix lo range checking in lo_config. * update memory slot assembly test. * Fix merge bug.
PR #3468 does not address the problem of frame. After reading this conversion here is what I think would make sense from a physics point of view. Let's assume that the aim of
Consequences:
|
The one aspect I am still unclear on is how do we properly handle frames when changing frequencies. Consider the situation below for a cross-resonance gate with control i and target j.
I don't think this is captured within the current |
We can close this. The |
…#2833) This PR adds more informative errors of qubit_lo_freq and meas_lo_freq are missing. A set of tests are added other assemble_schedule optional kwargs to ensure the desired behaviour. As a follow-up Qiskit#2832 has been raised to discuss a better lo interface. Closes Qiskit#2547 * Add tests for kwargs in assemble schedules. Make qubit_lo_freq and meas_lo_freq raise errors if not supplied. * Error is now raised for missing qubit and measurement los. * Added additional missing assemble kwarg tests. * Change test assertion. * Update changelog. * linting. * Fix lo range checking in lo_config. * update memory slot assembly test. * Fix merge bug.
What is the expected enhancement?
I find the current lo interface confusing to use. I think it is a combination of naming and functionality. My main complaint is that:
qubit_lo_freq
andmeas_lo_freq
seem to serve the same purpose asschedule_los
This can be confusing for users. I think it may be better to replace
qubit_lo_freq
andmeas_lo_freq
byqubit_freq_est
andmeas_freq_est
and then have the default behaviour to be too use these to create and padschedule_los
if not supplied. This way there is only one explicit behaviour to set the los for assembly (schedule_los
).Thoughts @nkanazawa1989 and @itoko?
The text was updated successfully, but these errors were encountered: