Opened 21 months ago
Closed 7 months ago
#30350 closed enhancement (fixed)
Remove ATLAS
Reported by: | mkoeppe | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-9.5 |
Component: | packages: standard | Keywords: | |
Cc: | jhpalmieri, dimpase, fbissey, jpflori | Merged in: | |
Authors: | John Palmieri | Reviewers: | Matthias Koeppe |
Report Upstream: | N/A | Work issues: | |
Branch: | 272424a (Commits, GitHub, GitLab) | Commit: | 272424a33ba1a68b221b160ebbbc9c89852aa464 |
Dependencies: | Stopgaps: |
Description (last modified by )
(from #19719)
Our version of ATLAS is outdated and has not been tested in a long time. Latest upstream version as of 2021-09-21 is 3.11.40, released 2018-10-02, see https://github.com/math-atlas/math-atlas/releases
We remove this package.
However, we keep the makefile variable BLAS
(used in packages' dependencies
files), rather than hardcoding openblas in dependencies. It will make it easier to plug in another BLAS implementation if one comes along in the future, and it is also needed for #29387.
Change History (29)
comment:1 Changed 21 months ago by
- Cc fbissey jpflori added
comment:2 Changed 19 months ago by
- Milestone changed from sage-9.2 to sage-9.3
comment:3 Changed 16 months ago by
- Branch set to u/jhpalmieri/remove-atlas
comment:4 follow-ups: ↓ 12 ↓ 13 Changed 16 months ago by
- Commit set to 3c3022ce28961e3f75bb729525f2b1a05834807f
comment:5 Changed 16 months ago by
- Commit changed from 3c3022ce28961e3f75bb729525f2b1a05834807f to 0afdab5ef5a560c75f737ca7274635d268600f89
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
0afdab5 | trac 30350: remove the ATLAS package
|
comment:6 Changed 16 months ago by
comment:7 Changed 14 months ago by
- Milestone changed from sage-9.3 to sage-9.4
Sage development has entered the release candidate phase for 9.3. Setting a new milestone for this ticket based on a cursory review of ticket status, priority, and last modification date.
comment:8 Changed 10 months ago by
- Milestone changed from sage-9.4 to sage-9.5
comment:9 Changed 8 months ago by
I support removing this package in 9.5. The branch needs rebasing
comment:10 Changed 8 months ago by
- Commit changed from 0afdab5ef5a560c75f737ca7274635d268600f89 to 8ba1d79f84e6030c01ede985e813e35a3d180feb
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
8ba1d79 | trac 30350: remove the ATLAS package
|
comment:11 Changed 8 months ago by
Rebased. I deleted the reference to ATLAS in the cygwin instructions in src/doc/en/installation/source.rst
, but the other questions in comment:4 remain.
comment:12 in reply to: ↑ 4 Changed 8 months ago by
Replying to jhpalmieri:
- what to do with
openblas/spkg-configure.m4
? Any changes necessary?
We could either keep the --with-blas=...
option but make it an error if anything other than openblas
is passed, or remove it.
comment:13 in reply to: ↑ 4 Changed 8 months ago by
Replying to jhpalmieri:
- what about the
spkg-install.in
scripts forscipy
andnumpy
? They refer to anATLAS
environment variable. Leave as is or change?
No change needed. The scripts just set the environment variable to a safe value so the installation does not pick up random things that may be in the user's environment. (Not sure why the default is different for macOS and for other systems.)
comment:14 Changed 8 months ago by
The changes to the documentation look fine.
comment:15 Changed 8 months ago by
Is this the way to remove the --with-blas
option?
-
build/pkgs/openblas/spkg-configure.m4
diff --git a/build/pkgs/openblas/spkg-configure.m4 b/build/pkgs/openblas/spkg-configure.m4 index 177bbb1d4f..c81533ba71 100644
a b SAGE_SPKG_CONFIGURE([openblas], [ 110 110 LIBS="$SAVE_LIBS" 111 111 CFLAGS="$SAVE_CFLAGS" 112 112 ]) 113 ], [ 114 dnl REQUIRED-CHECK 115 AS_IF([test "x$with_blas" = xopenblas], [ 116 sage_require_openblas=yes 117 sage_require_atlas=no]) 118 ], [ 119 dnl PRE 120 AC_MSG_CHECKING([BLAS library]) 121 AC_ARG_WITH([blas], 122 [AS_HELP_STRING([--with-blas=openblas], 123 [use OpenBLAS as BLAS library (default)])] 124 [AS_HELP_STRING([--with-blas=atlas], 125 [use ATLAS as BLAS library])],, 126 [with_blas=openblas] # default 127 ) 128 AS_CASE(["$with_blas"], 129 [openblas], [], 130 [atlas], [sage_spkg_install_openblas=no], 131 [AC_MSG_ERROR([allowed values for --with-blas are 'atlas' and 'openblas'])]) 132 AC_MSG_RESULT([$with_blas]) 133 AC_SUBST([SAGE_BLAS], [$with_blas]) 134 ] 113 ] 135 114 )
comment:16 Changed 8 months ago by
Yes, that looks right
comment:17 Changed 8 months ago by
After removing, this line:
build/make/Makefile.in:BLAS = @SAGE_BLAS@
needs changing
comment:18 Changed 8 months ago by
Can I just change it to BLAS = openblas
?
comment:19 Changed 8 months ago by
Yes
comment:20 Changed 8 months ago by
- Commit changed from 8ba1d79f84e6030c01ede985e813e35a3d180feb to 69db83912a25f1cfa4341df59b2a5a45bb27a3cb
Branch pushed to git repo; I updated commit sha1. New commits:
69db839 | trac 30350: remove "--with-blas=" option to ./configure
|
comment:21 Changed 8 months ago by
Okay, here are those changes.
comment:22 Changed 8 months ago by
build/pkgs/atlas/distros/
still contains some stuff
comment:23 Changed 8 months ago by
- Commit changed from 69db83912a25f1cfa4341df59b2a5a45bb27a3cb to 272424a33ba1a68b221b160ebbbc9c89852aa464
Branch pushed to git repo; I updated commit sha1. New commits:
272424a | trac 30350: remove remaining bits in build/pkgs/atlas/distros
|
comment:24 Changed 8 months ago by
Fixed, thanks.
comment:25 Changed 8 months ago by
- Description modified (diff)
- Status changed from new to needs_review
comment:26 Changed 8 months ago by
- Description modified (diff)
comment:27 Changed 8 months ago by
- Reviewers set to Matthias Koeppe
- Status changed from needs_review to positive_review
This seems to work well.
comment:28 Changed 8 months ago by
Thank you!
comment:29 Changed 7 months ago by
- Branch changed from u/jhpalmieri/remove-atlas to 272424a33ba1a68b221b160ebbbc9c89852aa464
- Resolution set to fixed
- Status changed from positive_review to closed
Here is an attempt. I have questions:
openblas/spkg-configure.m4
? Any changes necessary?spkg-install.in
scripts forscipy
andnumpy
? They refer to anATLAS
environment variable. Leave as is or change?src/doc/en/installation/source.rst
.Anyway, this is a start.
New commits:
trac 30350: remove the ATLAS package