Opened 5 years ago

Closed 5 years ago

#20886 closed enhancement (fixed)

Upgrade lrslib to version 6.2; build a shared library; build parallel (multicore/MPI) plrs, mplrs

Reported by: mkoeppe Owned by:
Priority: major Milestone: sage-7.3
Component: packages: optional Keywords: days78
Cc: dimpase, vdelecroix, mkoeppe, chapoton, fbissey, tscrim, vinceknight, campbell, vbraun, jdemeyer Merged in:
Authors: Matthias Koeppe Reviewers: Travis Scrimshaw
Report Upstream: Reported upstream. No feedback yet. Work issues:
Branch: 8b3ad56 (Commits, GitHub, GitLab) Commit: 8b3ad56cb3e5d17c52670740c61a7b9e72bf8a91
Dependencies: Stopgaps:

Status badges

Description (last modified by tscrim)

This ticket upgrades lrslib to version 6.2. This version, according to http://cgm.cs.mcgill.ca/~avis/C/lrs.html, has the following new features:

  • (new in 6.0) mplrs: C wrapper for lrs that allows for parallelization on clusters of machines and uses the MPI library quickstart
  • (major revision in 6.1) lrsnash, 2nash: Computes all Nash equlibria of a two person non-cooperative game. 2nash is a 2-processor parallel version

Since Polymake (#20892) seems to need the shared library of lrslib, but lrslib uses a hand-written makefile that does not work on Mac OS X, I have added an autotools build system. It is available here: https://github.com/mkoeppe/lrslib/tree/autoconfiscation

"Upstream" URL: https://github.com/mkoeppe/lrslib/releases/download/lrslib-062%2Bautotools-2016-07-05/lrslib-062.autotools-2016-07-05.tar.gz Download and put into upstream under the name "lrslib-062+autotools-2016-07-05.tar.gz"

Change History (41)

comment:1 Changed 5 years ago by mkoeppe

  • Cc dimpase added

comment:2 Changed 5 years ago by mkoeppe

  • Cc vdelecroix mkoeppe chapoton added
  • Description modified (diff)
  • Summary changed from Upgrade lrslib to version 6.2 to Upgrade lrslib to version 6.2 and build a shared library

comment:3 Changed 5 years ago by vdelecroix

  • Description modified (diff)

comment:4 Changed 5 years ago by mkoeppe

Thanks for the tarball location. I think I made the patch for last version from here: https://github.com/mkoeppe/lrslib

comment:5 Changed 5 years ago by fbissey

  • Cc fbissey added

comment:6 Changed 5 years ago by mkoeppe

  • Description modified (diff)

comment:7 Changed 5 years ago by mkoeppe

  • Cc tscrim added

comment:8 Changed 5 years ago by kcrisman

  • Cc vinceknight campbell added

Though lrs would still be an optional package (I think it is currently)?

comment:9 Changed 5 years ago by mkoeppe

  • Summary changed from Upgrade lrslib to version 6.2 and build a shared library to Upgrade lrslib to version 6.2; build a shared library; build parallel (multicore/MPI) plrs, mplrs

The 'lrs' package was renamed to 'lrslib' while transitioning to new-style packages (#18127). 'lrslib' installs both the library and the executables.

comment:10 Changed 5 years ago by mkoeppe

  • Description modified (diff)
  • Report Upstream changed from N/A to Not yet reported upstream; Will do shortly.

comment:11 Changed 5 years ago by mkoeppe

  • Branch set to u/mkoeppe/upgrade_lrslib_to_version_6_2__build_a_shared_library__build_parallel__multicore_mpi__plrs__mplrs

comment:12 Changed 5 years ago by mkoeppe

  • Commit set to 02fcdb9e0fab1fcdd3ae5f75c269ba6d9ed15d8b
  • Status changed from new to needs_review

New commits:

02fcdb9Upgrade lrs to version 062 (autotoolized)

comment:13 Changed 5 years ago by kcrisman

When you do this, be sure to test all the game theory stuff with the optional package. Do they need renaming? I don't think I even knew about this renaming to lib.

comment:14 Changed 5 years ago by mkoeppe

The renaming was done in #18127, and at that time some "#optional" tests had to be adjusted.

comment:15 Changed 5 years ago by kcrisman

Awesome.

comment:16 Changed 5 years ago by git

  • Commit changed from 02fcdb9e0fab1fcdd3ae5f75c269ba6d9ed15d8b to bb8b7408bce8e83449f580e8ab1a6a14a09d01f4

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

bb8b740Adjust scripts

comment:17 Changed 5 years ago by mkoeppe

Needs testing (especially on non-Mac OS X).

comment:18 Changed 5 years ago by mkoeppe

  • Authors set to Matthias Koeppe

comment:19 Changed 5 years ago by kcrisman

  • Reviewers set to Karl-Dieter Crisman

comment:20 Changed 5 years ago by kcrisman

  • Reviewers Karl-Dieter Crisman deleted

comment:21 Changed 5 years ago by tscrim

  • Status changed from needs_review to needs_work

I tried to use the link on the ticket for the upstream tarball, and there seems to be a checksum difference (in addition to the file name being wrong).

comment:22 Changed 5 years ago by mkoeppe

  • Description modified (diff)

comment:23 Changed 5 years ago by mkoeppe

Sorry, I had forgotten to update the link.

comment:24 Changed 5 years ago by mkoeppe

  • Status changed from needs_work to needs_review

comment:25 Changed 5 years ago by tscrim

  • Status changed from needs_review to needs_work

Thanks (though the tarball have a . instead of a + in the name). However, this failed to build for me with Ubuntu 14.04 LTS due to it not finding boost:

[lrslib-062+autotools-2016-06-28a] /usr/bin/ld: cannot find -lboost_thread
[lrslib-062+autotools-2016-06-28a] /usr/bin/ld: cannot find -lboost_system

Does this need the full version of boost or is Sage's stripped down version sufficient? FYI - I don't have a system-wide install of boost.

comment:26 Changed 5 years ago by tscrim

Also, to that effect, we should add a dependencies file.

comment:27 follow-up: Changed 5 years ago by mkoeppe

It looks like I need to improve the configure script to detect that boost_thread and boost_system are missing.

Please try with the full boost package, this is what I installed. I'll add it to dependencies.

comment:28 Changed 5 years ago by git

  • Commit changed from bb8b7408bce8e83449f580e8ab1a6a14a09d01f4 to e96fab278cdf889711359115a6112dbaa3cd3f06

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

e96fab2Add dependencies

comment:29 Changed 5 years ago by mkoeppe

  • Status changed from needs_work to needs_review

comment:30 in reply to: ↑ 27 Changed 5 years ago by kcrisman

Replying to mkoeppe:

It looks like I need to improve the configure script to detect that boost_thread and boost_system are missing.

Please try with the full boost package, this is what I installed.

That's annoying, considering we didn't need that before. Is there any possibility of an lrs which doesn't require the full boost?

comment:31 Changed 5 years ago by tscrim

I would somewhat hope there could be a way to link it to the cropped version of boost we ship with Sage. I could not test if this would link to the systemwide boost or would just force an install of the full Sage spkg boost. However, this worked without any trouble for me on my Ubuntu machine (which also installed boost).

comment:32 Changed 5 years ago by tscrim

  • Keywords days78 added

comment:33 Changed 5 years ago by git

  • Commit changed from e96fab278cdf889711359115a6112dbaa3cd3f06 to 35c1b6cac5abbcf1ecb7fdbc41990b0dd9cd105f

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

1a24aaclrslib: Remove dependency on full boost package; conditionalize plrs build on boost
35c1b6cMerge tag '7.3.beta6' into t/20886/upgrade_lrslib_to_version_6_2__build_a_shared_library__build_parallel__multicore_mpi__plrs__mplrs

comment:34 Changed 5 years ago by mkoeppe

  • Description modified (diff)
  • Report Upstream changed from Not yet reported upstream; Will do shortly. to Reported upstream. No feedback yet.

I've updated the package's configure script so it detects whether the full boost is available. If so, it builds one additional binary, plrs, which is not used by current Sage.

comment:35 Changed 5 years ago by tscrim

  • Cc vbraun jdemeyer added
  • Reviewers set to Travis Scrimshaw

Thank you. I think, which is why I've cc-ed Jeroen and Volker if one of them could confirm, that the correct way to do the dependencies file is to use $(MP_LIBRARY). Once that is done, I think we can set this to a positive review.

comment:36 Changed 5 years ago by vbraun

Make it so

comment:37 Changed 5 years ago by git

  • Commit changed from 35c1b6cac5abbcf1ecb7fdbc41990b0dd9cd105f to 8b3ad56cb3e5d17c52670740c61a7b9e72bf8a91

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

8b3ad56lrslib: Fix up dependencies

comment:38 Changed 5 years ago by tscrim

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

This looks good to me now; thanks.

Note to Volker, you will have to rename the tarball when you put it on the server.

comment:39 Changed 5 years ago by mkoeppe

Thanks for reviewing, Travis!

comment:40 Changed 5 years ago by mkoeppe

Follow-up:

  • #20969: Expose new lrslib features: lrsnash, 2nash
  • #18199: sage.geometry.polyhedron should have an lrs (lrslib) backend

comment:41 Changed 5 years ago by vbraun

  • Branch changed from u/mkoeppe/upgrade_lrslib_to_version_6_2__build_a_shared_library__build_parallel__multicore_mpi__plrs__mplrs to 8b3ad56cb3e5d17c52670740c61a7b9e72bf8a91
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.