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

MAINT: prepare for SciPy 1.8.0rc4 #15479

Merged
merged 6 commits into from Jan 29, 2022

Conversation

tylerjereddy
Copy link
Contributor

  • backport the two merged PRs with a backport label

  • this needs to fix the remaining conda-forge PROPACK issues

  • @mckib2 I don't see a submodule change here? was the maintenance branch already ahead of main in that regard? Not sure I fully trust current submodule settings/situation, so good to double check we have the right submodule commit in use here (seems to be the latest one you can get locally).

  • cc @h-vetinari

  • I'm still slightly confused about how PROPACK related issues could remain at this point--my objective was to turn off (skip) all propack related tests in rc3 with the env variable--I still think some are not getting skipped properly as I noted in the rc3 backport PR

mdhaber and others added 3 commits January 28, 2022 20:28
* FIX: add G77 wrappers to PROPACK build

* BLD: point PROPACK to fork branch temporarily

* FIX: linting and setup.py fixes

* BLD: only use G77 PROPACK wrappers if needs_g77_abi_wrapper

* FIX: _is_32bit() logical error fix; compile without optimizations for 32 bit to solve some problems

* FIX: lapack info clobber removed

* BLD: back to scipy propack; adjustments for 32-bit build

* BLD: update submodule to newest PROPACK revision

* BLD: drop ilp64 support

* MAINT: update to latest PROPACK main
@tylerjereddy tylerjereddy added the maintenance Items related to regular maintenance tasks label Jan 29, 2022
@tylerjereddy
Copy link
Contributor Author

For the submodule business, it looks like that snuck in with the REL commit before this to bump to rc4, which wasn't really intentional (I thought we added a guard to make this harder to do...):

image

Anyway, the net result there is acceptable I suppose.

@tylerjereddy
Copy link
Contributor Author

Things I still plan to check here:

  • scipy/sparse/linalg/tests/test_propack.py tests need to be skipping when USE_PROPACK is not set; I was convinced this was true already for test_svds, but never really understood why the skip was supposed to happen for test_propack.py, and indeed the segfault on OSX conda-forge seemed to happen in that set of tests (the MKL backport may solve this, but is orthogonal to the intention--those tests shouldn't be running at all unless the user opts in to use PROPACK at runtime); I'm almost certain that I'm going to restore the skip I wanted to add in the rc3 backport PR here: MAINT: release branch PROPACK switch (default off) #15432 (using the environment variable, NOT the import check since we can always import)
  • .gitmodules submodule update machinery is consistent with main (more PROPACK patches may come for Windows support on maintenance branch, having this work reliably would be useful)

For remaining failures on conda-forge:

  • the first point above and the MKL patch here should prevent the segfaults in two ways for OSX + MKL
  • for Windows, conda-forge needs to turn off pythran, we gave up on trying to support binaries with pythran on Windows in the short term with distutils mysteries
  • for their aarch64 linux failure, it is a single test failure I haven't seen before, maybe they can vendor a patch to skip one test

@tylerjereddy tylerjereddy added this to the 1.8.0 milestone Jan 29, 2022
* `test_propack.py` should now skip all tests when
`USE_PROPACK` is not set
@tylerjereddy
Copy link
Contributor Author

This does what I want now re: PROPACK skipping. python runtests.py -t "scipy/sparse/linalg/tests/test_propack.py" -- -rsx doesn't run a single test when USE_PROPACK is not set. It properly runs 40 tests when USE_PROPACK is set.

I double checked the diff here--looks right to me. If CI passes, I will merge and proceed with the rc4 release.

@tylerjereddy
Copy link
Contributor Author

Ok, two timeouts in CI are just the usual for this maintenance branch, so let's merge and hope we don't need a fifth rc.

@tylerjereddy tylerjereddy merged commit 4f3969d into scipy:maintenance/1.8.x Jan 29, 2022
@tylerjereddy tylerjereddy deleted the treddy_180rc4 branch January 29, 2022 22:47
@tylerjereddy
Copy link
Contributor Author

1.8.0rc4 wheel builds are underway; I'm a little concerned about some activity I saw in the NumPy wheel repo related to MacOS Azure changes--perhaps there's a decent chance I'm going to have replicate the NumPy wheels patches as well.. (cc @charris ?).

This is the patch I may need to "mimic" for our wheels: MacPython/numpy-wheels#149

@tylerjereddy
Copy link
Contributor Author

well, the Azure MacOS jobs seem to be passing.. so maybe SciPy is "ok" for this for now...

@h-vetinari
Copy link
Member

So the mechanism is now working, but I'm now wondering if the environment variable shouldn't be called SCIPY_USE_PROPACK rather than USE_PROPACK...? Too late for 1.8.0, but just thinking out loud...

@tylerjereddy
Copy link
Contributor Author

more encouragement to fix the problem and delete the variable entirely I suppose

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Items related to regular maintenance tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants