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

Jump to bottom

python3Packages.cirq: unbreak #96883

Merged
merged 1 commit into from Sep 2, 2020
Merged

python3Packages.cirq: unbreak #96883

merged 1 commit into from Sep 2, 2020

Conversation

drewrisinger
Copy link
Contributor

Motivation for this change

This is a pre-ZHF PR to unbreak a package before release.

Found out it was broken due to changing dependencies or something.

Remove unneeded test disables.
Add changelog.
Unpin most python dependencies.

Things done
  • 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"
  • 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.

drewrisinger added a commit to drewrisinger/nur-packages that referenced this pull request Sep 1, 2020
@drewrisinger
python3Packages.cirq: minor cleanup
199f944 
Some prep work ahead of Nixpkgs 20.09. Matches work (mostly) in
NixOS/nixpkgs#96883.

Minor meta update.
@drewrisinger drewrisinger mentioned this pull request Sep 1, 2020
Merged
drewrisinger added a commit to drewrisinger/nur-packages that referenced this pull request Sep 1, 2020
@drewrisinger
python3Packages.cirq: minor cleanup
dd2e687 
Some prep work ahead of Nixpkgs 20.09. Matches work (mostly) in
NixOS/nixpkgs#96883.

Minor meta update.
@jonringer
Copy link
Contributor

not sure if it's just my machine, or numpy:

$ nix build --no-link --keep-going --option build-use-sandbox relaxed -f /home/jon/.cache/nixpkgs-review/pr-96883/build.nix
builder for '/nix/store/flvaydgqqqlpjnhm3s4d1cxb444j3qv5-python3.8-cirq-0.8.2.drv' failed with exit code 1; last 10 log lines:
  test_example_runs_bcs_mean_field_perf                197,441.5890 (505.45)   246,864.2670 (541.90)   210,672.8178 (528.86)   20,686.5867 (>1000.0)  202,633.1730 (510.30)   19,923.2685 (>1000.0)       1;0      4.7467 (0.00)          5           1
  test_example_runs_hello_line_perf                    219,470.2300 (561.84)   309,193.5710 (678.72)   238,316.9818 (598.26)   39,638.7597 (>1000.0)  220,879.2550 (556.25)   24,512.7475 (>1000.0)       1;1      4.1961 (0.00)          5           1
  -----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

  Legend:
    Outliers: 1 Standard Deviation from Mean; 1.5 IQR (InterQuartile Range) from 1st Quartile and 3rd Quartile.
    OPS: Operations Per Second, computed as 1 / Mean
  =========================== short test summary info ============================
  FAILED cirq/ion/convert_to_ion_gates_test.py::test_convert_to_ion_gates - ass...
  ===== 1 failed, 8644 passed, 36 xfailed, 255 warnings in 120.70s (0:02:00) =====
builder for '/nix/store/rn05sywqhwg835gq3g56mhs1cyh31lmg-python3.7-cirq-0.8.2.drv' failed with exit code 1; last 10 log lines:
  test_example_runs_bcs_mean_field_perf                211,374.1080 (555.23)   307,129.9690 (711.02)   239,041.2258 (614.29)   39,523.0382 (>1000.0)  226,088.3760 (582.58)   42,361.8932 (>1000.0)       1;0      4.1834 (0.00)          5           1
  test_example_runs_hello_line_perf                    252,885.7500 (664.27)   303,608.6790 (702.87)   272,028.4302 (699.06)   23,228.6949 (>1000.0)  257,157.8810 (662.64)   38,013.4590 (>1000.0)       1;0      3.6761 (0.00)          5           1
  -----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

  Legend:
    Outliers: 1 Standard Deviation from Mean; 1.5 IQR (InterQuartile Range) from 1st Quartile and 3rd Quartile.
    OPS: Operations Per Second, computed as 1 / Mean
  =========================== short test summary info ============================
  FAILED cirq/ion/convert_to_ion_gates_test.py::test_convert_to_ion_gates - ass...
  ===== 1 failed, 8644 passed, 36 xfailed, 255 warnings in 131.97s (0:02:11) =====
cannot build derivation '/nix/store/3g24fz969li527ai4gfb550cc9fdhzbs-env.drv': 2 dependencies couldn't be built
[0 built (2 failed), 4 copied (45.0 MiB), 13.3 MiB DL]
error: build of '/nix/store/3g24fz969li527ai4gfb550cc9fdhzbs-env.drv' failed
https://github.com/NixOS/nixpkgs/pull/96883
2 packages failed to build:
python37Packages.cirq python38Packages.cirq

@drewrisinger
Copy link
Contributor Author

@jonringer I can't reproduce:

https://github.com/NixOS/nixpkgs/pull/96883
2 packages built:
python37Packages.cirq python38Packages.cirq

I've had trouble with that test before, the failures I've seen were machine precision-ish issues (0.750...2 != 0.75). I can add that patch/test disable if you'd like.

@jonringer
Copy link
Contributor

I'm not sure the perf tests give a lot more validity when packaging (great upstream in a controlled environment). And it might be worthwhile to remove them altogether to ensure less flakey builds.

@drewrisinger
python3Packages.cirq: unbreak
e28ef25 
Remove unneeded test disables.
Add changelog.
Unpin most python dependencies.
@drewrisinger
Copy link
Contributor Author

The actual test failing is here:

FAILED cirq/ion/convert_to_ion_gates_test.py::test_convert_to_ion_gates - ass...

Which is not a benchmark, and is sort of relevant to my use case for this package, which is why I'm hesitant to disable it entirely.
https://github.com/quantumlib/Cirq/blob/62ef7f7a26cf6e4c5a64359f5bbe47c0cb2765e8/cirq/ion/convert_to_ion_gates_test.py#L39

Disabling benchmarks cuts about 30 seconds off the build time, so I'll do that (but separate from above).

jonringer
jonringer approved these changes Sep 2, 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.

LGTM

Result of nixpkgs-review pr 96883 1

2 packages built:
  • python37Packages.cirq
  • python38Packages.cirq

@jonringer jonringer merged commit ab525fa into NixOS:master Sep 2, 2020
@drewrisinger drewrisinger deleted the dr-pr-cirq branch September 2, 2020 16:44
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

Assignees
No one assigned
Labels
6.topic: python 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