Skip to content

Conversation

kt474
Copy link
Member

@kt474 kt474 commented Apr 25, 2025

Summary

Documented changes in 2239.feat.rst

Details and comments

Fixes #2236
Fixes #2237
Fixes #2147

@ElePT ElePT added this to the 0.39.0 milestone May 6, 2025
Copy link
Collaborator

@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.

Thanks @kt474, I took an initial look at the PR and I see that some details need to be ironed out and in general is a tricky feature to develop. I left a few comments here and there (some really minor), but I think that my main points are:

  1. The documentation of the "ibm_cloud" channel should stay while the option is supported, even if it's not the recommended one. We should extend the documentation with the new "ibm_quantum_platform" channel, make sure the new name is used in the examples and deprecation message, and mention that it's the new default and follows the same configuration path as "ibm_cloud". After the alternative is in place, we can deprecate "ibm_cloud" explicitly. The new name is too close to "ibm_quantum" and many users will naively assume that it is replacing it making the migration even more complex.

  2. Regarding your 2 questions from the PR description:

  • Should service.backends() return all backends available through all instances?
  • Fetching all instances then fetching backends for every instance is very slow (ntc 5779 would help)

My take is that with any change that we make that adds API calls, we should make sure users are not hit with a performance regression if they aren't getting anything out of it (so if you really need this functionality, sure, you might be willing to sacrifice speed, but some users won't need it). So I would only really fetch all instances if no instance or default_instance is set, and would only iterate over all instances and backends in that case. Also brainstorming here, this might be a good opportunity to try async API calls, which might mitigate the performance hit a bit?

Copy link
Collaborator

@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.

Sharing some conclusions from our internal discussion on how to advance with this PR:

  1. We will not add default_instance, and use instance instead. We will document the parameter in save_account as: “This is an optional parameter. If set, it will define a default instance for service instantiation, if not set, the service will fetch all instances related to a specific token” (or something along these lines). We will add this change in behavior to the reno, mentioning that it’s now optional and that not setting it gives access to all instances. We will also let the docs team know about the change.
  2. We will add the following new optional parameters to both save_account and __init__ : account , region , plans_preference. Where if the value is set in init it will overwrite the default from save_account.
  3. If no instance is set during init, to do authentication, we can call IBM Cloud platform APIs to get all the service instances the user has access to

@mtreinish
Copy link
Member

So I'm playing with this locally and some observations, right now if you do:

QiskitRuntimeService.save_account(channel="ibm_quantum_platform", token="...")

it saves an ibm_cloud channel account in the json file. Then when you instantiate the service from the saved credentials it loads up as a ibm_cloud channel account and not ibm_quantum_platform. This causes a warning to be printed when you load the account because there is no instance set QiskitRuntimeService(). I kind of expected this to just all work by default and show all the instances I have since I didn't set a specific one.

@kt474 kt474 modified the milestones: 0.39.0, 0.40.0 May 15, 2025
@ElePT ElePT force-pushed the iqp-instance-account-options branch from bd1d2dd to cb3910a Compare May 27, 2025 15:25
@ElePT ElePT changed the title WIP Add new channel and allow instance selection with new platform Add new channel and allow instance selection with new platform May 27, 2025
@ElePT ElePT marked this pull request as ready for review May 27, 2025 17:34
@ElePT ElePT mentioned this pull request May 28, 2025
ElePT
ElePT previously approved these changes May 28, 2025
Copy link
Collaborator

@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.

I believe that the PR is finally in a mergeable state, thanks for the effort @kt474. The service seems to work as I expect it to, at least in terms of instance selection, prioritization and backend retrieval, retrieving former jobs and sessions, and erroring on wrong input combinations.

I would like to see a greater test coverage of these new changes, especially in integration tests, as well as better documentation. But realistically we cannot block the PR on these, and they can be improved gradually independently of the release schedule.

@ElePT ElePT enabled auto-merge May 28, 2025 14:29
@kt474 kt474 disabled auto-merge May 28, 2025 15:05
@kt474 kt474 merged commit 5088124 into Qiskit:main May 28, 2025
17 checks passed
@kt474 kt474 added the Changelog: New Feature Include in the Added section of the changelog label May 28, 2025
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.

List service instances in the new IBM Quantum Platform Selection of instances in the new IBM Quantum Platform New IQP API integration
3 participants