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

qiskit.algorithms move as an independent package #44

Merged
merged 8 commits into from
Aug 31, 2023

Conversation

ElePT
Copy link
Collaborator

@ElePT ElePT commented Jun 27, 2023

In an attempt to inform and coordinate the move of the algorithms repo, I have quickly pulled together an RFC.

Copy link

@Eric-Arellano Eric-Arellano left a comment

Choose a reason for hiding this comment

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

Thanks! Where will the docs move to? qiskit.org/ecosystem/algorithms? That name would match how we have e.g. qiskit-nature at ecosystem/nature.

FYI we can set up redirects in cloudflare from the qiskit.org/documentation/ subpages to ecosystem/algorithms if we want.

@ElePT
Copy link
Collaborator Author

ElePT commented Jun 27, 2023

Thanks! Where will the docs move to? qiskit.org/ecosystem/algorithms? That name would match how we have e.g. qiskit-nature at ecosystem/nature.

FYI we can set up redirects in cloudflare from the qiskit.org/documentation/ subpages to ecosystem/algorithms if we want.

Exactly, I was thinking of qiskit.org/ecosystem/algorithms. And I was just about to add the docs redirect idea to the RFC when I read your comment, I think it makes a lot of sense!

Copy link

@Eric-Arellano Eric-Arellano left a comment

Choose a reason for hiding this comment

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

The docs plan looks good to me.

Please wait for other reviewers before merging.

Copy link
Member

@kdk kdk left a comment

Choose a reason for hiding this comment

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

Following discussion in the team meeting today, I'm good with path 1.

Comment on lines 101 to 103
2. Add a dependency of `qiskit_algorithms` to qiskit-terra so that installing terra will also install the
algorithms until the deprecation period is over. [Blocker: the separate algorithms package would have to
be ready and be published ahead of terra releasing]
Copy link
Member

Choose a reason for hiding this comment

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

If this dependency is added to Terra, I'd recommend pinning the version using the ~= compatible release clause or its equivalent.

ElePT and others added 2 commits August 22, 2023 18:30
@1ucian0 1ucian0 changed the title Algorithms Move RFC qiskit.algorithms move as an independent package Aug 28, 2023
@1ucian0 1ucian0 added the RFC proposal New RFC is proposed label Aug 28, 2023
wshanks added a commit to wshanks/qiskit-terra-feedstock that referenced this pull request Aug 29, 2023
* All of the old build configuration was moved under the qiskit-terra output
* The qiskit output installs the separate package under qiskit_pkg in
  the source code.
  - For the moment, the qiskit and qiskit-terra package versions and
    build numbers do not match, but with version 0.45, they should be
synchronized.
  - The qiskit output could be noarch if it were possible for conda to
    support multiple outputs with an arch-specific subpackage and a
noarch package that depends on it.
* The qiskit.algorithms test was removed as qiskit.algorithms  is moving
  to a separate package. See Qiskit/RFCs#44.
from `qiskit.algorithms` to `qiskit_algorithms`. The code will continue existing
in `qiskit-terra` until the deprecation period is over [Blocker: release of `qiskit_algorithms-0.1.0`,
see below].
2. What should we do with the algorithms migration guide? we could leave it in terra during the deprecation period,
Copy link
Member

Choose a reason for hiding this comment

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

Migration guides usually stay online after the deprecation period.

Copy link
Collaborator Author

@ElePT ElePT Aug 30, 2023

Choose a reason for hiding this comment

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

This particular guide is for migrating from QuantumInstance-based qiskit.algorithms to primitive-based qiskit.algorithms. I am all for keeping guides for as long as possible, but I am not sure how to balance this with "completely decoupling qiskit.algorithms from qiskit" efforts. That is why we suggested to move this guide to the qiskit-algorithms repository (but keep the opflow/quantum instance ones in qiskit). It would stay alive but decoupled. This is also consistent with feedback that we are getting on the qiskit-ibm-runtime docs to remove qiskit.algorithms from migration guides (see Qiskit/qiskit-ibm-runtime#1032)

Copy link
Member

Choose a reason for hiding this comment

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

At least when I remember discussing this a while ago when @woodsp-ibm brought this up the thinking was that we'll have the stable docs builds for historical releases so we can just point people to: https://qiskit.org/documentation/stable/0.44/migration_guides/algorithms_migration.html and not worry about having to keep the guides in source form forever.

Copy link
Member

Choose a reason for hiding this comment

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

As we do not have older versions of docs published in the apps what we have been doing there, once a migration guide no longer can be run/built from main is to convert it as a rst file so it gets included but its content is no longer executed. I had been thinking that if the guide was wanted to be kept/publihsed longer in Qiskit that potentially in main its contents just got replaced by a link to the stable published version where it existed since the older versions of the docs were kept. That seemed simpler to me and also being a link one could state that the deprecation period was over or whatever other text you wanted there.

@mtreinish mtreinish self-assigned this Aug 30, 2023
Copy link
Member

@mtreinish mtreinish left a comment

Choose a reason for hiding this comment

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

With the exception of one copy paste error inline I think this is fine to merge. There are still some small questions/musings and steps that diverged a bit from what we implemented but that's fine for an rfc as it's a document of a plan not necessarily everything that was implemented. But not worth blocking or changing at this point since the rfc is basically all implemented at this point

0012-algorithms-migration.md Outdated Show resolved Hide resolved
@1ucian0 1ucian0 merged commit a6e4ebe into Qiskit:master Aug 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFC proposal New RFC is proposed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants