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

Fixed calibration experiments failing on backends with no coupling map #1117

Merged
merged 3 commits into from
Apr 18, 2023

Conversation

wshanks
Copy link
Collaborator

@wshanks wshanks commented Mar 30, 2023

On backends with no coupling map (such as one could configure with DynamicsBackend from qiskit-dynamics), calibration experiments failed because the BaseCalibrationExperiment tried to pass the backend's coupling_map (run through BackendData) to terra's CouplingMap which acceprts None. The transpiler pass FullAncillaAllocation behaves differently when passed None than when passed CouplingMap(None).

On backends with no coupling map (such as one could configure with
`DynamicsBackend` from qiskit-dynamics), calibration experiments failed
because the `BaseCalibrationExperiment` tried to pass the backend's
`coupling_map` (run through `BackendData`) to terra's `CouplingMap`
which acceprts `None`. The transpiler pass `FullAncillaAllocation`
behaves differently when passed `None` than when passed
`CouplingMap(None)`.
@wshanks
Copy link
Collaborator Author

wshanks commented Mar 30, 2023

Closes #1116

@wshanks wshanks requested a review from eggerdj March 30, 2023 20:53
@wshanks wshanks added backport stable potential The issue or PR might be minimal and/or import enough to backport to stable Changelog: Bugfix Include in the "Fixed" section of the changelog labels Mar 30, 2023
Copy link
Contributor

@eggerdj eggerdj left a comment

Choose a reason for hiding this comment

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

This looks good. Is it possible to setup a small test instance for a backend that does not have a coupling map? Can we maybe use this?

from qiskit_experiments.test.pulse_backend import SingleTransmonTestBackend
from qiskit_experiments.framework import BackendData

data = BackendData(SingleTransmonTestBackend())
data.coupling_map

Note that the resulting coupling map here is [] and not None. Should the code also distinguish [] from None?

Copy link
Collaborator

@nkanazawa1989 nkanazawa1989 left a comment

Choose a reason for hiding this comment

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

What is blocking this PR? When I locally test against main branch (-eterra-main) I started to see some cal test failures. This makes me think we already have sufficient tests and we should merge this before 0.24rc1 is released (otherwise terra release will break CI again).

@wshanks
Copy link
Collaborator Author

wshanks commented Apr 13, 2023

What is blocking this PR?

I hadn't had a chance to look at the tests, but I think that is the only blocker.

@wshanks
Copy link
Collaborator Author

wshanks commented Apr 17, 2023

Okay, I added a test as a second commit. I confirmed that cherry-picking the test commit on main without the other commit with the fix fails during the SetLayout pass (the empty coupling map causes it not to expand with ancilla and then it fails when asserts that the highest qubit index in the circuit plus one is equal to the number of qubits in the circuit).

Copy link
Contributor

@eggerdj eggerdj left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@coruscating coruscating added this pull request to the merge queue Apr 18, 2023
Merged via the queue into qiskit-community:main with commit ac26da0 Apr 18, 2023
mergify bot pushed a commit that referenced this pull request Apr 18, 2023
#1117)

On backends with no coupling map (such as one could configure with
`DynamicsBackend` from qiskit-dynamics), calibration experiments failed
because the `BaseCalibrationExperiment` tried to pass the backend's
`coupling_map` (run through `BackendData`) to terra's `CouplingMap`
which acceprts `None`. The transpiler pass `FullAncillaAllocation`
behaves differently when passed `None` than when passed
`CouplingMap(None)`.

(cherry picked from commit ac26da0)
coruscating pushed a commit that referenced this pull request Apr 18, 2023
…p (backport #1117) (#1148)

This is an automatic backport of pull request #1117 done by
[Mergify](https://mergify.com).


Co-authored-by: Will Shanks <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport stable potential The issue or PR might be minimal and/or import enough to backport to stable Changelog: Bugfix Include in the "Fixed" section of the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants