Update GSL to the latest upstream release (1.14) & permit parallel building.

[sagetrac-drkirkby]

The version of the GNU Scientific Library (GSL) in Sage is 1.10, which is almost 3 years old. The latest, 1.14 was released about 4 months ago.

There is also a large number of bugs in the spkg-check and spkg-install files

• spkg-check did not exit with a non-zero error code if the make check failed. It did however report the error, but it is highly likely to be missed in a large log file. This issue was reported at spkg-check for gsl does not cause an exit on errors. #9531, so that ticket can be closed when this one is closed.
• spkg-install did not exit if configure failed to configure properly. Again the error was reported.
• spkg-install did not exit if make failed to build GSL correctly. Again the error was reported.
• spkg-install did not exit if make install failed to install GSL properly. Again the error was reported.
• The self-tests were failing on some platforms, due to the fact /bin/rm: cannot remove `libtoolT. This was also true of the latest version, but exporting RM to "rm -f" solved that, as suggested at http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=523750

== Note to the release managers ==

The positively reviewed package can be found here.

http://boxen.math.washington.edu/home/kirkby/patches/gsl-1.14.spkg

All patches are in the repository in the .spkg - nothing needs to be added to the Sage library.

CC: @nexttime @qed777

Component: packages: standard

Author: David Kirkby

Reviewer: Mike Hansen, Peter Jeremy, Leif Leonhardy

Merged: sage-4.5.3.alpha0

Issue created by migration from https://trac.sagemath.org/ticket/9533

[qed777]

comment:1

A possibility for another ticket: Checking whether we can replace make with $MAKE in GSL's spkg-install.

[sagetrac-drkirkby]

comment:2

Replying to @qed777:

A possibility for another ticket: Checking whether we can replace make with $MAKE in GSL's spkg-install.

I'm pretty sure we can, since I have built this multiple times on my Sun Blade 2000 SPARC with 2 threads and on my Sun Ultra 27 with 12 threads. This is outside the Sage environment, so my settings for MAKE would have been used. There were no problems building on either in parallel, so I don't think it will be a problem building with $MAKE.

I'll test that before uploading a patch and providing a link.

Dave

[sagetrac-drkirkby]

comment:3

I'm convinced this is ok built in parallel. I tested it several times in parallel (outside of Sage) before you even mentioned it. But after you said this, I used $MAKE in spkg-install and systematically checked it on different systems

• 19 parallel builds on a Sun Ultra 27, with OpenSolaris using between 2 and 1000 threads.
• 5 parallel builds on bsd.math, with OS 10.6, using 4 threads.
• 5 parallel builds on sage.math, with Ubunta, using 8 threads.
• 3 parallel builds on a Sun Blade 2000, with Solaris 10, using between 2 and 4 threads. (Code compiled 64-bit)
• 3 parallel builds on a Sun Blade 2000, with Solaris 10, using between 2 and 4 threads. (Code compiled 32-bit)

In all cases, all the self-tests for GSL passed.

I've run the doctests on sage.math. I was quite expecting to get a few failures due to different results from different algorithms that might be used in the GSL library, but to my surprise:

----------------------------------------------------------------------
All tests passed!
Total time for all tests: 1095.8 seconds
kirkby@sage:/scratch/kirkby/sage-4.5$ 

Here's a link to the package.

http://boxen.math.washington.edu/home/kirkby/patches/gsl-1.14.spkg

I'm going to upload 3 patches. The first was all the updates. The second uses {{{$MAKE}} and the final one just prints an informative message when the tests pass. Is there a sensible way of reversing these patches once they are commited, so I don't need 3 of them? Anyway, the patches are for review only. They don't need to be applied to the package.

The first patch is quite large, as it removes a big patch.

Dave
Dave

[sagetrac-drkirkby]

Attachment: 9533.patch.gz

Main patch, which makes many changes to clean up the spkg-check and spkg-install.

[sagetrac-drkirkby]

Attachment: 9533.use-Dollar-MAKE.patch.gz

Changes 'make' to '$MAKE' to allow faster parallel builds. This has been extensively tested

[sagetrac-drkirkby]

Add a message to indicate the tests have passed.

[sagetrac-drkirkby]

comment:4

Attachment: 9533.print-passed-message.patch.gz

[sagetrac-drkirkby]

Author: David Kirkby

[sagetrac-drkirkby]

comment:6

On bsd.math

$ make ptestlong

<SNIP> 

----------------------------------------------------------------------
All tests passed!
Total time for all tests: 2503.7 seconds
[kirkby@bsd sage-4.5.rc1]$ 

[nexttime]

comment:7

At first glance (I only looked at the patches) ok, except

• Append to CFLAGS et al. rather than set/overwrite them
• Typo: 1x s/builing/building/ (2nd patch SPKG.txt)
• Perhaps add to SPKG.txt that spkg-check also now uses $MAKE
• Move the "successful check" message out of the else part, right to the bottom ;-)

I don't know why you want to create a single patch, but you can simply take an old vanilla GSL spkg and apply your patches in order with hg import --no-commit ... (or use patch), and after that, just do a commit.

I'll take a closer look at the package tomorrow, I think.

[nexttime]

comment:8

Replying to @nexttime:

you can simply take an old vanilla GSL spkg and apply your patches in order with hg import --no-commit ... (or use patch), and after that, just do a commit.

Oh, I just noticed Mercurial doesn't like successive imports without commit, simply use patch < ... for the second and third.

[nexttime]

comment:9

Replying to @nexttime:

Oh, I just noticed Mercurial doesn't like successive imports without commit, simply use patch < ... for the second and third.

hg import -f --no-commit ... (three times) does the trick as well... :)

[sagetrac-drkirkby]

comment:10

Replying to @nexttime:

At first glance (I only looked at the patches) ok, except

• Append to CFLAGS et al. rather than set/overwrite them

OK.

• Typo: 1x s/builing/building/ (2nd patch SPKG.txt)

Cheers

• Perhaps add to SPKG.txt that spkg-check also now uses $MAKE

Yes.

• Move the "successful check" message out of the else part, right to the bottom ;-)

Yes, good idea. It saves a few bytes.

I don't know why you want to create a single patch

I think its more difficult to read patches when they are very small. I'd personally rather see one patch that fixes all the problems, rather than loads of patches fixing parts of it. But I'll just add another patch here.

Dave

[sagetrac-drkirkby]

Patch taking into account comments by reviewer, and a bit of a tidyup.

[sagetrac-drkirkby]

comment:12

Attachment: 9533-tidyup-and-resolve-flag-issues.patch.gz

Leif,
I've taken your comments into account. I realised that despite what src/INSTALL says, -g -O2 were not added. So I've added them back, as they were before. I also noticed the web site I gave for the solution to the libtool problem (export RM="rm -f") was not the correct one. So I updated the web link in the description of this, and also in SPKG.txt

I also reformatted SPKG.txt so no lines are wider than 80 characters.

I've replaced the package, so it can still be found at http://boxen.math.washington.edu/home/kirkby/patches/gsl-1.14.spkg

Dave

[sagetrac-drkirkby]

Slightlly simpler spkg-check

[nexttime]

comment:13

Attachment: 9533-improve-spkg-check.patch.gz

A few (more general) questions:

• Should we make sure that $SAGE_LOCAL is set? (in both spkg-install and spkg-check)
• Should optimization be disabled (-O0) if SAGE_DEBUG is yes as some other packages do?
• Anything to add if SAGE_FAT_BINARY is yes?
• I think CFLAGS et al. should be set in spkg-check as well, as make check usually involves compilation, too.
• Also modifying LDFLAGS, e.g. if SAGE64=yes, even if currently not directly used by upstream, should be safe.

P.S.: My intention regarding the else ... exit 0 part was rather avoiding other people adding "dead code" below it. Looking at the (fairly small) file in whole this seems less a danger... :)

[nexttime]

comment:14

Just for the record (mhansen?):
"GSL should compile with GCC under Cygwin on Microsoft Windows.There is a gsl package in the standard Cygwin distribution which contains any patches needed."

[sagetrac-drkirkby]

comment:15

Replying to @nexttime:

A few (more general) questions:

• Should we make sure that $SAGE_LOCAL is set? (in both spkg-install and spkg-check)

I think that would be wise. I will address that.

• Should optimization be disabled (-O0) if SAGE_DEBUG is yes as some other packages do?

To be practical, SAGE_DEBUG is not really going to be very useful, as too few packages use it. But I can make that change. Even if every .spkg in Sage used SAGE_DEBUG that way, many upstream packages add -O2 anyway.

• Anything to add if SAGE_FAT_BINARY is yes?

I do not believe so. This does not link with anything other than ATLAS.

• I think CFLAGS et al. should be set in spkg-check as well, as make check usually involves compilation, too.

In general you are right, but in this case it is not necessary. I know that, as I was using SAGE64 set to yes to add the -m64 option. That's a pretty critical option on 64-bit builds, but it was not necessary to add it. I assume that's in the Makefile which has already been created.

But I accept it would be safer, just in case GSL change the build process in some way. I am aware of cases where this has been an issue in spkg-check. I'll modify that.

• Also modifying LDFLAGS, e.g. if SAGE64=yes, even if currently not directly used by upstream, should be safe.

Yes, again, it may be safer to do this.

P.S.: My intention regarding the else ... exit 0 part was rather avoiding other people adding "dead code" below it. Looking at the (fairly small) file in whole this seems less a danger... :)

No problem. I did wonder if its best to exit a script with exit 0 rather than just let it exit. I know you can get away without specifying an exit code, but I'm not sure if it's good practice or not.

Dave

[sagetrac-drkirkby]

Attachment: 9533-add-SAGE_DEBUG-and_CFLAG64.patch.gz

Adds support for SAGE_DEBUG, CFLAG64, checks SAGE_ROOT and exports LDFLAGS on 64-bit builds

[sagetrac-drkirkby]

comment:16

I've made those changes. The package can be found here

http://boxen.math.washington.edu/home/kirkby/patches/gsl-1.14.spkg

It's interesting to compare compile and test times both with and without SAGE_DEBUG

• SAGE_DEBUG=yes, SAGE_CHECK unset = 30.7s
• SAGE_DEBUG unset, SAGE_CHECK unset = 50.7s
• SAGE_DEBUG=yes, SAGE_CHECK=yes 1m:24s
• SAGE_DEBUG unset, SAGE_CHECK=yes 1m:34s

So it takes 20 seconds longer to build when using optimisation. That time penalty drops to 10 seconds when the tests are run, as they obviously run quicker. (All times are measured on a 3.33 GHz Sun Ultra 27)

[nexttime]

comment:28

Oh, I actually took the "val" as an abbreviation, and thought you did mean this one:
Force GSL to be build with no optimisation if SAGE_DEBUG is set to "yes"

Maybe to your surprise, I'm ok with the rest. ;-)

(The only thing that could be clarified is "64-bit builds" meaning 64-bit builds on systems where the compiler default is 32-bit, i.e. when SAGE64 is "yes".)

Leif

[sagetrac-drkirkby]

Attachment: 64-bit-build-clarification.patch.gz

Clarify what are 64-bit builds

[sagetrac-drkirkby]

comment:29

I've updated SPKG.txt to make it clearer what are 64-bit builds.

Package can be found here.

http://boxen.math.washington.edu/home/kirkby/patches/gsl-1.14.spkg

[nexttime]

comment:30

I finally found out what we have to do to make GSL use ATLAS's CBLAS in Sage: Nothing! 8D

(A closer look at $SAGE_ROOT/devel/sage-main/module_list.py helped. I hope other Sage packages that require CBLAS will also link with libcblas.*, not libgslcblas.*.)

The only thing we could (but perhaps shouldn't) do is run GSL's testsuite with ATLAS's CBLAS library instead.
But since GSL's implementation is used as a fall-back in Sage, we should leave it as is, and - if at all - only do an additional run of the testsuite with ATLAS's CBLAS library. (But I've not yet figured out how simple that is; in worst case we'd have to make clean, make again with LDFLAGS="$LDFLAGS -L$SAGE_LOCAL/lib -lcblas -latlas" and then make check.)

[nexttime]

comment:31

Final ptestlong now running - will take nearly 4 hours...

[sagetrac-drkirkby]

comment:32

Replying to @nexttime:

I finally found out what we have to do to make GSL use ATLAS's CBLAS in Sage: Nothing! 8D

(A closer look at $SAGE_ROOT/devel/sage-main/module_list.py helped. I hope other Sage packages that require CBLAS will also link with libcblas.*, not libgslcblas.*.)

The only thing we could (but perhaps shouldn't) do is run GSL's testsuite with ATLAS's CBLAS library instead.
But since GSL's implementation is used as a fall-back in Sage, we should leave it as is, and - if at all - only do an additional run of the testsuite with ATLAS's CBLAS library. (But I've not yet figured out how simple that is; in worst case we'd have to make clean, make again with LDFLAGS="$LDFLAGS -L$SAGE_LOCAL/lib -lcblas -latlas" and then make check.)

I think at this point in time, just merging the updated GSL is sufficient. That was the aim of the ticket.

Other tasks, if we tackle them, should be on another ticket.

There are three related issues I am aware of

• Sage ships with blas-20070724.spkg which is a BLAS library, and according to François Bissey, this is unnecessary, as we have ATLAS. http://groups.google.co.uk/group/sage-devel/msg/a75a7e796a5c3a91
• We should be updating ATLAS. It was agreed to update ATLAS, and I said I would do it, but it far from a trivial affair. I need to discuss with Carl Witty how to build this in parallel. ATLAS takes about 8-10 hours to build in 't2' as there are no tuning parameters for that processor.
• I've got no idea what GSL is used for in Sage. If it's just special functions, which is quite possible, then its pointless worrying about it anyway.

My biggest priority now is to get Sage working properly on OpenSolaris - it builds, but just dumps core as soon as one starts it. Hence I want to be able to verify the different parts of Sage are working - I'm less concerned if they are fully optimised.

If I believe the tools on Solaris, starting Sage, before one gets to the

sage:

prompt, there is already several memory leaks. I see the quality control issues in Sage (doctest, self-tests, memory leaks), more important than addling extra functionality, or speeding Sage up.

Dave

[nexttime]

comment:33

Replying to @sagetrac-drkirkby:

Replying to @nexttime:

I finally found out what we have to do to make GSL use ATLAS's CBLAS in Sage: Nothing! 8D [...]

I think at this point in time, just merging the updated GSL is sufficient. That was the aim of the ticket.

Yes, I agree. I only wanted to say that regarding GSL nothing further has to be done - not even on a follow-up ticket.

[...]
I see the quality control issues in Sage (doctest, self-tests, memory leaks), more important than addling extra functionality, or speeding Sage up.

Agree on that, too. Removing unnecessary parts or extra upstream baggage and improving the documentation are other things I consider important, the former of course less.

[sagetrac-drkirkby]

comment:34

Replying to @nexttime:

Replying to @sagetrac-drkirkby:

Replying to @nexttime:

I finally found out what we have to do to make GSL use ATLAS's CBLAS in Sage: Nothing! 8D [...]

I think at this point in time, just merging the updated GSL is sufficient. That was the aim of the ticket.

Yes, I agree. I only wanted to say that regarding GSL nothing further has to be done - not even on a follow-up ticket.

Given what you have discovered, I suspect SPKG.txt in this package should be changed again. Can you suggest a set of changes. I'll then make sure the information is correct.

Dave

[nexttime]

comment:35

Suggestion:

diff --git a/SPKG.txt b/SPKG.txt
--- a/SPKG.txt
+++ b/SPKG.txt
@@ -38,17 +38,15 @@

 == Dependencies ==

- * None - GSL does currently not depend on any other Sage package to 
-   compile, link and pass all GSL's self-tests. However, as of 
-   20th July 2010, ATLAS is listed as a dependency in spkg/standard/deps. 
+ * None - GSL 1.14 does not depend on any other Sage package to compile,
+   link and pass all of GSL's self-tests. Despite that fact, as of 
+   20th July 2010, ATLAS is listed as a dependency in spkg/standard/deps.
+   (It comes with its own CBLAS implementation that is e.g. used when running
+   the GSL test suite during installation; however, the Sage library only
+   uses it as a fall-back, if e.g. ATLAS's CBLAS library is not present.)

 == Special Update/Build Instructions ==
- * According to the GSL web page: http://www.gnu.org/software/gsl/
-   "GSL requires a BLAS library for vector and matrix operations. The 
-   default CBLAS library supplied with GSL can be replaced by the tuned 
-   ATLAS library for better performance". Exactly how one would use ATLAS is
-   not clear (there are no obvious options for the 'configure' script), 
-   but in principle it could be done. 
+ * Currently nothing special to be done.

[nexttime]

comment:36

Oh, I also changed the item regarding the addition of the "Special Update/Build Instructions" section, which is missing above...

[nexttime]

comment:37

- * Added the "Special Update/Build Instructions" section from SPKG.txt which
-   was previously missing. 
+ * Added the "Special Update/Build Instructions" section to SPKG.txt which
+   was previously missing, though currently no special steps are required.

  * Added notes to SPKG.txt about an unnecessary ATLAS dependency in
-   $SAGE_ROOT/spkg/standard/deps
+   $SAGE_ROOT/spkg/standard/deps, and an explanation why GSL does *not*
+   depend on ATLAS.

- * Added notes to SPKG.txt about how ATLAS could in principle be used to 
-   improve the performance of some of GSL's functionality. 

- * Force GSL to be build with no optimisation if SAGE_DEBUG is set to "yes"
+ * Force GSL to be built with no optimisation if SAGE_DEBUG is set to "yes"

(This snippet looks a bit funny, but one should be able to see what I've changed in addition.)

[nexttime]

comment:38

Just found something else:

There should be quotes around the environment variables tested with -z ($SAGE_LOCAL and $CFLAG64) in both spkg-check and spkg-install, otherwise these tests could give syntax errors.

[sagetrac-drkirkby]

comment:39

Replying to @nexttime:

Just found something else:

There should be quotes around the environment variables tested with -z ($SAGE_LOCAL and $CFLAG64) in both spkg-check and spkg-install, otherwise these tests could give syntax errors.

I disagree. Can you show me an example of how [ -z $foobar ] produces a syntax error, which can be solved by use of [ -z "$foobar" ]? I accept the quotes can do no harm, but I do not believe they are necessary.

I need to go out soon - I look at the other changes later today.

BTW, what was the result of your ptestlong?

Dave

[sagetrac-drkirkby]

Attachment: 9533-resolve-CFLAG64-and-SAGE_ROOT-issues.patch.gz

Put quotes around CFLAG64 and SPKG_ROOT and update SPKG.txt to reflect discoveries about ATLAS

[sagetrac-drkirkby]

comment:40

Replying to @sagetrac-drkirkby:

Replying to @nexttime:

Just found something else:

There should be quotes around the environment variables tested with -z ($SAGE_LOCAL and $CFLAG64) in both spkg-check and spkg-install, otherwise these tests could give syntax errors.

I now accept you were right over this (someone else has confirmed you were right on comp.unix.shell). I've made what I think are all the changes you suggest now.

The package is at

http://boxen.math.washington.edu/home/kirkby/patches/gsl-1.14.spkg

Is that OK now?

Dave

[nexttime]

Reviewer: Mike Hansen, Peter Jeremy, Leif Leonhardy

[nexttime]

comment:41

Well, this one:

 * Added notes to SPKG.txt about how ATLAS could in principle be used to
   improve the performance of some of GSL's functionality.

is still there, but never mind.

Note to the release managers

All patches here are to the spkg's repo, i.e. no patches have to be applied to the Sage library.

[sagetrac-drkirkby]

comment:42

Replying to @nexttime:

Well, this one:

 * Added notes to SPKG.txt about how ATLAS could in principle be used to
   improve the performance of some of GSL's functionality.

is still there, but never mind.

Thank you for the positive review.

I thought I might as well fix it, so just removed that from SPKG.txt.

Dave

[sagetrac-drkirkby]

Attachment: remove-unnecessary-comment.patch.gz

Remove a minor erro in the coments.

[sagetrac-drkirkby]

comment:43

Note to the release managers

'''As Leif says above, all the patches here are in the package repository. Nothing is needed for the Sage library

The package can be found here.'''

http://boxen.math.washington.edu/home/kirkby/patches/gsl-1.14.spkg

[nexttime]

comment:44

Replying to @nexttime:

Final ptestlong now running - will take nearly 4 hours...

In addition to the numerous tests by others above, I've successfully tested the new package with both Sage 4.5 and 4.5.1 on Ubuntu 9.04 x86 and x86_64, gcc 4.3.3 and gcc 4.5.0, by installing the package as well as building Sage from scratch with it. (All ptestlongs passed.)

[sagetrac-drkirkby]

comment:47

Note to the release managers

Perhaps this is stating it too many times, but better too often than too rare.

The positively reviewed package can be found here.

http://boxen.math.washington.edu/home/kirkby/patches/gsl-1.14.spkg

No changes are needed to the Sage library - you do not t need to apply any of the patches on this ticket - they are all in the repository for the .spkg.

[qed777]

Merged: sage-4.5.3.alpha0