Remove backend cache from QiskitRuntimeService

nkanazawa1989 commented 3 months ago

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 commented 3 months ago

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%
<!--	Total:	40	47	85.11%	-->

Files with Coverage Reduction	New Missed Lines	%
qiskit_ibm_runtime/qiskit_runtime_service.py	2	77.07%
<!--	Total:	2		-->

Totals
Change from base Build 9402463581:	-0.07%
Covered Lines:	6017
Relevant Lines:	7312

💛 - Coveralls

jyu00 commented 3 months ago

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 commented 3 months ago

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%
<!--	Total:	41	42	97.62%	-->

Files with Coverage Reduction	New Missed Lines	%
qiskit_ibm_runtime/qiskit_runtime_service.py	2	78.2%
<!--	Total:	2		-->

Totals
Change from base Build 9402463581:	-0.002%
Covered Lines:	6015
Relevant Lines:	7304

💛 - Coveralls

nkanazawa1989 commented 3 months ago

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 commented 3 months ago

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%
<!--	Total:	42	46	91.3%	-->

Files with Coverage Reduction	New Missed Lines	%
qiskit_ibm_runtime/qiskit_runtime_service.py	2	78.2%
<!--	Total:	2		-->

Totals
Change from base Build 9402463581:	-0.02%
Covered Lines:	6016
Relevant Lines:	7307

💛 - Coveralls

jyu00 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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%
<!--	Total:	42	46	91.3%	-->

Files with Coverage Reduction	New Missed Lines	%
qiskit_ibm_runtime/qiskit_runtime_service.py	2	78.2%
<!--	Total:	2		-->

Totals
Change from base Build 9452478202:	-0.02%
Covered Lines:	6048
Relevant Lines:	7341

💛 - Coveralls

coveralls commented 3 months ago

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.

For more information on this, see Tracking coverage changes with pull request builds.
To avoid this issue with future PRs, see these Recommended CI Configurations.
For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

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%
<!--	Total:	42	46	91.3%	-->

Files with Coverage Reduction	New Missed Lines	%
qiskit_ibm_runtime/qiskit_runtime_service.py	2	78.2%
<!--	Total:	2		-->

Totals
Change from base Build 9452478202:	-0.02%
Covered Lines:	6048
Relevant Lines:	7341

Qiskit / qiskit-ibm-runtime