Opened 7 years ago

Closed 6 years ago

#12672 closed enhancement (duplicate)

Build PPL with its C interface

Reported by: SimonKing Owned by: tbd
Priority: major Milestone: sage-duplicate/invalid/wontfix
Component: packages: standard Keywords: PPL C interface
Cc: aginiewicz Merged in:
Authors: Reviewers: Jeroen Demeyer
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description

Currently, the standard PPL spkg only builds the C++ interface. However, CLooG-PPL (see #12666) needs the C-header ppl_c.h as well.

Therefore I suggest to slightly modify the install script of our PPL spkg:

  • Do make install instead of make install-strip -- I admit that I am not sure whether this is needed, and perhaps make install-strip yields faster code. A reviewer should comment on that.
  • Modify the configuration options, such that the C interface is built.

spkg: http://boxen.math.washington.edu/home/SimonKing/SAGE/spkgs/ppl-0.11.2.p2.spkg

Change History (17)

comment:1 Changed 7 years ago by SimonKing

  • Status changed from new to needs_review

comment:2 Changed 7 years ago by SimonKing

Interestingly, when I tried doc tests with a Sage version that used massive optimization (via graphite), a lot of tests went a lot slower (by a factor of 2 in some cases) than without the optimization.

I had the impression that the particularly bad cases were related to polyhedra. Therefore, I try to build ppl with make install-strip again, and keep my fingers crossed that it will improve the situation.

comment:3 Changed 7 years ago by SimonKing

It could be that the bad performance came from using GMP instead of MPIR. It turns out that cloog-ppl can be built with MPIR (originally, I thought GMP is required). Anyway, I am now building Sage without GMP but with high optimization, and hope that it'll work better.

comment:4 Changed 7 years ago by aginiewicz

  • Cc aginiewicz added

This also fixes #11391, where on Arch Linux and some other new 32-bit distros there are missing symbols in ppl during building of pycrypto - enabling C interface makes it work again. I haven't noticed this ticket, as I started testing fix for 11391 before this one was created, so I posted nearly same spkg in #11391 (with C interface, but without install vs install-strip change).

As I worked on the same thing just few hours ago, after quick check on sage 4.8 I just made in VM I can say that it works, but I don't feel experienced enough to comment on install vs install-strip, so I'm leaving it as needing review.

comment:5 Changed 7 years ago by jdemeyer

make install-strip removes debugging information from the PPL library. I don't see the need for that, so I actually prefer install over install-strip.

comment:6 Changed 7 years ago by jdemeyer

  • Status changed from needs_review to needs_work

Unfortunately, this seems to require a recent vesion of M4 (or at least, it fails when M4 isn't present):

checking whether to build the Parma Watchdog Library... yes
checking whether to build the ppl_lcdd program... yes
checking whether to build the ppl_lpsol program... yes
checking whether to build the ppl_pips program... yes
checking which interfaces are enabled... cxx c
checking for GNU M4 that supports accurate traces... configure: error: no acceptable m4 could be found in $PATH.
GNU M4 1.4.5 or later is required; 1.4.11 is recommended
Error configuring the Parma Polyhedra Library.

(I don't have much time now to think about this or investigate this further)

Version 0, edited 7 years ago by jdemeyer (next)

comment:7 follow-up: Changed 7 years ago by jdemeyer

This is a duplicate of #11391 (which has the same solution for a different problem).

comment:8 in reply to: ↑ 7 Changed 7 years ago by SimonKing

Replying to jdemeyer:

This is a duplicate of #11391 (which has the same solution for a different problem).

I think it is not a duplicate. #11391 is about building ppl on ArchLinux and OpenSuse and so on. The ticket here is about building ppl with its c interface. So, these are orthogonal problems.

comment:9 Changed 7 years ago by SimonKing

  • Status changed from needs_work to needs_review

comment:10 Changed 7 years ago by SimonKing

  • Status changed from needs_review to needs_work

comment:11 Changed 7 years ago by SimonKing

It remains "needs work", because of the problem on OS X you mentioned. So, do we need an M4 package??

comment:12 follow-up: Changed 6 years ago by kcrisman

I don't see why one couldn't just do the usual thing we do in spkg-install for unusual systems - just a check for Darwin with version 8 or 9 or something. Or even check for m4's existence and version (can this be done easily?). In either case, just don't do

-    --enable-interfaces=c++ --disable-fpmath
+    --enable-interfaces="c++ c" --disable-fpmath

for that specific situation, maybe echoing a message. Then #12666 can raise an error message if one doesn't have the correct header, saying "please get a system with a recent enough m4, do sage -i ppl, and then reinstall this optional spkg".

comment:13 in reply to: ↑ 12 ; follow-up: Changed 6 years ago by jdemeyer

Replying to kcrisman:

just a check for Darwin with version 8 or 9 or something.

That's too fragile, nobody guarantees that this is the only system with a bad m4.

Or even check for m4's existence and version

If you create a patch, I'll be happy to review it. Note that we already require the existence of m4 in prereq (MPIR needs it).

comment:14 in reply to: ↑ 13 Changed 6 years ago by kcrisman

Replying to jdemeyer:

Replying to kcrisman:

just a check for Darwin with version 8 or 9 or something.

That's too fragile, nobody guarantees that this is the only system with a bad m4.

True, in fact it's quite likely others will be thus.

Or even check for m4's existence and version

If you create a patch, I'll be happy to review it. Note that we already require the existence of m4 in prereq (MPIR needs it).

Last edited 6 years ago by kcrisman (previous) (diff)

comment:15 Changed 6 years ago by kcrisman

Here is a first try.

  • spkg-install

    diff --git a/spkg-install b/spkg-install
    a b  
    3939    fi
    4040done
    4141
    42 ./configure --prefix="$SAGE_LOCAL" --libdir="$SAGE_LOCAL/lib" \
    43     --with-gmp-prefix="$SAGE_LOCAL" --enable-coefficients=mpz \
    44     --enable-interfaces="c++ c" --disable-fpmath
     42# If m4 is not recent enough, we fall back to only enabling the C++ interface
     43M4_VERSION=`m4 --version | grep GNU | cut -d " " -f 3`
     44if [[ $M4_VERSION < 1.4.5 ]]; then
     45    echo "Configuring only for C++ interface"
     46    echo "Install GNU M4 1.4.5 or later for C interface"
     47    ./configure --prefix="$SAGE_LOCAL" --libdir="$SAGE_LOCAL/lib" \
     48        --with-gmp-prefix="$SAGE_LOCAL" --enable-coefficients=mpz \
     49        --enable-interfaces=c++ --disable-fpmath
     50else
     51    echo "Great, you have GNU M4 1.4.5 or later"
     52    echo "Configuring for C++ and C interfaces"
     53    ./configure --prefix="$SAGE_LOCAL" --libdir="$SAGE_LOCAL/lib" \
     54        --with-gmp-prefix="$SAGE_LOCAL" --enable-coefficients=mpz \
     55        --enable-interfaces="c++ c" --disable-fpmath
     56fi
    4557if [ $? -ne 0 ]; then
    4658   echo >&2 "Error configuring the Parma Polyhedra Library."
    4759   exit 1

But it doesn't work, because the extended bash test doesn't quite do what we want.

$ if [[ 1.4.11 > 1.4.5 ]]; then echo "bigger"; fi
$

I cringe at the more comprehensive solutions to this problem I found on the internet, though. The sort -V command might have been helpful, but that flag isn't universally available.

comment:16 Changed 6 years ago by jdemeyer

  • Authors Simon King deleted
  • Milestone changed from sage-5.10 to sage-duplicate/invalid/wontfix
  • Reviewers set to Jeroen Demeyer
  • Status changed from needs_work to positive_review

Fixed by #14232.

comment:17 Changed 6 years ago by jdemeyer

  • Resolution set to duplicate
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.