scipy v1.7.0 #169
Merged
scipy v1.7.0 #169
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
h-vetinari
requested review from
jakirkham,
msarahan,
ocefpaf and
rgommers
as code owners
|
Hi! This is the friendly automated conda-forge-linting service. I just wanted to let you know that I linted all conda-recipes in your PR ( |
|
Interesting new error... This looks like an additional dependency: scipy/scipy@bbc1936, which now expectes a git-submodule in Let's try switching to PyPI... |
|
@conda-forge/boost-cpp Besides the philosophical aspect of the question, there also seem to be build errors stemming from boost, e.g. Though I have to say, that path looks a bit suspect as well @tylerjereddy |
What would the advantage be? SciPy requires this exact version of Boost (1.76.0 didn't work), it may or may not be patched in the future, and SciPy only supports building it via that submodule.
Yep, |
|
Should be fixed by scipy/scipy#14196 |
|
Still failing with that patch - but then, it might be that this requires a newer sdist? I guess I could try deleting the stuff in there, OTOH, the failure on linux doesn't look related: on windows, the failure looks pythran-related. |
|
On windows there's some lovely template spew: |
Indeed
This is numpy version related. It shouldn't try to use the |
Ah same problem, we're including the generated file in the sdist. |
The only blocker to using a more recent Boost version is a GCC bug fixed in version 7. AFAIK, it's only the backwards compatibility to GCC 4.8/5 that prevents updating I think the above is not pythran related, but rather complaining about unavailability of the newer numpy random API. Is the numpy version consistent? EDIT: Ralf already identified the numpy random API, didn't see that, apologies |
|
Update (aside from the other issues): osx-arm builds pending on |
Actually the needed solution is different. We still ship generated C/C++ files from Cython code. The problem is the use of an absolute path in an include statement. That should be replaced by using the correct include dir in the |
|
This |
|
So, at least |
|
Working on resolving the issues today. |
rgommers
added a commit
to rgommers/scipy
that referenced
this pull request
Jun 13, 2021
This supercedes scipygh-14196, and closes scipygh-14199. The `*.cxx` files for Boost contain an absolute path, which is coming from the use of `src_dir` in `stats/_boost/setup.py`. That makes the generated code non-portable. Should fix the failures with `1.7.0rc1` in conda-forge/scipy-feedstock#169. Generated sources now also contain NumPy-version-dependent code (e.g. to use the numpy.random Cython API for >= 1.19.0), which is another good reason to stop shipping generated sources. The Cython build dependency was already specified correctly in `pyproject.toml`. Note that `python setup.py sdist` will generate some noise for Cython sources, like: ``` non-existing path in 'scipy/spatial': 'ckdtree.cxx' ``` This is coming from numpy.distutils, which sees `config.add_extension(somefile.c)` while `somefile.c` doesn't exist. This is harmless.
rgommers
added a commit
to rgommers/scipy
that referenced
this pull request
Jun 13, 2021
This supercedes scipygh-14196, and closes scipygh-14199. The `*.cxx` files for Boost contain an absolute path, which is coming from the use of `src_dir` in `stats/_boost/setup.py`. That makes the generated code non-portable. Should fix the failures with `1.7.0rc1` in conda-forge/scipy-feedstock#169. Generated sources now also contain NumPy-version-dependent code (e.g. to use the numpy.random Cython API for >= 1.19.0), which is another good reason to stop shipping generated sources. The Cython build dependency was already specified correctly in `pyproject.toml`. Note that `python setup.py sdist` will generate some noise for Cython sources, like: ``` non-existing path in 'scipy/spatial': 'ckdtree.cxx' ``` This is coming from numpy.distutils, which sees `config.add_extension(somefile.c)` while `somefile.c` doesn't exist. This is harmless.
|
OK, I had another idea to include the submodule (trivial in retrospect 🤦), which avoids the sdist issues. While linux & osx are passing for cpython now, windows still fails to build with some pythran-related errors ( Logs for build failure in
|
SciPy builds got heavier, on the main repo we're getting regular timeouts on our Azure Windows build + full test suite as well. Working on improving build times, but I think there's also scope to add caching better between CI runs (e.g., use Looks like there's no point restarting
Thanks @h-vetinari, +1 from me! Want to send a separate PR to add yourself? |
This reverts commit 052cff0.
…nda-forge-pinning 2021.06.22.04.56.18
I don't think the caching would be so easy to pair with the current conda-forge infrastructure.
As I mentioned above, there was at least one passing PPC build. I've also re-checked the diff since then and am retrying if reverting 052cff0 helps. I gave drone those rights, and have been able to restart builds on other feedstocks where I'm a maintainer.
Cool, happy to hear it! I took the liberty to add this to it here - it can wait until this PR is merged. |
Hey @mattip, no rush, just checking if there's been any news on this since last week. |
|
I haven't been able to make progress with the PyPy error. Is there a way to release the other builds but hold PyPy back? |
The builds will not be uploaded if the CI fails, so this would be mergeable as is without issue. |
|
I had a look at the Drone timeout, it loses almost 10 minutes on a conda solve:
And it was not even close to done - it timed out right after starting on |
rgommers
approved these changes
Jun 27, 2021
There was a problem hiding this comment.
Changes LGTM, and Windows, macOS and Linux builds are green. So let's get this in.
Thanks @h-vetinari and everyone else!
|
Update: all aarch builds are available now. For ppc, one build ran through (py39), been restarting the others but with no success so far. Note that ppc+pypy always stalls sometime well before the 50min cutoff, à la: |
|
Nice, thanks @h-vetinari. For the aarch64 builds, is it just getting scheduled on faster hardware? Would be nice to see the build log or a summary on why it worked now. |
AFAIU, there are two different machines that make up the drone queue, one larger and one smaller - the difference in build time can be quite substantial (~50%), which obviously becomes relevant if the builds are starting to time out on the smaller machine. You can see some passing builds after the restart here. |
|
PPC + cpython are now built as well, I've stopped retriggering PPC + PyPy because it reproducibly leads to hangs. That means 1.7.0 has now been built for all but the following three variants:
|
|
Thanks for shepherding those builds @h-vetinari! |
|
If I can figure out what is going on with PyPy + Cython + SciPy, I will create a new PR. |
This was referenced Jun 28, 2021
|
Moved to #173 |
Merged
Closed
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Also picking up some of the good bits from #168
TODOs:
follow-up: switch to sdists from GH (as soon as 1.7.0rc2 appears)