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: backports/prep for 1.13.1 #20632

Open
wants to merge 38 commits into
base: maintenance/1.13.x
Choose a base branch
from

Conversation

tylerjereddy
Copy link
Contributor

@tylerjereddy tylerjereddy commented May 2, 2024

We're about 3.5 weeks from the planned branching for SciPy 1.14.x. Since 1.13.0 was a bit rushed to support NumPy 2.x, and indeed I made a few oversights for backporting, it may make sense to at least assess how much work it would be to deliver 1.13.1. I've cleaned up the backport labels a bit too, since there were almost 50 of them. Many were things I could have backported to 1.12.x, but I think that would stretch me (and reviewers/support) too thin at this point.

Backports included here (so far):

  1. BUG: sparse: align dok_array.pop() to dict.pop() for case with no default arg #20322
  2. BUG: sync pocketfft again #20333
  3. DOC: optimize: fix wrong optional argument name in root(method="lm"). #20401
  4. DOC: add missing deprecations from 1.13.0 release notes #20435
  5. MAINT/DOC: fix syntax in 1.13.0 release notes #20437
  6. DOC: remove spurious backtick from release notes #20449
  7. BUG: linalg: fix ordering of complex conj gen eigenvalues #20473
  8. TST: tolerance bumps for the conda-forge builds #20474
  9. TST: compare absolute values of U and VT in pydata-sparse SVD test #20484
  10. BUG: Include Python.h before system headers. #20505 (already covered by BLD: Move Python-including files to start of source. #20537)
  11. BUG: linalg: fix eigh(1x1 array, driver='evd') f2py check #20516
  12. BUG: spherical_in for n=0 and z=0 #20527 (danger: I applied the patch manually to the old version of the special code; I did backport the new test though, and it did fail before/pass after my manual application of a slightly different patch)
  13. BLD: Fix error message for f2py generation fail #20530
  14. update openblas to 0.3.27 #20569
  15. BUG: Fix error with 180 degree rotation in Rotation.align_vectors() with an infinite weight #20573
  16. BUG: handle uint arrays in factorial{,2,k} #20607
  17. BUG: prevent QHull message stream being closed twice #20611
  18. BUG: fix Vor/Delaunay segfaults #20633
  19. BUG: ndimage: fix origin handling for {minimum, maximum}_filter #20653
  20. MAINT: stats.yulesimon: fix kurtosis #20654
  21. BUG: sparse: Clean up 1D input handling to sparse array/matrix and add tests #20444
  22. BUG: sparse: Fix summing duplicates for CSR/CSC creation from (data,coords) #20687
  23. MAINT: added doc/source/.jupyterlite.doit.db to .gitignore See #20264 #20280
  24. MAINT: lint: temporarily disable UP031 #20601
  25. MAINT/DEV: lint: disable UP032 #20629
  26. BUG: ndimage.value_indices: deal with unfixed types #20644
  27. CI: pin Python for MacOS conda #20727
  28. TST: Adapt to __array__(copy=True) #20533

Backports already merged to the release branch:

  1. REV: 1.13.x: revert changes to f2py and tempita handling in meson.build files #20567
  2. BLD: Move Python-including files to start of source. #20537

TODO:

  • perhaps deal with what I believe is NumPy copy semantics changes causing a few fails with pre-release (https://github.com/scipy/scipy/actions/runs/9102530687/job/25022358623?pr=20632), though I believe we don't really have to do that on the release branch and could also pin NumPy back a bit (maybe I'll evaluate size of patch needed locally...)
  • CI failure from CI: scipy installation failing in umfpack tests #20714 (persists in main at time of writing)
  • MAINT, TST: two types of failures observed on maintenance/1.13.x branch #20576 (at least one of those is showing up here already I think)
  • there are a few 1.13.0 performance/hang-related tickets open to keep an eye on I think (OpenBLAS and/or Qhull thread contention?)
  • gradually running more of the CI here, including wheel build flush eventually because of i.e., OpenBLAS bump
  • check for more backport candidates from the last few days
  • read the diff carefully
  • (minor) doc/source/.jupyterlite.doit.db isn't gitignored on this branch/PR when I do the doc build (which otherwise passes at the time of writing) locally
  • backport linter suppressions for stuff like UP031 Use format specifiers ... maybe?

dschult and others added 20 commits May 1, 2024 20:21
* Fixes scipygh-20300 by syncing `pocketfft` again, this time
to completely disable `aligned_alloc`; see scipy/pocketfft#3
for details, but in short our more conservative shim
was not sufficient for `conda-forge`, so let's just do the same
thing NumPy did...

[skip cirrus] [skip circle]
This was reported to cause test failures under windows + MKL in conda-forge
cf scipy#20471
A slight tolerance violation was reported on conda-forge in
scipy#20472
A small tolerance violation reported on conda-forge
in scipy#20472
An exact equality was reported as flaky on conda-forge in
scipy#20472

The tolerance violations are of the order of fp noise (< 2e-16), and I don't
think that pickling/unpickling guarantees bit-to-bit compatibility.
In principle, this may invoke recalculations and those may be subject
to fp noise. So I think it's OK to only require allclose(atol=eps)
instead of exact equality.
tol violation observed on conda-forge on win+blis; suggested in
scipy#20474 (comment)
f2py check was just too strict;
LAPACK docs indicate that for N=1, lwork>=1 is acceptable:

*  LWORK   (input) INTEGER
*          The dimension of the array WORK.
*          If N <= 1,               LWORK must be at least 1.
*          If JOBZ = 'N' and N > 1, LWORK must be at least 2*N+1.
*          If JOBZ = 'V' and N > 1, LWORK must be at least
*                                                1 + 6*N + 2*N**2.

https://www.netlib.org/lapack/explore-3.1.1-html/ssyevd.f.html

closes scipygh-20512
Co-authored-by: Ralf Gommers <ralf.gommers@gmail.com>
…ith an infinite weight (scipy#20573)

* Fix exact rotations at 180 deg, improve near 180 deg

Comments

* Tests for exact near 180 deg rotations

* Fix tests

* Code review updates

---------

Co-authored-by: Scott Shambaugh <scottshambaugh@users.noreply.github.com>
* Apply "old version" of the patch provided
at scipygh-20527
* Update the SciPy `1.13.1` release notes following
a series of backports.

[skip cirrus]
@tylerjereddy tylerjereddy added the maintenance Items related to regular maintenance tasks label May 2, 2024
@tylerjereddy tylerjereddy added this to the 1.13.1 milestone May 2, 2024
@lucascolley lucascolley removed scipy.optimize scipy.interpolate scipy._lib scipy.spatial Documentation Issues related to the SciPy documentation. Also check https://github.com/scipy/scipy.org Cython Issues with the internal Cython code base labels May 3, 2024
andfoy and others added 2 commits May 5, 2024 11:48
* Deals with the `spatial` part of scipygh-20623 (`Voronoi`
was also affected beyond the originally-reported `Delaunay`
segfault).

* Both classes are documented to accept arrays with two dimensions
only, so raise a `ValueError` in cases with other dimensions to
avoid the segfault.

* Other potential points of confusion here are the differences
between arrays with two dimensions and two dimensional arrays
that represent generators with more than two dimensions via
the number of columns, but this is just how things are for
most array programming languages of course. Also, the docs
don't explicitly say array-like for the generators I don't think, but this patch
only worked if I placed it after the coercion to array type.

[skip circle] [skip ci] [ci skip]
@tylerjereddy
Copy link
Contributor Author

tylerjereddy commented May 5, 2024

I added the backport Ralf suggested and the other spatial segfault fix from the last few days, so we're back at parity with the list of merged backport PRs.

That said, I'm skipping the CI for now because it is known to fail per the TODO list in the original comment above. I'll try to chip away at that as time permits.

@tylerjereddy
Copy link
Contributor Author

@steppi @czgdp1807 @dschmitz89 what do you think about applying the patch below (similar to the one suggested at: #20208 (comment)) to resolve the first part of #20576 on the release branch here? There's an explanation there about why I'm hesitant to backport gh-20393 -- do you agree with my hesitation?

--- a/scipy/stats/_continuous_distns.py
+++ b/scipy/stats/_continuous_distns.py
@@ -692,12 +692,10 @@ class beta_gen(rv_continuous):
         return _boost._beta_sf(x, a, b)
 
     def _isf(self, x, a, b):
-        with np.errstate(over='ignore'):  # see gh-17432
-            return _boost._beta_isf(x, a, b)
+        return sc.betaincinv(a, b, 1 - x)
 
     def _ppf(self, q, a, b):
-        with np.errstate(over='ignore'):  # see gh-17432
-            return _boost._beta_ppf(q, a, b)
+        return sc.betaincinv(a, b, q)
 
     def _stats(self, a, b):
         return (
diff --git a/scipy/stats/tests/test_distributions.py b/scipy/stats/tests/test_distributions.py
index 6d91bb8a6..de26dc3d8 100644
--- a/scipy/stats/tests/test_distributions.py
+++ b/scipy/stats/tests/test_distributions.py
@@ -4454,7 +4454,7 @@ class TestBeta:
         assert_equal(stats.beta.pdf(1, a, b), 5)
         assert_equal(stats.beta.pdf(1-1e-310, a, b), 5)
 
-    @pytest.mark.xfail(IS_PYPY, reason="Does not convert boost warning")
+    @pytest.mark.xfail(reason="Does not warn on special codepath")
     def test_boost_eval_issue_14606(self):
         q, a, b = 0.995, 1.0e11, 1.0e13
         with pytest.warns(RuntimeWarning):

I was able to confirm failures with invalid value encountered in _beta_ppf before the patch and fix after using clang-16 locally on ARM Mac.

@tylerjereddy
Copy link
Contributor Author

As for the second blocker in gh-20576, the assert_no_overwrite seems to be probing data overwrite/copy semantics to some degree, and if we diff against main for the failing custom class:

--- a/scipy/linalg/_testutils.py
+++ b/scipy/linalg/_testutils.py
@@ -12,8 +12,6 @@ class _FakeMatrix2:
         self._data = data
 
     def __array__(self, dtype=None, copy=None):
-        if copy:
-            return self._data.copy()
         return self._data

that seems like a pretty likely reason to deviate from main on behavior, so perhaps worth just pushing that in and seeing if it makes the Windows failure go away.

@dschmitz89
Copy link
Contributor

@steppi @czgdp1807 @dschmitz89 what do you think about applying the patch below (similar to the one suggested at: #20208 (comment)) to resolve the first part of #20576 on the release branch here? There's an explanation there about why I'm hesitant to backport gh-20393 -- do you agree with my hesitation?

--- a/scipy/stats/_continuous_distns.py
+++ b/scipy/stats/_continuous_distns.py
@@ -692,12 +692,10 @@ class beta_gen(rv_continuous):
         return _boost._beta_sf(x, a, b)
 
     def _isf(self, x, a, b):
-        with np.errstate(over='ignore'):  # see gh-17432
-            return _boost._beta_isf(x, a, b)
+         return sc.betaincinv(a, b, 1 - x)
 
     def _ppf(self, q, a, b):
-        with np.errstate(over='ignore'):  # see gh-17432
-            return _boost._beta_ppf(q, a, b)
+        return sc.betaincinv(a, b, q)
 
     def _stats(self, a, b):
         return (
diff --git a/scipy/stats/tests/test_distributions.py b/scipy/stats/tests/test_distributions.py
index 6d91bb8a6..de26dc3d8 100644
--- a/scipy/stats/tests/test_distributions.py
+++ b/scipy/stats/tests/test_distributions.py
@@ -4454,7 +4454,7 @@ class TestBeta:
         assert_equal(stats.beta.pdf(1, a, b), 5)
         assert_equal(stats.beta.pdf(1-1e-310, a, b), 5)
 
-    @pytest.mark.xfail(IS_PYPY, reason="Does not convert boost warning")
+    @pytest.mark.xfail(reason="Does not warn on special codepath")
     def test_boost_eval_issue_14606(self):
         q, a, b = 0.995, 1.0e11, 1.0e13
         with pytest.warns(RuntimeWarning):

I was able to confirm failures with invalid value encountered in _beta_ppf before the patch and fix after using clang-16 locally on ARM Mac.

For the inverse survival function _isf it must be return sc.betainccinv(a, b, x) instead, besides that the patch looks fine. Agree that sorting out everything that happened in special would be a big pain to backport, so let's keep it simple like this.

grlee77 and others added 13 commits May 15, 2024 11:40
* MAINT: stats: update boost to fix improve `skewnorm.ppf`
…d tests (scipy#20444)

* test 1d input to sparse. Add FutureWarnings and ValueErrors

* remove matrix changes. Let them create 2D

* correct imports

* rebase on main and update to support 1D CSR input
…oords) (scipy#20687)

* test and then fix duplicates for CSR/CSC creation from (data,coords)

* remove has_canonical_format check when summing duplicates.
Showed up as a linting error in an unrelated PR for me:
```
scipy/interpolate/_interpolate.py:918:30: UP032 [*] Use f-string instead of `format` call
scipy/interpolate/_interpolate.py:1972:30: UP032 [*] Use f-string instead of `format` call
```
This should not happen; the old code is fine, so this check needs to be
silenced or fixed separately. Similar to scipygh-20601.

[skip ci]
* Fixes scipygh-19423

* Add a few more `case` statements to account for
the (i.e., Windows) data types that don't have a fixed
width, and add a regression test.

[skip circle] [skip cirrus]
* some minor import fixes following a large
series of backports
* more import cleanups after backport activity
to make the linter happy
@tylerjereddy
Copy link
Contributor Author

tylerjereddy commented May 15, 2024

I've added substantially more backports and a few manual shims--we're back at parity with merged backport-labeled PRs.

Let's see where the regular CI lands. I did have to revert and unlabel one PR (gh-20643) based on local testing with clang-16 on Mac, but the rest made it in, so far. I expanded the TODO list above based on known CI failure in main, etc.

Edit: between gh-20727 and gh-20533, I suspect we'll be able to get the regular CI passing today.

thalassemia and others added 3 commits May 16, 2024 11:47
* We're seeing CI failures related to an undesirable
bump to Python `3.12` in this job, when the intention
was clearly to respect the Python version specific
in the GHA matrix. I didn't check too closely why
exactly it suddenly started happening, but some
packages weren't ready for `3.12` yet on this
job (`scikit-umfpack` in particular) and I don't
see too much harm in adding an extra pin to
respect the intention for the Python version.
* attempt to fix `M1 test - openblas` MacOS ARM CI job
based on some `gfortran` shims applied to a similar
job on `main`
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