Skip to content

MAINT: Remove openblas_support.py#20074

Closed
mattip wants to merge 10 commits intoscipy:mainfrom
mattip:remove-openblas_support
Closed

MAINT: Remove openblas_support.py#20074
mattip wants to merge 10 commits intoscipy:mainfrom
mattip:remove-openblas_support

Conversation

@mattip
Copy link
Copy Markdown
Contributor

@mattip mattip commented Feb 12, 2024

Builds on numpy/numpy#25505 to refactor wheel building

  • remove tools/openblas_support, use wheels instead
  • refactor to use a single cibw_before_build.sh script
  • move many of the CIBW env variables set in CI into pyproject.toml, which makes it easier to run cibuildwheel outside of CI and reduces some duplication
  • upgrade OpenBLAS from v0.3.20-571 (in the openblas_support script) and v0.3.23.293 (in the ci test runs) to v0.3.26, and add a single source of truth in a requirements/openblas_requirements.txt file

cc@ matthewfeickert

@github-actions github-actions bot added the CI Items related to the CI tools such as CircleCI, GitHub Actions or Azure label Feb 12, 2024
@rgommers
Copy link
Copy Markdown
Member

Thanks Matti. Some non-existing pip and undefined symbols - probably worth working on a fork to get one wheel build job per OS to pass?

@mattip
Copy link
Copy Markdown
Contributor Author

mattip commented Feb 12, 2024

Right, it is a shame to burn so much CI. I can reproduce the failures locally.

After the build starts to work, I am seeing

ImportError: <path>_fblas.cpython-311-x86_64-linux-gnu.so: undefined symbol: dznrm2_

This should be scipy_dznrm2_. I think there is some missing handling of BLAS_SYMBOL_PREFIX around scipy.linalg.get_blas_funcs

@rgommers
Copy link
Copy Markdown
Member

This should be scipy_dznrm2_. I think there is some missing handling of BLAS_SYMBOL_PREFIX around scipy.linalg.get_blas_funcs

That wouldn't be surprising, since we've never used a prefix for anything. I'm fairly sure that it'll be missing in more places. I think this is going to intersect substantially with the re-introduction of ILP64 support, which is ongoing in other branches. So we should probably pause this until ILP64 support is back.

@mattip
Copy link
Copy Markdown
Contributor Author

mattip commented Feb 12, 2024

OK

tylerjereddy added a commit to tylerjereddy/scipy that referenced this pull request Mar 7, 2024
* Related to scipygh-16930

* added a regression test, based on the reproducer in the issue,
that I confirmed does indeed fail locally on x86_64 Linux via:
`CIBW_BUILD=cp311-* cibuildwheel --platform linux --archs x86_64`

* this assumes that scipygh-20074 needs a little more time because
of all the moving parts related to that

* I spent a few hours on this today, and unfortunately I don't
see an obvious fix because the "newest of the old" OpenBLAS binaries we were
using at https://anaconda.org/multibuild-wheels-staging/openblas-libs/files
are too old to fix the test via `cibuildwheel` in my hands; conversely,
the new OpenBLAS binaries (https://anaconda.org/scientific-python-nightly-wheels/openblas-libs/files)
don't seem to offer a version that is simultaneously new enough while
not introducing at least a subset of the challenges experienced
by scipygh-20074 (yikes!)

[skip cirrus] [skip circle]
tylerjereddy added a commit to tylerjereddy/scipy that referenced this pull request Mar 11, 2024
* Related to scipygh-16930

* added a regression test, based on the reproducer in the issue,
that I confirmed does indeed fail locally on x86_64 Linux via:
`CIBW_BUILD=cp311-* cibuildwheel --platform linux --archs x86_64`

* this assumes that scipygh-20074 needs a little more time because
of all the moving parts related to that

* I spent a few hours on this today, and unfortunately I don't
see an obvious fix because the "newest of the old" OpenBLAS binaries we were
using at https://anaconda.org/multibuild-wheels-staging/openblas-libs/files
are too old to fix the test via `cibuildwheel` in my hands; conversely,
the new OpenBLAS binaries (https://anaconda.org/scientific-python-nightly-wheels/openblas-libs/files)
don't seem to offer a version that is simultaneously new enough while
not introducing at least a subset of the challenges experienced
by scipygh-20074 (yikes!)

[skip cirrus] [skip circle]
tylerjereddy added a commit that referenced this pull request Mar 12, 2024
* Related to gh-16930, but doesn't fix it just yet, just updates to
OpenBLAS `0.3.26` and adds an xfailed regression test (the test
doesn't yet pass on all platforms after the OpenBLAS version
bump)

* the xfailed regression test, based on the reproducer in the issue,
does indeed fail locally on x86_64 Linux before the patch (OpenBLAS version bump) and pass
after via:
`CIBW_BUILD=cp311-* cibuildwheel --platform linux --archs x86_64`

* this assumes that gh-20074 needs a little more time because
of all the moving parts related to that

* use manylinux2014 for linux-32 bit openblas

---------

Co-authored-by: mattip <matti.picus@gmail.com>
@lucascolley lucascolley changed the title Remove openblas support MAINT: Remove openblas_support.py Mar 14, 2024
@lucascolley lucascolley added maintenance Items related to regular maintenance tasks Build issues Issues with building from source, including different choices of architecture, compilers and OS labels Mar 14, 2024
@mattip
Copy link
Copy Markdown
Contributor Author

mattip commented Apr 24, 2024

which is ongoing in other branches

I am not sure which branches/PRs are relevant. Certainly #20510, are there others?

@rgommers
Copy link
Copy Markdown
Member

The key one was gh-19816 (and other PRs split off from that. They're all merged now, so this PR is unblocked I believe.

There are a lot of conflicts here, and I think it's a little tricky to figure out what is relevant and what is not. It may be better to open a new PR, using an scipy-openblas32 for a single wheel build first. Problems with the scipy_ prefix should be common to all builds I believe, so getting it to work for a single one (x86-64 manylinux should do) with the smallest diff possible would be a great start. I suspect then the rest will not be too difficult.

@mattip
Copy link
Copy Markdown
Contributor Author

mattip commented May 21, 2024

Replaced by #20585

@mattip mattip closed this May 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Build issues Issues with building from source, including different choices of architecture, compilers and OS CI Items related to the CI tools such as CircleCI, GitHub Actions or Azure maintenance Items related to regular maintenance tasks

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants