Skip to content

Add migration guide on Qiskit packaging #710

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

Merged
merged 20 commits into from
Jan 31, 2024

Conversation

jakelishman
Copy link
Member

@jakelishman jakelishman commented Jan 25, 2024

Part of #704.

This is a pretty large guide on migration recommendations and troubleshooting around the Qiskit 1.0 release.

I'll ask the other Qiskit developers and a few others to have a look at this for technical content as well.

Given the length of this, I suspect it might be better as a stand-alone migration guide.

This is a pretty large guide on migration recommendations and
troubleshooting around the Qiskit 1.0 release.
@jakelishman
Copy link
Member Author

The spell-checker is being racist against me 🇬🇧 🇬🇧 🇬🇧 haha

(I'll fix that later.)

```

4. If you are not planning to use the environment immediately, use the `deactivate` command to leave it.

Copy link
Member

Choose a reason for hiding this comment

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

The only potential gotcha here for users that I think is good to call out here is compatibility with other dependencies that depend on qiskit-terra and/or qiskit<1. I think this will trip up users the most more than not being able to pip install -U qiskit because it's the piece that requires understanding how pip does dep solving or lack thereof and people doing things like python -m venv test && test/bin/pip install qiskit>=1.0 && test/bin/pip install qiskit-qubit-reuse (this is a good reminder I should update the plugin package to not depend on terra before 1.0). This will end up in a mixed installation state between qiskit 1.0.0 and qiskit-terra 0.46.0. I think this really should be called out in the top and not just in the troubleshooting guide below.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've thrown in an admonition warning about this, including a link to the troubleshooting section if people do get into problems in 75f6e3a. Do you think it's enough?

I don't necessarily want to get too pessimistic up at the top of the guide - if they're just using IBM tooling (which I imagine most users are), they're hopefully not going to have any issues by the time 1.0 is actually released.

Comment on lines 327 to 340
This section uses some Python-packaging jargon to better explain what was happening.
The following words have special meanings:

* *module*: A single Python file.

* *package*: A directory containing an `__init__.py` and other files or packages that Python can read.
This is the actual code as installed on your system, and what executes when you run `import something`.
Python considers any directory that is on the *search path* to be something you can import (and will import quite a few more things besides!).

This is not the same object that you `pip install` (which is a *distribution*), but commonly the thing you `pip install` and the thing you `import` will have the same name.

* *submodule*, *subpackage*: These are somewhat imprecise terms, but are commonly used.
The *sub* part just means "contained inside a package".
A *submodule* is just a module and a *subpackage* is just a package, but they are part of a larger *package*.
Copy link
Member Author

@jakelishman jakelishman Jan 26, 2024

Choose a reason for hiding this comment

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

I tried to make this a proper HTML <dl> definition list, but we don't seem to have styling for that, so it ends up very sad:

Screenshot 2024-01-26 at 00 30 32

The bullets here aren't ideal, but better than whatever ^ that is.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I can reformat this into a table if that would seem more appropriate? terms in the left column, defs in the right?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm happy with whatever styling you think would be best. My guess is that a table might look a bit unbalanced and hard to read by squeezing the "definition" paragraphs a bit much, but I don't know and I'm fine deferring to you guys in design anyway.

Copy link

@coruscating coruscating left a comment

Choose a reason for hiding this comment

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

I don't have any suggestions on the For developers section—it looks good.

Qiskit 1.0 includes breaking changes, and several packages will have marked themselves as not-yet-compatible with it, so you might see errors from `pip` when running this until they have new versions.
Old versions of packages may also depend on the legacy `qiskit-terra` package, which might not immediately error out during this command, but might raise an error when running `import qiskit`.

If you know of any packages that are not yet compatible with Qiskit 1.0, you will have to not install those yet, and wait until the packages are updated.

Choose a reason for hiding this comment

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

Perhaps here you can link to the section on how to use pipdeptree to find dependency versions.

Copy link
Member Author

Choose a reason for hiding this comment

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

I put in an extra comment and link in f4b16f0, though I avoided dropping an actual link to the specific troubleshooting section below, because that's all about recovering from a broken environment. I don't actually know a simple way to get pip (or pipdeptree) to tell you the requirements of a particular prospective package without attempting to install it (though maybe the experimental pip index will eventually do that).

@beckykd
Copy link
Collaborator

beckykd commented Jan 26, 2024

@javabster requested that this be split into two documents: One for devs and one for users. Would you like me to take a few minutes to do that?

@jakelishman
Copy link
Member Author

jakelishman commented Jan 26, 2024

Imo, the scope covered is logically grouped into either one (as here) or four (splitting each of the second-level headings onto its own page). In case it's not clear from the unrendered MDX, the page structure currently looks like:

  • For users
    • Creating the new environment
    • Using the new environment
  • For developers
    • Recommendations for requirements
    • Recommendations for testing against Qiskit 1.0
  • Why these changes happened
    • The old Qiskit structure
    • The new Qiskit structure
  • Troubleshooting
    • [first sort of error]
    • [second sort of error]
    • ...

My concern with splitting it in two ("for users" and "for developers") is that the other two sections don't fold neatly into either of those categories; they're both aimed at everyone. They also don't have a natural grouping that would put only them together, and not with the two "for x" sections.

If the concern is primarily the length: "why this happened?" is not meant to be required reading - it's like an extension - and "troubleshooting" is meant to be something you find by searching for error messages, not something read end-to-end. The page intro is meant to help with conveying that.

Personally I think the one-page form is the best for discoverability for readers, and grouping on the scale of "Qiskit Runtime / Qiskit 0.44 / Qiskit 1.0 / Qiskit 2.0" (the migration-guides sidebar of next year) given that there'll be at least one other Qiskit 1.0 migration guide to help with API changes, but I'm not going to oppose a final decision from your team - you guys are in charge of the documentation architecture and how it's presented.

@jakelishman
Copy link
Member Author

In case it helps, here's a rendered version of the current page above the fold:

Screenshot 2024-01-26 at 21 36 58

Copy link
Contributor

@wshanks wshanks left a comment

Choose a reason for hiding this comment

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

Very thorough! I don't know how many users will read everything here to start out but it will be nice to point to when they ask for help.

The Qiskit 1.0 side of the test also checks for sentinel files that were present in old Qiskit versions and not Qiskit 1.0.

If you are a Qiskit developer, it's possible that you have old `qiskit.egg-info` or `qiskit-terra.egg-info` (or `*.dist-info`) directories present on your meta path (see `sys.meta_path`), left over from old editable installations.
In particular, check your working directory for any `*.egg-info` and `*.dist-info` directories.
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know if anyone is reading this far down in the guide, but I think the problem could also be __pycache__ directories if you use pip install -e. You could suggest git clean -fxd after the user checks that there are no important uncommitted files.

Copy link
Member Author

Choose a reason for hiding this comment

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

__pycache__ shouldn't cause problems here, because the first importlib search is for distributions not packages, and then the second is a search for "is qiskit.tools a loadable module with a backing source file?". It's not enough for the qiskit/tools directory to be on the path, even if qiskit/tools/__pycache__/__init__.*.pyc exists, because we test that the ModuleSpec has a proper location for the file (qiskit/tools.py or qiskit/tools/__init__.py) to verify it's not an empty namespace package left over from a dirty source tree.

I'm hesitant to suggest git clean for general use because it's too easy to accidentally take out local changes, or clean out all your cached tox environments, etc.

Copy link
Contributor

@wshanks wshanks Jan 31, 2024

Choose a reason for hiding this comment

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

My comment was misleading. When I said "this problem," I was thinking of the problem of left over files in a repo that had been installed in editable mode rather than the specific import qiskit problem with left over namespace package metadata.

I didn't fully test it but somehow there were some code removals that didn't trigger import errors for me with my editable install, and I had assumed the imports were still working because that is how things worked with Python 2 and the .pyc files left over in directories that had had the .py files removed. Maybe it was actually the info directories or something else. I didn't try to recreate the problem after doing the clean.

That particular case was coming up for me with other packages that set up Qiskit plugins in the package metadata (specifically qiskit-ibm-provider's transpiler pass plugin that imported from qiskit.extensions). It kept working in my editable install but then failed in the clean CI environment. It is a surprising case because I was not using the plugin or anything else from qiskit-ibm-provider but just having the distribution with the plugin installed is enough to trigger an error when trying to do a transpile. I was wondering if there was anything to mention related to that in the troubleshooting section, but I think ultimately it is a consequence of having a package that is not compatible with Qiskit 1.0 installed, so that is already covered.

Copy link
Member Author

@jakelishman jakelishman Jan 31, 2024

Choose a reason for hiding this comment

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

Could you send me a set of repro steps, and I'll look into either making the import check smarter, or adding an extra bit to the troubleshooting section in this guide in a day or so, after an initial version is public?



<span id="why-did-this-happen"></span>
## Why these changes happened
Copy link
Contributor

Choose a reason for hiding this comment

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

I like all the detail here. I wonder if there should be a section (maybe linkable) that just says that the files that were in qiskit-terra are now in qiskit and that pip does not know how to handle overlapping files and will overwrite and erase files from qiskit when operating on qiskit-terra and vice versa, as the quick version of why the careful steps described in the user section are necessary and then those interested can expand out from there.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that's an explanation of what about the environment is actually broken, more than "why we did this", maybe? Do you think we could leave that out, or at least add it a bit later? (I'm keen to get this merged and published.)

Copy link
Contributor

Choose a reason for hiding this comment

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

Right, I just suggested it here because I had originally suggested it up above before getting to this section and seeing that you had largely covered all the details of what was going on with packaging here. I'll leave it up to you. I don't know what is best to tell users. Mainly what I was thinking is that if a user understood about the same file paths being in both packages and pip not trying to avoid conflicts it might give intuition for understanding problems that arise.

@jakelishman
Copy link
Member Author

Abby and Becky: I've updated this with an extra couple of sentences from some of Will's suggestions above, but for the other points he's raised, I think it would be best to get this published and potentially expand the troubleshooting / explanation after we've got a first version published. We need a live link for Qiskit ASAP so we can do a couple of releases.

@abbycross
Copy link
Collaborator

Abby and Becky: I've updated this with an extra couple of sentences from some of Will's suggestions above, but for the other points he's raised, I think it would be best to get this published and potentially expand the troubleshooting / explanation after we've got a first version published. We need a live link for Qiskit ASAP so we can do a couple of releases.

For the purposes of releasing quickly, then, I'll approve this now.

@abbycross abbycross self-requested a review January 31, 2024 16:01
@jakelishman
Copy link
Member Author

I've set up the shortlink https://qisk.it/packaging-1-0 to point to where this page should appear (assuming the filename doesn't change, but I can just update the shortlink redirection if it does), so from the Qiskit side, I just need the page to appear now haha.

Copy link
Collaborator

@abbycross abbycross left a comment

Choose a reason for hiding this comment

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

Good to go for the first release!

@jakelishman
Copy link
Member Author

Thanks! That's great - at our absolute fastest, the relevant Qiskit release would still be a couple of hours away, but more probably it'll be tomorrow.

@abbycross
Copy link
Collaborator

Would you like me to merge it? Then I can start the process of syncing so that it publishes. Or feel free to merge!

@jakelishman
Copy link
Member Author

sure, thanks!

Merged via the queue into Qiskit:main with commit 1d734cd Jan 31, 2024
@jakelishman jakelishman deleted the migrate-1.0 branch January 31, 2024 16:07
frankharkins pushed a commit to frankharkins/documentation that referenced this pull request Jul 22, 2024
Part of Qiskit#704.

This is a pretty large guide on migration recommendations and
troubleshooting around the Qiskit 1.0 release.

I'll ask the other Qiskit developers and a few others to have a look at
this for technical content as well.

Given the length of this, I suspect it might be better as a stand-alone
migration guide.

---------

Co-authored-by: Rebecca Dimock <beckyd@us.ibm.com>
Co-authored-by: ABBY CROSS <across@us.ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants