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

openblas: Update to 0.3.23 #35371

Merged
merged 1 commit into from
May 22, 2023
Merged

openblas: Update to 0.3.23 #35371

merged 1 commit into from
May 22, 2023

Conversation

mkoeppe
Copy link
Member

@mkoeppe mkoeppe commented Mar 28, 2023

📚 Description

https://github.com/xianyi/OpenBLAS/releases/tag/v0.3.23

📝 Checklist

  • The title is concise, informative, and self-explanatory.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation accordingly.

⌛ Dependencies

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 28, 2023

(Updating to 0.3.22) Testing on macOS, I'm getting a bunch of errors like this:

sage -t --random-seed=88906055599883703380957581517190276719 src/sage/matrix/matrix_double_dense.pyx
**********************************************************************
File "src/sage/matrix/matrix_double_dense.pyx", line 476, in sage.matrix.matrix_double_dense.Matrix_double_dense.condition
Failed example:
    id.condition(p=1)
Expected:
    1.0
Got:
    +Infinity
**********************************************************************
File "src/sage/matrix/matrix_double_dense.pyx", line 521, in sage.matrix.matrix_double_dense.Matrix_double_dense.condition
Failed example:
    c = A.norm(2)*A.inverse().norm(2)
Exception raised:
    Traceback (most recent call last):
      File "sage/matrix/matrix_double_dense.pyx", line 345, in sage.matrix.matrix_double_dense.Matrix_double_dense.__invert__
        M._matrix_numpy = scipy.linalg.inv(self._matrix_numpy)
      File "/Users/mkoeppe/s/sage/sage-rebasing/worktree-clean/local/var/lib/sage/venv-python3.10/lib/python3.10/site-packages/scipy/linalg/_basic.py", line 975, in inv
        raise LinAlgError("singular matrix")
    numpy.linalg.LinAlgError: singular matrix

    During handling of the above exception, another exception occurred:

    Traceback (most recent call last):
      File "/Users/mkoeppe/s/sage/sage-rebasing/worktree-clean/src/sage/doctest/forker.py", line 695, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/Users/mkoeppe/s/sage/sage-rebasing/worktree-clean/src/sage/doctest/forker.py", line 1093, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.matrix.matrix_double_dense.Matrix_double_dense.condition[35]>", line 1, in <module>
        c = A.norm(Integer(2))*A.inverse().norm(Integer(2))
      File "sage/matrix/matrix2.pyx", line 9964, in sage.matrix.matrix2.Matrix.inverse
        return ~self
      File "sage/matrix/matrix_double_dense.pyx", line 347, in sage.matrix.matrix_double_dense.Matrix_double_dense.__invert__
        raise ZeroDivisionError("input matrix must be nonsingular")
    ZeroDivisionError: input matrix must be nonsingular

See also https://groups.google.com/g/sage-devel/c/YCVbmY6seIs

@mkoeppe
Copy link
Member Author

mkoeppe commented Mar 28, 2023

Updating numpy to 1.24.2 (via #35081) does not help with this.

@dimpase
Copy link
Member

dimpase commented Mar 28, 2023

What do scipy people do about it? They depend on OpenBLAS.

@jhpalmieri
Copy link
Member

Independently of this ticket, I made my own branch with an upgrade of openblas. For what it's worth, the documentation built for me (didn't see the error that arose using homebrew's openblas) but I do see the doctest failures.

@tornaria
Copy link
Contributor

tornaria commented Apr 3, 2023

openblas 0.3.23 was released, and all doctests in sage pass now.

vbraun pushed a commit that referenced this pull request Apr 6, 2023
    
<!-- Please provide a concise, informative and self-explanatory title.
-->
<!-- Don't put issue numbers in the title. Put it in the Description
below. -->
<!-- For example, instead of "Fixes #12345", use "Add a new method to
multiply two integers" -->

### 📚 Description
openblas 0.3.22 is broken, see:
- #35371
- scipy/scipy#18208
- OpenMathLib/OpenBLAS#3976

We reject it.

<!-- Describe your changes here in detail. -->
<!-- Why is this change required? What problem does it solve? -->
<!-- If this PR resolves an open issue, please link to it here. For
example "Fixes #12345". -->
<!-- If your change requires a documentation PR, please link it
appropriately. -->

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. It should be `[x]` not `[x
]`. -->

- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation accordingly.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on
- #12345: short description why this is a dependency
- #34567: ...
-->

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: #35377
Reported by: Matthias Köppe
Reviewer(s): John H. Palmieri
@jhpalmieri
Copy link
Member

Using the latest openblas 0.3.23 on homebrew works for me on a machine that gave me problems before (as noted on #35377).

@mkoeppe mkoeppe changed the title openblas: Update to 0.3.22 openblas: Update to 0.3.23 May 8, 2023
@mkoeppe
Copy link
Member Author

mkoeppe commented May 8, 2023

@github-actions
Copy link

github-actions bot commented May 8, 2023

Documentation preview for this PR is ready! 🎉
Built with commit: 27ac34f

@jhpalmieri
Copy link
Member

Does spkg-configure.m4 need to be changed to allow the use of version 0.3.23 from the system? I don't know m4 syntax so I don't know if only 0.3.22 is being rejected or all versions 0.3.22 and later.

@mkoeppe
Copy link
Member Author

mkoeppe commented May 9, 2023

That was done in #35524 (already merged)

@jhpalmieri
Copy link
Member

Let's proceed with this.

@mkoeppe
Copy link
Member Author

mkoeppe commented May 17, 2023

Thanks!

@mkoeppe mkoeppe removed this from the sage-10.0 milestone May 20, 2023
@mkoeppe mkoeppe added this to the sage-10.1 milestone May 20, 2023
@vbraun vbraun merged commit b147e29 into sagemath:develop May 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants