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

python3.pkgs.scipy: 1.10.1 -> 1.11.1 & more #239969

Closed
wants to merge 9 commits into from

Conversation

doronbehar
Copy link
Contributor

@doronbehar doronbehar commented Jun 26, 2023

Description of changes

Update some dependencies of scipy, and added:

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.11 Release Notes (or backporting 23.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@doronbehar
Copy link
Contributor Author

It compiles, and I even added blas and lapack overriding support as requested at #206866 . Currently I test cross compilation of it using:

nix build -Lf. pkgsCross.armv7l-hf-multiplatform.python3.pkgs.scipy

@doronbehar doronbehar marked this pull request as ready for review June 27, 2023 11:20
@doronbehar
Copy link
Contributor Author

Tested both armv7l-hf-multiplatform.python3.pkgs.scipy and aarch64-multiplatform.python3.pkgs.scipy and they both compile and seem to generate correct binaries.

Copy link
Member

@FRidh FRidh left a comment

Choose a reason for hiding this comment

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

Great work getting this going!

I'm of the opinion we should not be overriding the build phase. Adding pipBuildFlags is a small change now. The downside of course is the mass rebuild making it a bit time consuming to test.

pkgs/development/python-modules/scipy/default.nix Outdated Show resolved Hide resolved
pkgs/development/python-modules/scipy/default.nix Outdated Show resolved Hide resolved
pkgs/development/python-modules/scipy/default.nix Outdated Show resolved Hide resolved
@doronbehar
Copy link
Contributor Author

The pipBuildFlags thing works. Next step is to introduce meson-python.setupHooks.

@ofborg ofborg bot requested review from FRidh and tobim June 28, 2023 08:52
@doronbehar
Copy link
Contributor Author

Now using meson.setupHook and mesonFlags, the expression looks much cleaner, though still there is no meson-python.setupHook. I also added a better comment why we don't use --config-settings=setup-args and instead why we run meson setup manually. Once pip 23.1 or above lands in Nixpkgs, it shouldn't be hard to create meson-python.setupHook that will set pipBuildFlags with proper --config-settings=setup-args arguments.

@FRidh or @SomeoneSerge if you'd approve the changes, I can squash some of the commits and make the history cleaner as well.

@doronbehar doronbehar changed the title python3.pkgs.scipy: 1.10.1 -> 1.11.0 & more python3.pkgs.scipy: 1.10.1 -> 1.11.1 & more Jul 12, 2023
Diff: scipy/scipy@v1.10.1...v1.11.1

Significant changes to expression:

- Use `pypaBuildHook` to build it.
- Use `fetchFromGitHub` instead of `fetchPypi`.
- Support cross compilation using `pypaBuildHook` and `$mesonFlags`.
- Add a `passthru.updateScript` that downloads and updates hashes for datasets
  needed for tests.
- Cleanup:
  * Remove unneeded anymore `setupPyBuildFlags`.
  * Remove unnecessary `sed` command from preConfigure that fixed a Numpy
    issue in `setup.py`
  * Use `blas` and `lapack` directly, relying on them being overriden by
    the user in all of Python's packages' set - see commit 213d084 .
@doronbehar
Copy link
Contributor Author

I rebased the git history a bit, with almost a single commit per package. If there are no objections, with your permissions I'd like to merge this today / tomorrow.

@SuperSandro2000
Copy link
Member

I would like to cherry-pick this into python-updates unless there are review comments left we need to consider.

@SuperSandro2000
Copy link
Member

SuperSandro2000 commented Jul 18, 2023

Thanks, cherry-picked into #244135

Python batch upgrade automation moved this from In progress to Done Jul 18, 2023
@mweinelt
Copy link
Member

mweinelt commented Jul 30, 2023

This broke scipy on x86_64-darwin. PTAL. cc #245935

https://hydra.nixos.org/build/229710588
https://hydra.nixos.org/build/229711355

@doronbehar
Copy link
Contributor Author

doronbehar commented Jul 30, 2023

This broke scipy on x86_64-darwin. PTAL. cc #245935

hydra.nixos.org/build/229710588
hydra.nixos.org/build/229711355

@mweinelt these links don't seem related to scipy, putting here a link that seems more relevant to me:

https://hydra.nixos.org/eval/1797964?filter=scipy&compare=1797817&full=#tabs-now-fail

@doronbehar
Copy link
Contributor Author

A fix is possible at #246152 , will need to wait for ofborg to check it...

@mweinelt
Copy link
Member

mweinelt commented Jul 30, 2023

Please recheck the build steps for the linked builds. They are very much related.

https://hydra.nixos.org/log/cwpjczjxjav1kyq5p1ilav93rkl0gd3a-python3.10-scipy-1.11.1.drv

@doronbehar
Copy link
Contributor Author

Please recheck the build steps for the linked builds. They are very much related.

hydra.nixos.org/log/cwpjczjxjav1kyq5p1ilav93rkl0gd3a-python3.10-scipy-1.11.1.drv

I see. I'm still somewhat confused about the logs there (why is this aarch64 log not failing like the others?). But in any case, in #246152 as well, the build fails like you linked, and I have no idea how to fix that issue :/. It's also hard to debug this without a real darwin machine. Maybe contact upstream?

@rgommers
Copy link

Maybe contact upstream?

I'm already paying attention here.

In file included from scipy/stats/_stats_pythran.cpp:34:
In file included from /nix/store/97hrm3g9b47ishjflg1nhrp07sfnvijl-python3.10-pythran-0.13.1/lib/python3.10/site-packages/pythran/pythonic/include/numpy/ceil.hpp:8:
In file included from /nix/store/97hrm3g9b47ishjflg1nhrp07sfnvijl-python3.10-pythran-0.13.1/lib/python3.10/site-packages/pythran/xsimd/xsimd.hpp:55:
/nix/store/97hrm3g9b47ishjflg1nhrp07sfnvijl-python3.10-pythran-0.13.1/lib/python3.10/site-packages/pythran/xsimd/arch/xsimd_scalar.hpp:474:16: error: use of undeclared identifier '__exp10f'
        return __exp10f(x);
               ^
/nix/store/97hrm3g9b47ishjflg1nhrp07sfnvijl-python3.10-pythran-0.13.1/lib/python3.10/site-packages/pythran/xsimd/arch/xsimd_scalar.hpp:478:16: error: use of undeclared identifier '__exp10'; did you mean 'exp10'?
        return __exp10(x);
               ^~~~~~~
               exp10

It looks like the problem is that xsimd is too old:

Run-time dependency xsimd found: YES 9.0.1

If you upgrade xsimd to 11.0 the problem should disappear.

@doronbehar
Copy link
Contributor Author

Thanks @rgommers I updated #246152 to include that update. Let's see if our darwin CI says it's OK.

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

Successfully merging this pull request may close these issues.

python3Packages.scipy can't be build with other blasProvider than openBlas
6 participants