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 #16467

Merged
merged 1 commit into from
Apr 3, 2023
Merged

Conversation

MehdiChinoune
Copy link
Collaborator

No description provided.

@MehdiChinoune
Copy link
Collaborator Author

package-grokker lists these package to be rebuilt:
mingw-w64-octave
mingw-w64-qrupdate
mingw-w64-arpack
mingw-w64-scalapack
mingw-w64-trilinos
mingw-w64-python-cvxopt
mingw-w64-superlu
mingw-w64-mumps
mingw-w64-openturns
mingw-w64-python-scipy
mingw-w64-slepc
mingw-w64-petsc
mingw-w64-suitesparse

The upstream didn't mention any breaking changes!
Most of them depends on Fortran, maybe some Fortran interfaces were changed after they update their intern Lapack to 3.11

@MehdiChinoune
Copy link
Collaborator Author

mumps download link is unreachable, its website requires filling a form to download https://mumps-solver.org/index.php?page=dwnld, What should we do?

@lazka
Copy link
Member

lazka commented Mar 28, 2023

The Arch package uses https://graal.ens-lyon.fr/MUMPS/MUMPS_5.5.0.tar.gz

@mmuetzel
Copy link
Collaborator

mmuetzel commented Mar 29, 2023

Several downstream projects are experiencing issues with OpenBLAS 0.3.22. E.g.:
scipy/scipy#18208
sagemath/sage#35371
OpenMathLib/OpenBLAS#3976

Maybe, we should wait for upstream OpenBLAS to fix those issues before we update to that (or a newer) version.

@mmuetzel
Copy link
Collaborator

mmuetzel commented Mar 29, 2023

Regarding the package grokker results: All of those symbols have subsequent ordinals. The first one is symbol 8193 in the new library. That is 2^13+1 or 0x2001. The last one is at 8192 (0x2000) in the old library. Maybe some kind of buffer overflow or mismatch?

Edit: Anyway, I don't think we need to rebuild any dependencies. That looks like a false positive result from the package grokker.

@MehdiChinoune
Copy link
Collaborator Author

@jeremyd2019

@jeremyd2019
Copy link
Member

jeremyd2019 commented Mar 29, 2023

I don't think I gave any consideration to ordinals in package-grokker. I think it should be looking solely at the names, but perhaps I need to debug it with libraries that actually use ordinals. (I explicitly didn't handle ordinal-only exports)

@mmuetzel
Copy link
Collaborator

mmuetzel commented Mar 30, 2023

I don't think I gave any consideration to ordinals in package-grokker. I think it should be looking solely at the names, but perhaps I need to debug it with libraries that actually use ordinals. (I explicitly didn't handle ordinal-only exports)

I don't know where to find the sources for the package grokker. So, the following relies on a couple of assumptions:
Even if you don't work with ordinals explicitly, it's likely that most tools would return the symbols that are exported from a library ordered by ordinal.
I'm guessing that the package grokker performs some kind of set difference operation. I.e., it likely collects the set of symbol names exported from the current library and a set of symbol names from the updated library. It likely checks if the set difference (i.e., set from current library "without" the set from the updated library) is empty, and prints the set difference if it is non-empty.
Maybe, the algorithm used to calculate the set difference segments the original sets into subsets (e.g., to save computation time, memory, ...).
Let's say the set of symbols from the current library is set A. It might be segmented into subsets A1, A2, ... Equivalently for the updated library: set B might be segmented into subsets B1, B2, ...
The package grokker probably tries to calculate the set difference A \ B. It (or the underlying algorithm) might be doing that by splitting that operation into subsets like this: (A1 \ B1) U (A2 \ B2) U ... (where U should indicate a set union). But that doesn't lead to the correct results if elements from A1 "spilled over" to B2.

@MehdiChinoune
Copy link
Collaborator Author

I don't know where to find the sources for the package grokker.

https://github.com/jeremyd2019/package-grokker

@mmuetzel
Copy link
Collaborator

I can't reproduce that with this simple python script:

import random
import string
def get_random_string(length):
    letters = string.printable;
    result_str = ''.join(random.choice(letters) for i in range(random.choice(range(1,length))));
    return result_str;

random.seed(42);
a = set([get_random_string(200) for i in range(2^8000+800)]);
random.seed(42);
b = set([get_random_string(200) for i in range(2^8000+815)]);
print(a-b)

I don't know why the package grokker picked out those symbols...
Maybe, something with pefile when the number of symbols is large?

@jeremyd2019
Copy link
Member

ok, pefile has a default limit of processing 0x2000 exports, presumably to avoid getting DoS'ed in cases where it's being used in a virus scanner type situation where the DLL may be intentionally malformed. Probably should start an issue against package-grokker repo to track, getting off-topic for openblas pull request.

@jeremyd2019
Copy link
Member

package grokker is clear for this package now that the 0x2000 limit is increased.

@MehdiChinoune
Copy link
Collaborator Author

I have backported two patches that should fix issues with scipy and octave, should we wait more?

@mmuetzel
Copy link
Collaborator

If I understand the comments on the OpenBLAS issue correctly, there is still a regression in scipy that is currently being tracked down.
Maybe, we should still wait for that.

@MehdiChinoune MehdiChinoune changed the title openblas: update to 0.3.22 openblas: update to 0.3.23 Apr 1, 2023
@MehdiChinoune MehdiChinoune merged commit 7d36670 into msys2:master Apr 3, 2023
@MehdiChinoune MehdiChinoune deleted the openblas-update branch April 3, 2023 04:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants