Opened 7 years ago

Closed 6 years ago

Last modified 4 years ago

#14232 closed enhancement (fixed)

Update PPL to v1.0

Reported by: vbraun Owned by: tbd
Priority: major Milestone: sage-5.11
Component: packages: standard Keywords:
Cc: novoselt, dimpase Merged in: sage-5.11.beta1
Authors: Volker Braun, Jeroen Demeyer Reviewers: Dmitrii Pasechnik, Volker Braun
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Attachments (4)

trac_14232_ppl_doctest_fixes.patch (4.8 KB) - added by vbraun 6 years ago.
Updated patch
14232_bin_hgignore.patch (381 bytes) - added by jdemeyer 6 years ago.
ppl-1.0.p0.diff (3.3 KB) - added by jdemeyer 6 years ago.
14232_ppl_deps.patch (1.4 KB) - added by jdemeyer 6 years ago.

Download all attachments as: .zip

Change History (34)

comment:1 Changed 7 years ago by jdemeyer

How about

sage: C_Polyhedron(1, 'empty').max_space_dimension()
357913940            # 32-bit
1152921504606846974  # 64-bit

I believe that's the standard way.

comment:2 Changed 6 years ago by vbraun

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

comment:3 Changed 6 years ago by vbraun

Done!

Changed 6 years ago by vbraun

Updated patch

comment:4 Changed 6 years ago by vbraun

Fixed the # long doctests, too

comment:5 Changed 6 years ago by vbraun

  • Cc novoselt dimpase added

Straightforward version bump, still needs review

comment:6 Changed 6 years ago by dimpase

  • Status changed from needs_review to positive_review

looks good.

comment:7 Changed 6 years ago by jdemeyer

  • Milestone changed from sage-5.10 to sage-5.11
  • Reviewers set to Dmitrii Pasechnik

comment:8 Changed 6 years ago by jdemeyer

  • Dependencies set to #11391
  • Milestone changed from sage-5.11 to sage-pending

This exposes #11391 on Ubuntu 13.04.

comment:9 Changed 6 years ago by jdemeyer

Working on a solution, hang on...

comment:10 Changed 6 years ago by jdemeyer

  • Authors changed from Volker Braun to Volker Braun, Jeroen Demeyer
  • Description modified (diff)
  • Status changed from positive_review to needs_work

comment:11 Changed 6 years ago by jdemeyer

  • Dependencies #11391 deleted
  • Milestone changed from sage-pending to sage-5.11
  • Status changed from needs_work to needs_review

comment:12 Changed 6 years ago by jdemeyer

Added fix for #11391 and #12672, needs review.

spkg diff: ppl-1.0.p0.diff

comment:13 Changed 6 years ago by vbraun

  • Status changed from needs_review to positive_review

Looks good to me.

I've updated the compilerwrapper to revert LD_LIBRARY_PATH to SAGE_ORIG_LD_LIBRARY_PATH, see #10572. This would be a generic way to avoid similar headache in the future, and slowly work towards getting rid of the whole LD_LIBRARY_PATH hack.

Changed 6 years ago by jdemeyer

comment:14 Changed 6 years ago by jdemeyer

  • Description modified (diff)
  • Status changed from positive_review to needs_work

comment:15 Changed 6 years ago by jdemeyer

  • Status changed from needs_work to needs_review

comment:16 Changed 6 years ago by vbraun

  • Status changed from needs_review to positive_review

looks good to me

comment:17 Changed 6 years ago by jdemeyer

Why do we configure PPL with --disable-fpmath? This was already present in the initial PPL spkg:

 * configure --disable-fpmath works now (since we don't use the
  floating point math stuff).
Last edited 6 years ago by jdemeyer (previous) (diff)

Changed 6 years ago by jdemeyer

comment:18 Changed 6 years ago by jdemeyer

  • Description modified (diff)
  • Status changed from positive_review to needs_work

comment:19 Changed 6 years ago by jdemeyer

  • Status changed from needs_work to needs_review

In order to avoid race conditions which happen when GCC is ran while installing PPL, we need 14232_ppl_deps.patch. This needs review.

I also made a further small change to the spkg to support the PPL_CONFIGURE environment variable, this also needs review.

comment:20 Changed 6 years ago by vbraun

  • Reviewers changed from Dmitrii Pasechnik to Dmitrii Pasechnik, Volker Braun
  • Status changed from needs_review to positive_review

The floating-point stuff in PPL needs a funky rounding mode set, so its not going to work as expected in Sage. Which is why I disabled it in the configure.

This ticket is just a textbook case of the perils of abusing LD_LIBRARY_PATH.

comment:21 Changed 6 years ago by jdemeyer

  • Merged in set to sage-5.11.beta1
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:22 Changed 6 years ago by jdemeyer

  • Merged in sage-5.11.beta1 deleted
  • Resolution fixed deleted
  • Status changed from closed to new

Changed 6 years ago by jdemeyer

comment:23 Changed 6 years ago by jdemeyer

  • Status changed from new to needs_review

Added GLPK to PPL dependencies since PPL seems to use GLPK.

14232_ppl_deps.patch needs review.

comment:24 follow-up: Changed 6 years ago by jdemeyer

It seems that there was a patch on #10039 to add the GLPK dependency, but by mistake, this patch wasn't merged.

comment:25 in reply to: ↑ 24 Changed 6 years ago by dimpase

  • Status changed from needs_review to positive_review

Replying to jdemeyer:

It seems that there was a patch on #10039 to add the GLPK dependency, but by mistake, this patch wasn't merged.

indeed. See https://www.cs.unipr.it/mantis/view.php?id=500 which seems to say that the dependency on GLPK will eventually be gone, but not yet...

comment:26 Changed 6 years ago by jdemeyer

  • Merged in set to sage-5.11.beta1
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:27 Changed 4 years ago by leif

Meanwhile at PPL 1.1, we have:

-rw-r--r-- 1 leif leif 57351174 2015-04-29 16:21 local/lib/libppl.a
-rw-r--r-- 1 leif leif 83958276 2015-04-29 16:21 local/lib/libppl_c.a
-rwxr-xr-x 1 leif leif     1236 2015-05-02 18:29 local/lib/libppl_c.la
lrwxrwxrwx 1 leif leif       17 2015-04-29 16:21 local/lib/libppl_c.so -> libppl_c.so.4.0.0
lrwxrwxrwx 1 leif leif       17 2015-04-29 16:21 local/lib/libppl_c.so.4 -> libppl_c.so.4.0.0
-rwxr-xr-x 1 leif leif 29967312 2015-04-29 16:21 local/lib/libppl_c.so.4.0.0
-rwxr-xr-x 1 leif leif     1156 2015-05-02 18:29 local/lib/libppl.la
lrwxrwxrwx 1 leif leif       16 2015-04-29 16:21 local/lib/libppl.so -> libppl.so.13.0.0
lrwxrwxrwx 1 leif leif       16 2015-04-29 16:21 local/lib/libppl.so.13 -> libppl.so.13.0.0
-rwxr-xr-x 1 leif leif 19604376 2015-04-29 16:21 local/lib/libppl.so.13.0.0

I.e., while the library version of libppl got bumped, libppl_c is still at 4 (the same version as in PPL 0.x!).

So we still have to build PPL "serially" (not in parallel with other packages) to avoid potential race conditions with old system GCCs linked against that version; GCC 4.8 and any later version use ISL instead of PPL. (And PPL was optionally used since GCC 4.4 IIRC, so the "critical" versions are 4.4.x to 4.7.x.)

comment:28 Changed 4 years ago by leif

(Once we require full C++11 support / GCC >= 4.8.1, we can change that.)

comment:29 follow-up: Changed 4 years ago by vbraun

The real issue is a) abuse of LD_LIBRARY_PATH and b) necroposting on long-closed tickets ;-)

comment:30 in reply to: ↑ 29 Changed 4 years ago by leif

Replying to vbraun:

necroposting on long-closed tickets ;-)

ROFL. I was just wondering whether it's still an issue (i.e., whether we could remove PPL from the sequential toolchain build) -- it is, so I didn't open a ticket for changing that; and as of Sage 6.7.beta3, build/Makefile still refers to this ticket:

# TOOLCHAIN consists of dependencies determined by build/install,
# including for example the GCC package.
toolchain: $(TOOLCHAIN)

# Build all packages that GCC links against serially, otherwise this
# leads to race conditions where some library which is used by GCC gets
# reinstalled. Since system GCCs might use Sage's libraries, we do this
# unconditionally. We still use the dependency checking from $(MAKE),
# so this will not trigger useless rebuilds.
# See #14168 and #14232.
toolchain-deps:
        $(MAKE) $(INST)/$(ZLIB)
        $(MAKE) $(INST)/$(SAGE_MP_LIBRARY)
        $(MAKE) $(INST)/$(MPFR)
        $(MAKE) $(INST)/$(MPC)
        $(MAKE) $(INST)/$(PPL)

You can open a new ticket for letting build/Makefile refer to some further new ticket of course.

Note: See TracTickets for help on using tickets.