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

Migration guide for providers to implement V2 primitives #2496

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

ElePT
Copy link
Collaborator

@ElePT ElePT commented Dec 17, 2024

This is a proposal for content to address #2469. After a brief discussion with @jyu00 we agreed that we should focus this migration guide on showing how to implement new primitives for providers. However, I have not been involved in all discussions regarding the vision for this migration (and conversations around backend.run tend to raise a lot of different opinions) so I would appreciate confirmation that the content proposed fits the direction we want to show.

The page is currently in a pretty raw state, missing formatting, API ref links, a review on the titles, capitalization, etc, but I wanted to open a PR as fast as possible to ask for opinions on including the backend primitive wrappers as a tool for provider migration. This idea came from inspecting the only instance I know of external provider beyond the immediate product sphere, which is https://github.com/qiskit-community/qiskit-aqt-provider/tree/master, and subclasses backend primitives to expose their own primitive implementations. Following this, I decided to structure the page as follows:

  • motivation: @jyu00 mentioned this was often requested by users, I tried to keep it short and touch on the usefulness of the abstraction level of the primitives
  • Backend primitives as a migration alternative for providers that already implemented backend.run (following the AQT example)
  • Custom primitives as an alternative for providers that want it, or didn't implement backend.run (following the StatevectorSampler and StatevectorEstimator) examples

Please let me know if you have any other take on how the content should look like @javabster, @jyu00.

@qiskit-bot
Copy link
Contributor

One or more of the following people are relevant to this code:

@ElePT ElePT changed the title [WIP] Migration guide for providers to implement V2 primitives Migration guide for providers to implement V2 primitives Dec 18, 2024
# Migrate provider interfaces from `backend.run` to primitives

## Motivation and context

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the very first paragraph should mention whom this migration guide is for, since it'd be irrelevant to the majority of people that consume IQP docs.

docs/migration-guides/external-providers-primitives-v2.mdx Outdated Show resolved Hide resolved
docs/migration-guides/external-providers-primitives-v2.mdx Outdated Show resolved Hide resolved
docs/migration-guides/external-providers-primitives-v2.mdx Outdated Show resolved Hide resolved
(for example: `execute_workload(QPU)` or `resource.access()`) or local simulator,
as long as the final inputs and outputs conform to the established standards set by the primitive interfaces.

## If your provider already implemented `backend.run`
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm a bit confused by this section... this reads like the target audience is people who use 3rd party providers, and not the providers themselves (e.g. users of IonQProvider and not IonQ itself).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I am not too set on having this section, to be honest, that's why I mentioned in the PR description that I wanted a second opinion on this approach. I based it on the only kind of external (still mostly IBMers) provider that I know implemented primitives, that is: https://github.com/qiskit-community/qiskit-aqt-provider/tree/master. They based their primitives on the backend primitives as shown in this example, and I thought it might be a good shortcut for other providers too, but I also think it might be too hacky. If you don't think it adds any value, it could be removed.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we can point out backend primitives as an example, but don't explicitly say they should use it. It's also to their benefit to optimize for their hardware instead of using an out-of-box implementation.

This section would actually be quite helpful in the existing backend.run migration guide. If, for example, someone is using backend.run across Qiskit Runtime + Provider Foo and want to migrate to primitives. They can use this as an example to write to primitives interface even if Foo hasn't migrated yet.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I found this page https://docs.quantum.ibm.com/open-source/create-a-provider#providing-an-interface-for-execution, which is a simpler version of what I had in mind. I think we just need to add more "meat" to it, like saying how the workflow is likely

  • validation, such as checking the options are valid
  • pre-processing, such as applying any error suppression techniques you support, and broadcasting the inputs
  • execution to get counts
  • post-processing, such as calculating the exp val and applying error mit

Copy link
Collaborator Author

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

So based on @jyu00's review, it looks like the way forward with this PR will involve the following:

  1. Moving the current If your provider already implemented backend.run section (L40-93) to the qiskit runtime backend.run migration guide and renaming it accordingly. This would address Add to backend.run migration guide for 3rd party provider #2543.

  2. Keeping the If your provider didn't implement backend.run or you prefer a fully custom implementation section as the main example of the guide (renaming it accordingly), and potentially updating the code to match @jyu00's expectations. I took a look at https://docs.quantum.ibm.com/open-source/create-a-provider#providing-an-interface-for-execution and it's essentially a more lightweight version of the example I added. It does look better, but the moment we want to show some useful reference for input/output pre-post processing it gets ugly, so I think that is unavoidable. The classes can of course be renamed. The only element I see in that example compared to the proposed example in this page is the __init__ method. So I would say the action points are:

  1. As for the introduction/motivation section, the only feedback that hasn't been applied yet is the one in Migration guide for providers to implement V2 primitives #2496 (comment). I personally like the mention of other migration guides in the introduction, but if you think it will substantially improve the content then I am not so strongly against it.

If someone from the docs team can take over points 1 and 3, I could try to carve out some time next week to polish the code example from point 2.

@abbycross
Copy link
Collaborator

If someone from the docs team can take over points 1 and 3, I could try to carve out some time next week to polish the code example from point 2.

Sure we can do that! - FYI @beckykd

@beckykd
Copy link
Collaborator

beckykd commented Jan 21, 2025

  1. Moving the current If your provider already implemented backend.run section (L40-93) to the qiskit runtime backend.run migration guide and renaming it accordingly.

Done

@beckykd
Copy link
Collaborator

beckykd commented Jan 21, 2025

3. As for the introduction/motivation section, the only feedback that hasn't been applied yet

I went ahead and moved the links down. I think it'll be easier for admins to find and share if it's in a separate section.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

6 participants