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.

Sign up for GitHub

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

Jump to bottom

python3Packages.qiskit: 0.14.0 -> 0.17.0 #78772

Merged
merged 6 commits into from Apr 8, 2020
Merged

python3Packages.qiskit: 0.14.0 -> 0.17.0 #78772

merged 6 commits into from Apr 8, 2020

Conversation

drewrisinger
Copy link
Contributor

Motivation for this change

pythonPackages.qiskit build was broken because upstream refactored the package into several smaller packages.

This PR fully refactors the Qiskit library into the subpackages, which are all tested as much as possible, and includes new dependency packages as necessary (or updates to existing packages).

Things done

My apologies, I know this is a pretty big PR to review. Every new dependency added was necessary for build & tests to pass, minus stestr which is primarily for faster testing.

  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip". No diff detected
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

Note: most of these packages are missing maintainers. I'm not sure how that process works, and will update when told.

@drewrisinger
Copy link
Contributor Author

The failing check seems to be related to removing obsolete IBMQuantumExperience package.

@drewrisinger drewrisinger changed the title pythonPackages.qiskit: fix broken & 0.14.0 -> 0.14.1 pythonPackages.qiskit: refactor Jan 29, 2020
@drewrisinger drewrisinger changed the title pythonPackages.qiskit: refactor pythonPackages.qiskit: fix broken Jan 29, 2020
jonringer
jonringer reviewed Jan 29, 2020
View reviewed changes
Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

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

If you want to split this into many PRs, that would probably be a good idea

pkgs/development/libraries/libcint/default.nix Outdated Show resolved Hide resolved
pkgs/development/libraries/libcint/default.nix Outdated Show resolved Hide resolved
pkgs/development/libraries/libcint/default.nix Outdated Show resolved Hide resolved
pkgs/development/libraries/libcint/default.nix Outdated Show resolved Hide resolved
pkgs/development/libraries/libcint/default.nix Outdated Show resolved Hide resolved
pkgs/development/python-modules/qiskit-terra/default.nix Outdated Show resolved Hide resolved
pkgs/development/python-modules/qiskit/default.nix Outdated Show resolved Hide resolved
pkgs/development/python-modules/scs/default.nix Outdated Show resolved Hide resolved
pkgs/development/python-modules/scs/default.nix Outdated Show resolved Hide resolved
pkgs/development/python-modules/scs/default.nix Outdated Show resolved Hide resolved
jonringer
jonringer reviewed Jan 29, 2020
View reviewed changes
Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

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

If you want to split this into many PRs, that would probably be a good idea

@drewrisinger
Copy link
Contributor Author

If you want to split this into many PRs, that would probably be a good idea

I can do that. We're just looking at ~20 PRs, which is a pain to submit. Any tips on resolving dependency issues b/c the later PRs won't build w/o the previous PRs being approved?

@jonringer
Copy link
Contributor

honestly, a package for each PR isn't a terrible idea: limits scope, makes edits easy, faster to review

@drewrisinger
Copy link
Contributor Author

Splitting into multiple PRs, one per package. Most issues addressed. My apologies in advance for the deluge of PRs @jonringer

@drewrisinger drewrisinger force-pushed the dr-pr-python-qiskit branch 2 times, most recently from 46d173e to 18acdae Compare January 30, 2020 14:46
This was referenced Jan 30, 2020
Merged
Merged
Merged
Closed
Merged
Merged
Merged
Closed
Closed
Merged
Closed
Merged
Merged
Merged
Merged
@drewrisinger
Copy link
Contributor Author

@GrahamcOfBorg eval

@drewrisinger
Copy link
Contributor Author

@GrahamcOfBorg build python37Packages.qiskit-aqua
@GrahamcOfBorg build python38Packages.qiskit-aqua
@GrahamcOfBorg build python37Packages.qiskit
@GrahamcOfBorg build python38Packages.qiskit

@ofborg ofborg bot requested a review from pandaman64 April 7, 2020 13:04
jonringer
jonringer reviewed Apr 7, 2020
View reviewed changes
Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

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

otherwise LGTM

pkgs/development/python-modules/ipyvue/default.nix Show resolved Hide resolved
pkgs/development/python-modules/ipyvuetify/default.nix Show resolved Hide resolved
@drewrisinger drewrisinger force-pushed the dr-pr-python-qiskit branch 2 times, most recently from a235dc0 to f21ba3b Compare April 7, 2020 19:07
@drewrisinger
pythonPackages.ipyvue: init at 1.3.1
2df8238
@drewrisinger
pythonPackages.ipyvuetify: init at 1.2.2
8b75740
@drewrisinger
python3Packages.qiskit-ibmq-provider: 0.5.0 -> 0.6.0
2133da1
@drewrisinger
pythonPackages.IBMQuantumExperience: remove deprecated package
c3c0f47 
Package was deprecated, and effectively replaced by
pythonPackages.qiskit-ibmq-provider.
@drewrisinger
pythonPackages.qiskit: 0.14.0 -> 0.17.0
df05335 
Updates the qiskit package to follow new Git structure, where
subcomponents are broken into separate packages (terra, aer, etc).

Refactor/cleanup
@drewrisinger
python3Packages.qiskit-aqua: disable slow tests
cb2e60f 
Arbitrarily disable any tests > 10s on my machine to lighten the build
load (both for borg CI & Hydra builds). All disabled slow tests pass
locally.
@drewrisinger
Copy link
Contributor Author

Ok. Apologies. Built locally with nixpkgs-review pr 78772.
@GrahamcOfBorg eval
Updated in this PR:
@GrahamcOfBorg build python37Packages.qiskit python37Packages.qiskit-ibmq-provider
Added in this PR:
@GrahamcOfBorg build python37Packages.ipyvue python38Packages.ipyvue python37Packages.ipyvuetify python38Packages.ipyvuetify
Dependent of qiskit (enabled now that not broken):
@GrahamcOfBorg build python37Packages.qasm2image

jonringer
jonringer approved these changes Apr 8, 2020
View reviewed changes
Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

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

diff LGTM
commits LGTM

https://github.com/NixOS/nixpkgs/pull/78772
9 package built:
python37Packages.ipyvue python37Packages.ipyvuetify python37Packages.qasm2image python37Packages.qiskit python37Packages.qiskit-aqua python37Packages.qiskit-ibmq-provider python38Packages.ipyvue python38Packages.ipyvuetify python38Packages.qiskit-aqua

@jonringer jonringer merged commit 09cd8b6 into NixOS:master Apr 8, 2020
@jonringer
Copy link
Contributor

it only took two months, but we did it :)

@drewrisinger
Copy link
Contributor Author

drewrisinger commented Apr 8, 2020 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jonringer jonringer jonringer approved these changes

@FRidh FRidh Awaiting requested review from FRidh

@pandaman64 pandaman64 Awaiting requested review from pandaman64

Assignees
No one assigned
Labels
6.topic: python 8.has: clean-up 8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 11.by: package-maintainer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@drewrisinger @jonringer