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

Remove backend cache from QiskitRuntimeService #1732

Merged
merged 10 commits into from
Jun 11, 2024

Conversation

nkanazawa1989
Copy link
Contributor

@nkanazawa1989 nkanazawa1989 commented Jun 7, 2024

Summary

In #1715 we implemented new pattern to retrieve backend from the service, namely, opt-int. With opt-in, the same backend can have different Target, and thus they must be different Python objects.

This PR removes backend instance cache mechanism from QiskitRuntimeService. Instead, cache for the configuration data is implemented in the RuntimeClient. IBMBackend object is created every time it is requested, but the source data is cached and the communication with the server occurs only once.

Details and comments

Once after we announce the fractional gate opt-in (not limited to fractional, some future opt-ins), user may try to compare the performance of our system with and without opt-in. One may write

backend_with_fg = service.backend("my_backend", use_fractional_gates=True)
backend_without_fg = service.backend("my_backend", use_fractional_gates=False)

# Run our system with fractional gates
sampler = SamplerV2(backend_with_fg)
isa_circ = transpile(qc, backend_with_fg)
result_with_fg = sampler.run(isa_circ).result()

# Run our system without fractional gates
sampler = SamplerV2(backend_without_fg)
isa_circ = transpile(qc, backend_without_fg)
result_without_fg = sampler.run(isa_circ).result()

Note that this does NOT work as expected BUT doesn't raise any warning or error. Note that backend_with_fg is silently mutated at the second retrieval via the cache, and both sampler jobs run without fractional gates. One may wonder why two results look comparable.

This PR will remove this complication and make internal code much simpler.

@coveralls
Copy link

coveralls commented Jun 7, 2024

Pull Request Test Coverage Report for Build 9412509945

Details

  • 40 of 47 (85.11%) changed or added relevant lines in 1 file are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.07%) to 82.289%

Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit_ibm_runtime/qiskit_runtime_service.py 40 47 85.11%
Files with Coverage Reduction New Missed Lines %
qiskit_ibm_runtime/qiskit_runtime_service.py 2 77.07%
Totals Coverage Status
Change from base Build 9402463581: -0.07%
Covered Lines: 6017
Relevant Lines: 7312

💛 - Coveralls

@jyu00
Copy link
Collaborator

jyu00 commented Jun 7, 2024

I think there is value in caching the backend data, so we don't need to call the server again to retrieve the same exactly data just to build a target that with different basis gates.

Having said that, the existing caching code is just too complicated, and this PR is a good opportunity to clean it up. A follow up improvement can be a "backend cache" that only stores server data (backend config etc), andBackend objects are dynamically created with different target, instance, etc but use the same server data.

@coveralls
Copy link

coveralls commented Jun 10, 2024

Pull Request Test Coverage Report for Build 9443084694

Details

  • 41 of 42 (97.62%) changed or added relevant lines in 1 file are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.002%) to 82.352%

Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit_ibm_runtime/qiskit_runtime_service.py 41 42 97.62%
Files with Coverage Reduction New Missed Lines %
qiskit_ibm_runtime/qiskit_runtime_service.py 2 78.2%
Totals Coverage Status
Change from base Build 9402463581: -0.002%
Covered Lines: 6015
Relevant Lines: 7304

💛 - Coveralls

@nkanazawa1989
Copy link
Contributor Author

nkanazawa1989 commented Jun 10, 2024

follow up improvement can be a "backend cache" that only stores server data (backend config etc), andBackend objects are dynamically created with different target, instance, etc but use the same server data.

Done in deb6cb6. Cache code is now very simple.

Edit:

Some numbers from benchmark. On my laptop

  • First backend retrieval 1.22 sec
  • Second retrieval 1.6 ms

Of course it depends on internet bandwidth, but seems like this simple cache is sufficient.

@coveralls
Copy link

coveralls commented Jun 10, 2024

Pull Request Test Coverage Report for Build 9445194300

Details

  • 42 of 46 (91.3%) changed or added relevant lines in 2 files are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.02%) to 82.332%

Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit_ibm_runtime/qiskit_runtime_service.py 41 42 97.62%
qiskit_ibm_runtime/api/clients/runtime.py 1 4 25.0%
Files with Coverage Reduction New Missed Lines %
qiskit_ibm_runtime/qiskit_runtime_service.py 2 78.2%
Totals Coverage Status
Change from base Build 9402463581: -0.02%
Covered Lines: 6016
Relevant Lines: 7307

💛 - Coveralls

@jyu00 jyu00 mentioned this pull request Jun 10, 2024
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 10, 2024

Actually, one more comment - Both backend.properties() and backend.defaults() currently have a refresh keyword that says "If True, re-query the server for the backend properties/defaults. Otherwise, return a cached version.". Since we only cache configuration now, those need to be deprecated and later removed, unless you want to also add caching for properties/defaults.

@nkanazawa1989
Copy link
Contributor Author

Since we only cache configuration now, those need to be deprecated and later removed, unless you want to also add caching for properties/defaults.

I think this behavior should be okey because instance-wise these information are cached. Maybe I should write upgrade note.

@coveralls
Copy link

coveralls commented Jun 11, 2024

Pull Request Test Coverage Report for Build 9459880776

Details

  • 42 of 46 (91.3%) changed or added relevant lines in 2 files are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.02%) to 82.387%

Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit_ibm_runtime/qiskit_runtime_service.py 41 42 97.62%
qiskit_ibm_runtime/api/clients/runtime.py 1 4 25.0%
Files with Coverage Reduction New Missed Lines %
qiskit_ibm_runtime/qiskit_runtime_service.py 2 78.2%
Totals Coverage Status
Change from base Build 9452478202: -0.02%
Covered Lines: 6048
Relevant Lines: 7341

💛 - Coveralls

@jyu00 jyu00 merged commit e0aed70 into Qiskit:main Jun 11, 2024
20 checks passed
@coveralls
Copy link

coveralls commented Jun 11, 2024

Pull Request Test Coverage Report for Build 9465140454

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 42 of 46 (91.3%) changed or added relevant lines in 2 files are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.02%) to 82.387%

Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit_ibm_runtime/qiskit_runtime_service.py 41 42 97.62%
qiskit_ibm_runtime/api/clients/runtime.py 1 4 25.0%
Files with Coverage Reduction New Missed Lines %
qiskit_ibm_runtime/qiskit_runtime_service.py 2 78.2%
Totals Coverage Status
Change from base Build 9452478202: -0.02%
Covered Lines: 6048
Relevant Lines: 7341

💛 - Coveralls

kt474 added a commit to kt474/qiskit-ibm-runtime that referenced this pull request Jun 11, 2024
@kt474 kt474 added the Changelog: New Feature Include in the Added section of the changelog label Jun 11, 2024
kt474 added a commit that referenced this pull request Jun 11, 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.

4 participants