Opened 5 years ago

Closed 4 years ago

Last modified 4 years ago

#24669 closed defect (fixed)

Re-enable threads on OpenBLAS

Reported by: Erik Bray Owned by: Erik Bray
Priority: major Milestone: sage-8.4
Component: packages: standard Keywords: openblas cygwin
Cc: Merged in:
Authors: Erik Bray Reviewers: Volker Braun, Julian Rüth
Report Upstream: N/A Work issues:
Branch: 2b52932 (Commits, GitHub, GitLab) Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by Erik Bray)

It was disabled in #22021 to work around some deadlocks that were occurring, and still further problems were reported in #23933.

But these issues appear to be fixed as of OpenBLAS 0.2.20 which Sage is now on. It would be good if more people can test this though (especially on OSX).

This also includes a patch for Cygwin without which this change renders openblas broken on Cygwin. This will resolve issues like #22822 (of course, it doesn't actually fix the Cygwin openblas package if it gets used by mistake, but that will be fixed too if upstream accepts my patch).

Change History (20)

comment:1 Changed 5 years ago by git

Commit: fb0b09f11e95d62b24fe9f142a100fcc4bda4712edaf23506300e80205bb5275bffad7575df32062

Branch pushed to git repo; I updated commit sha1. New commits:

edaf235Add patch for supporting threads + fork() on Cygwin

comment:2 Changed 5 years ago by Erik Bray

Description: modified (diff)
Status: newneeds_review

New commits:

edaf235Add patch for supporting threads + fork() on Cygwin

comment:3 Changed 5 years ago by Volker Braun

Reviewers: Volker Braun
Status: needs_reviewpositive_review

comment:4 Changed 5 years ago by Erik Bray

Thanks--this fix will be nice to have.

comment:5 Changed 5 years ago by Erik Bray

Milestone: sage-8.2sage-8.3

comment:6 Changed 5 years ago by Volker Braun

Status: positive_reviewneeds_work

Breaks the testsuite

$ SAGE_CHECK=yes sage -p openblas
[...]
gfortran -O2 -Wall -m64  -L/mnt/disk/home/release/Sage/local/lib -Wl,-rpath,/mnt/disk/home/release/Sage/local/lib  -o cblat1 cblat1.o ../libopenblas_haswell-r0.2.20.a -lm -lgfortran -lm -lgfortran 
gfortran -O2 -Wall -m64  -L/mnt/disk/home/release/Sage/local/lib -Wl,-rpath,/mnt/disk/home/release/Sage/local/lib  -o zblat1 zblat1.o ../libopenblas_haswell-r0.2.20.a -lm -lgfortran -lm -lgfortran 
../libopenblas_haswell-r0.2.20.a: file not recognized: File truncated
collect2: error: ld returned 1 exit status
make[1]: *** [Makefile:138: cblat1] Error 1

comment:7 Changed 5 years ago by Erik Bray

I'm not convinced yet that that has anything to do with this patch (especially considering it was accepted upstream.

comment:8 Changed 5 years ago by git

Commit: edaf23506300e80205bb5275bffad7575df32062115685b7c25b0bfd3e59802c402be42a99ed21a2

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

115685bRe-enable USE_THREADS for openblas, since the issues this was working around appear to be fixed; see

comment:9 Changed 5 years ago by Erik Bray

Well, I was able to reproduce it so I guess I'm convinced now :) But I don't see any obvious mechanism by which this would cause such an issue...

Last edited 5 years ago by Erik Bray (previous) (diff)

comment:10 Changed 4 years ago by Erik Bray

Milestone: sage-8.3sage-8.4

I believe this issue can reasonably be addressed for Sage 8.4.

comment:11 Changed 4 years ago by Timo Kaufmann

For what it's worth, threads are not disabled in nix's openblas and all our tests (sage and other packages) pass.

comment:12 Changed 4 years ago by Erik Bray

Yeah, I'm not sure why this is causing problems when running the test suite. I suspect it's also an upstream issue though, or an oddity in how we run the tests; there should be no problem normally for enabling threads...

I just haven't bothered to look into this yet, but this really should be fixed.

comment:13 Changed 4 years ago by Erik Bray

Owner: set to Erik Bray

comment:14 Changed 4 years ago by git

Commit: 115685b7c25b0bfd3e59802c402be42a99ed21a2ce4173b0ebbdc41d0695f31f4bcfc0106498a3fa

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

ce4173bRe-enable USE_THREADS for openblas, since the issues this was working around appear to be fixed; see

comment:15 Changed 4 years ago by Erik Bray

Rebased on 8.4.beta0; not that there was a merge conflict or anything (it's a very simple change), but just because the original commit was pretty old...

comment:16 Changed 4 years ago by git

Commit: ce4173b0ebbdc41d0695f31f4bcfc0106498a3fa2b52932cd8979cb0e06bd3fc91184cb8bc06ac25

Branch pushed to git repo; I updated commit sha1. New commits:

2b52932also remove USE_THREAD=0 from spkg-check

comment:17 Changed 4 years ago by Erik Bray

Status: needs_workneeds_review

Turns out the answer was quite simple: We also just had to remove USE_THREAD=0 from the spkg-check script :|

comment:18 Changed 4 years ago by Julian Rüth

Reviewers: Volker BraunVolker Braun, Julian Rüth
Status: needs_reviewpositive_review

comment:19 Changed 4 years ago by Volker Braun

Branch: u/embray/openblas/threads2b52932cd8979cb0e06bd3fc91184cb8bc06ac25
Resolution: fixed
Status: positive_reviewclosed

comment:20 Changed 4 years ago by Julian Rüth

Commit: 2b52932cd8979cb0e06bd3fc91184cb8bc06ac25

I believe that this broke sage -tp on some machines, see #26118.

Note: See TracTickets for help on using tickets.