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

Fractional gate opt-in #1715

Merged
merged 11 commits into from
Jun 6, 2024
Merged

Fractional gate opt-in #1715

merged 11 commits into from
Jun 6, 2024

Conversation

nkanazawa1989
Copy link
Contributor

@nkanazawa1989 nkanazawa1989 commented May 30, 2024

Summary

This PR adds the fractional gate opt-in feature. In IBM backend architecture, dynamic circuits and fractional gates will be exclusively supported. However, Qiskit Target model doesn't express such a constraint. This means one must create two target instances, namely, one only with the fractional gate instructions and the other one only with the control flow instructions. The choice of target is the responsibility of the end users.

This opt-in might be removed without announce when IBM backend architecture changes.

Details and comments

This PR is draft to conclude the final design.

  • API (fixed)

Both QiskitRuntimeService.backends() and QiskitRuntimeService.backend() gains use_fractional_gate argument. User needs to set True to include fractional gates in the backend target.

  • Mutability (need help)

In the current implementation, the service instance pre caches all backends associated with the account. This has been reasonable design. However, with the new flag, the same backend will have two instances with/without fractional gates/control flow instructions in their target. Change of the flag will silently mutate the cached backend. For example,

backend_dc = service.backends("some_backend", use_fractional_gates=False)
backend_fg = service.backends("some_backend", use_fractional_gates=True)

qc = QuantumCircuit(...)
qc.if_else(...)

isa_qc = transpile(qc, backend_dc)  # This fails because id(backend_dc) == id(backend_fg)

The easiest solution is to deepcopy the instance before returning it to users (target is created when the end user calls any attribute from the target for the first time). We don't like deepcopy though. Alternatively we can remove this cache and always call _create_backend_obj when the backend is retrieved. This maybe increase REST GET call overhead to get configuration data from server, but makes internal code simpler (and might reduce memory footprint if retrieved backend is GC'd). If the user doesn't retrieve the same backend multiple times there is no overhead with this change.

  • Breaking API change (need help)

We set default of use_fractional_gates to True because dynamic circuit seems not being heavily used. However this is indeed a breaking API change for end users because it silently mutates basis gates and changes the resulting isa circuits, and maybe fidelity too. We can also start with False, then turn it on with deprecation warning if we want. From the API break viewpoint, another concern is a user can still use qiskit-ibm-runtime before this commit even after our backends start to report fractional basis gates. This will create a target with mixture of fractional and control flow instructions, which will create invalid isa circuits when it includes any control flow. In the current version there is no warning mechanism in the client software, which might frustrate end users using outdated runtime.

TODO

  • Write unittest
  • Write release note

@coveralls
Copy link

coveralls commented May 31, 2024

Pull Request Test Coverage Report for Build 9401855600

Details

  • 32 of 33 (96.97%) changed or added relevant lines in 6 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.08%) to 82.354%

Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit_ibm_runtime/qiskit_runtime_service.py 4 5 80.0%
Files with Coverage Reduction New Missed Lines %
qiskit_ibm_runtime/utils/backend_converter.py 1 79.59%
Totals Coverage Status
Change from base Build 9389721792: 0.08%
Covered Lines: 6039
Relevant Lines: 7333

💛 - Coveralls

@nkanazawa1989
Copy link
Contributor Author

(clue for review)

I added unit tests in three steps:

  • b9ac43b: Test for convert_to_target function is added. This is the core feature of the PR. I also modified the interface so that we can individually select dynamic and fractional feature. When our backends support both, we can just set True in both arguments without any API change. Note that FakeBackends already do this, because the simulator backend doesn't have constraints. In addition, if we disable dynamic feature, this could break other unit tests (including community package) for dynamic circuits relying on our fake backends.

  • 736e060: Test for backends(..., use_fractional_gates=True) is added. This requires modification for the test backend, so that the fake runtime client can refer to the actual model data of fake backends by backend names (before this commit every backend uses FakeLima).

  • 7453ce3: Test to verify Sampler.run can reject un-executable pubs on our backends.

Copy link
Collaborator

@jyu00 jyu00 left a comment

Choose a reason for hiding this comment

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

Some relatively minor comments, but overall looks good!

qiskit_ibm_runtime/qiskit_runtime_service.py Outdated Show resolved Hide resolved
qiskit_ibm_runtime/qiskit_runtime_service.py Show resolved Hide resolved
release-notes/unreleased/1715.feat.rst Show resolved Hide resolved
qiskit_ibm_runtime/fake_provider/__init__.py Outdated Show resolved Hide resolved
qiskit_ibm_runtime/fake_provider/__init__.py Outdated Show resolved Hide resolved
Co-authored-by: Jessie Yu <[email protected]>
@nkanazawa1989
Copy link
Contributor Author

Thanks @jyu00 I updated the PR. Do you have any comment on the rest of two topics in the PR comment?: Mutability and Breaking API change

@yaelbh
Copy link
Collaborator

yaelbh commented Jun 2, 2024

Do you want to add checks at the client side that PEC and PEA are not requested along with fractional gates?

nkanazawa1989 and others added 2 commits June 3, 2024 13:00
Copy link
Collaborator

@jyu00 jyu00 left a comment

Choose a reason for hiding this comment

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

LGTM!

@jyu00
Copy link
Collaborator

jyu00 commented Jun 3, 2024

Do you want to add checks at the client side that PEC and PEA are not requested along with fractional gates?

Doing it on the client side has the disadvantage that down level client code would become stale when updates are made on the server side (e.g. if/when we support fractional gates with dynamic circuits). So I was really hoping the validation service will be setup in time, although that's not looking likely. Regardless, I'd highly recommend doing the check in a separate PR, if we don't want to wait on the validation service.

@jyu00 jyu00 merged commit 6479813 into Qiskit:main Jun 6, 2024
20 checks passed
@kt474 kt474 added the Changelog: New Feature Include in the Added section of the changelog label Jun 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: New Feature Include in the Added section of the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants