Opened 4 years ago

Closed 3 years ago

Last modified 3 years ago

#24669 closed defect (fixed)

Re-enable threads on OpenBLAS

Reported by: embray Owned by: embray
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 embray)

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 4 years ago by git

  • Commit changed from fb0b09f11e95d62b24fe9f142a100fcc4bda4712 to edaf23506300e80205bb5275bffad7575df32062

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

edaf235Add patch for supporting threads + fork() on Cygwin

comment:2 Changed 4 years ago by embray

  • Description modified (diff)
  • Status changed from new to needs_review

New commits:

edaf235Add patch for supporting threads + fork() on Cygwin

comment:3 Changed 4 years ago by vbraun

  • Reviewers set to Volker Braun
  • Status changed from needs_review to positive_review

comment:4 Changed 4 years ago by embray

Thanks--this fix will be nice to have.

comment:5 Changed 3 years ago by embray

  • Milestone changed from sage-8.2 to sage-8.3

comment:6 Changed 3 years ago by vbraun

  • Status changed from positive_review to needs_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 3 years ago by embray

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

comment:8 Changed 3 years ago by git

  • Commit changed from edaf23506300e80205bb5275bffad7575df32062 to 115685b7c25b0bfd3e59802c402be42a99ed21a2

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 3 years ago by embray

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 3 years ago by embray (previous) (diff)

comment:10 Changed 3 years ago by embray

  • Milestone changed from sage-8.3 to sage-8.4

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

comment:11 Changed 3 years ago by gh-timokau

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 3 years ago by embray

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 3 years ago by embray

  • Owner changed from (none) to embray

comment:14 Changed 3 years ago by git

  • Commit changed from 115685b7c25b0bfd3e59802c402be42a99ed21a2 to ce4173b0ebbdc41d0695f31f4bcfc0106498a3fa

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 3 years ago by embray

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 3 years ago by git

  • Commit changed from ce4173b0ebbdc41d0695f31f4bcfc0106498a3fa to 2b52932cd8979cb0e06bd3fc91184cb8bc06ac25

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

2b52932also remove USE_THREAD=0 from spkg-check

comment:17 Changed 3 years ago by embray

  • Status changed from needs_work to needs_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 3 years ago by saraedum

  • Reviewers changed from Volker Braun to Volker Braun, Julian Rüth
  • Status changed from needs_review to positive_review

comment:19 Changed 3 years ago by vbraun

  • Branch changed from u/embray/openblas/threads to 2b52932cd8979cb0e06bd3fc91184cb8bc06ac25
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:20 Changed 3 years ago by saraedum

  • Commit 2b52932cd8979cb0e06bd3fc91184cb8bc06ac25 deleted

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

Note: See TracTickets for help on using tickets.