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

Backend converter bugfix (edge case) #11609

Merged
merged 2 commits into from
Jan 23, 2024

Conversation

nkanazawa1989
Copy link
Contributor

Summary

Backend V2 converter raises unexpected error. This is an example code to reproduce the bug.

backend = SomeBackendV1()  # Some backend includes calibration for sx
del backend._properties._gate["sx"]

backend_v2 = BackendV2Converter(backend)
backend_v2.target  # This causes error
TypeError: argument of type 'NoneType' is not iterable

Details and comments

Qiskit Experiments relies on FakeOpenPulse2Q to build own test backend. This backend is defective as it contains calibration for u1, u2, u3, and cx but properties are defined only for u1, u3, cx. This configuration unintentionally reproduces the same situation as written above. The target converter is upgraded to handle this edge case. Missing information in FakeOpenPulse2Q is also added.

@nkanazawa1989 nkanazawa1989 added Changelog: Bugfix Include in the "Fixed" section of the changelog mod: fake_provider Related to the fake_provider module and fake backends labels Jan 20, 2024
@nkanazawa1989 nkanazawa1989 requested review from jyu00 and a team as code owners January 20, 2024 04:22
@qiskit-bot
Copy link
Collaborator

One or more of the the following people are requested to review this:

  • @Qiskit/terra-core
  • @nkanazawa1989

@coveralls
Copy link

Pull Request Test Coverage Report for Build 7592109423

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.04%) to 89.505%

Totals Coverage Status
Change from base Build 7586004387: 0.04%
Covered Lines: 59477
Relevant Lines: 66451

💛 - Coveralls

Copy link
Contributor

@ElePT ElePT left a comment

Choose a reason for hiding this comment

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

This is an interesting edge case I had not thought about before. I think it's good to have it documented in the test comment and release note. LGTM. Only one question @nkanazawa1989, is this something we would like to backport? thinking especially of the qiskit-experiments dependencies, if we don't backport, this fix will only be available from qiskit 1.0 onwards.

@wshanks
Copy link
Contributor

wshanks commented Jan 23, 2024

It would not hurt anything to backport the part adding the extra gates to the FakeOpenPulse2Qproperties, but it is probably not a big deal (it probably isn't used outside of Qiskit and qiskit-experiments?). The qiskit-experiments tests using FakeOpenPulse2Q (converting it to BackendV2) only fail with Qiskit main, not 0.45. I think there have been some other changes to the backend conversion code that triggered the issue, so backporting the fix here would only make sense if backporting the earlier changes.

With this change, the qiskit-experiments tests pass. I was a little worried that they wouldn't because in qiskit-experiments we do a little bit of hacking to make up for the missing property information already and I thought that might conflict with filling in the information here.

@nkanazawa1989
Copy link
Contributor Author

Thanks @ElePT @wshanks . I'll merge the PR. I don't think the backport is really necessary unless some user writes a bug report issue about this. In practice we don't report calibration without characterizing the gate, so gate property must always exist.

@nkanazawa1989 nkanazawa1989 added this pull request to the merge queue Jan 23, 2024
Merged via the queue into Qiskit:main with commit 9d43757 Jan 23, 2024
12 checks passed
@wshanks
Copy link
Contributor

wshanks commented Feb 13, 2024

https://github.com/Mergifyio backport stable/0.46

Copy link
Contributor

mergify bot commented Feb 13, 2024

backport stable/0.46

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request Feb 13, 2024
* Bug fix backend converter

* Add upgrade note

(cherry picked from commit 9d43757)

# Conflicts:
#	qiskit/providers/backend_compat.py
wshanks pushed a commit that referenced this pull request Feb 13, 2024
…#11609)

(cherry picked from commit 9d43757 with
the backend_compat changes removed)
github-merge-queue bot pushed a commit that referenced this pull request Feb 13, 2024
…#11609) (#11788)

(cherry picked from commit 9d43757 with
the backend_compat changes removed)

Co-authored-by: Naoki Kanazawa <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: Bugfix Include in the "Fixed" section of the changelog mod: fake_provider Related to the fake_provider module and fake backends
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants