Opened 2 years ago

Closed 14 months ago

#30350 closed enhancement (fixed)

Remove ATLAS

Reported by: Matthias Köppe Owned by:
Priority: major Milestone: sage-9.5
Component: packages: standard Keywords:
Cc: John Palmieri, Dima Pasechnik, François Bissey, Jean-Pierre Flori Merged in:
Authors: John Palmieri Reviewers: Matthias Koeppe
Report Upstream: N/A Work issues:
Branch: 272424a (Commits, GitHub, GitLab) Commit: 272424a33ba1a68b221b160ebbbc9c89852aa464
Dependencies: Stopgaps:

Status badges

Description (last modified by Matthias Köppe)

(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 2 years ago by Matthias Köppe

Cc: François Bissey Jean-Pierre Flori added

comment:2 Changed 2 years ago by Matthias Köppe

Milestone: sage-9.2sage-9.3

comment:3 Changed 22 months ago by John Palmieri

Branch: u/jhpalmieri/remove-atlas

comment:4 Changed 22 months ago by John Palmieri

Commit: 3c3022ce28961e3f75bb729525f2b1a05834807f

Here is an attempt. I have questions:

  • what to do with openblas/spkg-configure.m4? Any changes necessary?
  • what about the spkg-install.in scripts for scipy and numpy? They refer to an ATLAS environment variable. Leave as is or change?
  • I don't know anything about cygwin, so I don't know what to do about the installation instructions referring to ATLAS in src/doc/en/installation/source.rst.

Anyway, this is a start.


New commits:

3c3022ctrac 30350: remove the ATLAS package

comment:5 Changed 22 months ago by git

Commit: 3c3022ce28961e3f75bb729525f2b1a05834807f0afdab5ef5a560c75f737ca7274635d268600f89

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

0afdab5trac 30350: remove the ATLAS package

comment:6 Changed 22 months ago by Matthias Köppe

Authors: John Palmieri

comment:7 Changed 20 months ago by Matthias Köppe

Milestone: sage-9.3sage-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 17 months ago by Matthias Köppe

Milestone: sage-9.4sage-9.5

comment:9 Changed 14 months ago by Matthias Köppe

I support removing this package in 9.5. The branch needs rebasing

comment:10 Changed 14 months ago by git

Commit: 0afdab5ef5a560c75f737ca7274635d268600f898ba1d79f84e6030c01ede985e813e35a3d180feb

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

8ba1d79trac 30350: remove the ATLAS package

comment:11 Changed 14 months ago by John Palmieri

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 14 months ago by Matthias Köppe

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 14 months ago by Matthias Köppe

Replying to jhpalmieri:

  • what about the spkg-install.in scripts for scipy and numpy? They refer to an ATLAS 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 14 months ago by Matthias Köppe

The changes to the documentation look fine.

comment:15 Changed 14 months ago by John Palmieri

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], [ 
    110110  LIBS="$SAVE_LIBS"
    111111  CFLAGS="$SAVE_CFLAGS"
    112112 ])
    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 ]
    135114)

comment:16 Changed 14 months ago by Matthias Köppe

Yes, that looks right

comment:17 Changed 14 months ago by Matthias Köppe

After removing, this line:

build/make/Makefile.in:BLAS = @SAGE_BLAS@

needs changing

comment:18 Changed 14 months ago by John Palmieri

Can I just change it to BLAS = openblas?

comment:19 Changed 14 months ago by Matthias Köppe

Yes

comment:20 Changed 14 months ago by git

Commit: 8ba1d79f84e6030c01ede985e813e35a3d180feb69db83912a25f1cfa4341df59b2a5a45bb27a3cb

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

69db839trac 30350: remove "--with-blas=" option to ./configure

comment:21 Changed 14 months ago by John Palmieri

Okay, here are those changes.

comment:22 Changed 14 months ago by Matthias Köppe

build/pkgs/atlas/distros/ still contains some stuff

comment:23 Changed 14 months ago by git

Commit: 69db83912a25f1cfa4341df59b2a5a45bb27a3cb272424a33ba1a68b221b160ebbbc9c89852aa464

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

272424atrac 30350: remove remaining bits in build/pkgs/atlas/distros

comment:24 Changed 14 months ago by John Palmieri

Fixed, thanks.

comment:25 Changed 14 months ago by Matthias Köppe

Description: modified (diff)
Status: newneeds_review

comment:26 Changed 14 months ago by Matthias Köppe

Description: modified (diff)

comment:27 Changed 14 months ago by Matthias Köppe

Reviewers: Matthias Koeppe
Status: needs_reviewpositive_review

This seems to work well.

comment:28 Changed 14 months ago by John Palmieri

Thank you!

comment:29 Changed 14 months ago by Volker Braun

Branch: u/jhpalmieri/remove-atlas272424a33ba1a68b221b160ebbbc9c89852aa464
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.