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

Drop support for backendV2 in from_backend and _to_schedule_list #249

Merged

Conversation

to24toro
Copy link
Contributor

Summary

closes #236.

Details and comments

The type hint of an argument backend in from_backend supports both backendV1 and backendV2. However, we found it confuses people because almost all backendV2 cannot be used for dynamics backend in reality.

In addition, I changed the type hint of an argument backend from BackendV2 to BackendV1 in _to_schedule_list. In _to_schedule_list, build_schedule is used. Scheduling with backendV2 fails because of some bugs in terra. I am assigned to work on it but cannot solve it yet.

In the future, supporting BackendV2 again in these functions is essential for users.

@@ -958,7 +958,7 @@ def _get_acquire_instruction_timings(


def _to_schedule_list(
run_input: List[Union[QuantumCircuit, Schedule, ScheduleBlock]], backend: BackendV2
run_input: List[Union[QuantumCircuit, Schedule, ScheduleBlock]], backend: BackendV1
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is actually called only on DynamicsBackend, which is BackendV2. I'm assuming it works on DynamicsBackend otherwise DynamicsBackend would never work. Is this issue in Terra in the main branch or on a current release?

Copy link
Contributor Author

@to24toro to24toro Jul 21, 2023

Choose a reason for hiding this comment

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

Oh, sorry I misunderstood it. We cannot change this type hint.
Anyway, DynamicsBackend would never work if it has the same architecture as backendV2. I haven't solved this issue in the current release 0.25 because I found other bugs in the current code in terra. However, I am not sure if DynamicsBackend would not work in fact because for example, DynamicsBackend enable to measure all qubits independently which is different from the real ibm backend.

p.s.
Scheduling with backendV2 needs meas_map and DynamicsBackend has to have meas_map. (#248 (comment))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I fixed at bf2cdc5 once.

@DanPuzzuoli
Copy link
Collaborator

The only other thing I'm wondering with this PR is, now that we are removing BackendV2 from the type hints, should add a line to the from_backend doc string that this will work for BackendV2 if it has a configuration and defaults. Just as we don't want users to think this will work for all BackendV2, causing confusion, we also want them to be aware that it will work with the ones that have configuration and defaults for backwards compatibility.

@DanPuzzuoli DanPuzzuoli merged commit b59336d into qiskit-community:main Aug 2, 2023
DanPuzzuoli added a commit to DanPuzzuoli/qiskit-dynamics that referenced this pull request Aug 2, 2023
DanPuzzuoli added a commit that referenced this pull request Aug 8, 2023
* adding sympy as an explicit requirement (#233)

* Add warning for JAX versions on import of Dynamics (#232)

* Add probability normalization to DynamicsBackend sampling routine (#239)

* Add warning if digital carrier exceeds Nyquist frequency in pulse -> signal conversion (#242)

* Fix bug with carrier_freq being a JAX tracer if envelope is constant in Signal (#247)

* Remove subsystem_labels option from DynamicsBackend (#248)

* Rename subsystem_dims Dict to subsystem_dims_dict (#250)

* Fix ClassicalRegister counting in DynamicsBackend (#252)

* temporary fix for docs (#253)

* Drop support for backendV2 in from_backend (#249)

* incrementing version number

* cleaning up reno files

---------

Co-authored-by: Kento Ueda <[email protected]>
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.

DynamicsBackend.form_backend fails using backendV2
2 participants