Opened 6 years ago

Closed 6 years ago

#20886 closed enhancement (fixed)

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

Reported by: Matthias Köppe Owned by:
Priority: major Milestone: sage-7.3
Component: packages: optional Keywords: days78
Cc: Dima Pasechnik, Vincent Delecroix, Matthias Köppe, Frédéric Chapoton, François Bissey, Travis Scrimshaw, Vincent Knight, campbell, Volker Braun, Jeroen Demeyer 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 Travis Scrimshaw)

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

Cc: Dima Pasechnik added

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

Cc: Vincent Delecroix Matthias Köppe Frédéric Chapoton added
Description: modified (diff)
Summary: Upgrade lrslib to version 6.2Upgrade lrslib to version 6.2 and build a shared library

comment:3 Changed 6 years ago by Vincent Delecroix

Description: modified (diff)

comment:4 Changed 6 years ago by Matthias Köppe

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

comment:5 Changed 6 years ago by François Bissey

Cc: François Bissey added

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

Description: modified (diff)

comment:7 Changed 6 years ago by Matthias Köppe

Cc: Travis Scrimshaw added

comment:8 Changed 6 years ago by Karl-Dieter Crisman

Cc: Vincent Knight campbell added

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

comment:9 Changed 6 years ago by Matthias Köppe

Summary: Upgrade lrslib to version 6.2 and build a shared libraryUpgrade 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 6 years ago by Matthias Köppe

Description: modified (diff)
Report Upstream: N/ANot yet reported upstream; Will do shortly.

comment:11 Changed 6 years ago by Matthias Köppe

Branch: u/mkoeppe/upgrade_lrslib_to_version_6_2__build_a_shared_library__build_parallel__multicore_mpi__plrs__mplrs

comment:12 Changed 6 years ago by Matthias Köppe

Commit: 02fcdb9e0fab1fcdd3ae5f75c269ba6d9ed15d8b
Status: newneeds_review

New commits:

02fcdb9Upgrade lrs to version 062 (autotoolized)

comment:13 Changed 6 years ago by Karl-Dieter Crisman

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

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

comment:15 Changed 6 years ago by Karl-Dieter Crisman

Awesome.

comment:16 Changed 6 years ago by git

Commit: 02fcdb9e0fab1fcdd3ae5f75c269ba6d9ed15d8bbb8b7408bce8e83449f580e8ab1a6a14a09d01f4

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

bb8b740Adjust scripts

comment:17 Changed 6 years ago by Matthias Köppe

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

comment:18 Changed 6 years ago by Matthias Köppe

Authors: Matthias Koeppe

comment:19 Changed 6 years ago by Karl-Dieter Crisman

Reviewers: Karl-Dieter Crisman

comment:20 Changed 6 years ago by Karl-Dieter Crisman

Reviewers: Karl-Dieter Crisman

comment:21 Changed 6 years ago by Travis Scrimshaw

Status: needs_reviewneeds_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 6 years ago by Matthias Köppe

Description: modified (diff)

comment:23 Changed 6 years ago by Matthias Köppe

Sorry, I had forgotten to update the link.

comment:24 Changed 6 years ago by Matthias Köppe

Status: needs_workneeds_review

comment:25 Changed 6 years ago by Travis Scrimshaw

Status: needs_reviewneeds_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 6 years ago by Travis Scrimshaw

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

comment:27 Changed 6 years ago by Matthias Köppe

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 6 years ago by git

Commit: bb8b7408bce8e83449f580e8ab1a6a14a09d01f4e96fab278cdf889711359115a6112dbaa3cd3f06

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

e96fab2Add dependencies

comment:29 Changed 6 years ago by Matthias Köppe

Status: needs_workneeds_review

comment:30 in reply to:  27 Changed 6 years ago by Karl-Dieter Crisman

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 6 years ago by Travis Scrimshaw

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 6 years ago by Travis Scrimshaw

Keywords: days78 added

comment:33 Changed 6 years ago by git

Commit: e96fab278cdf889711359115a6112dbaa3cd3f0635c1b6cac5abbcf1ecb7fdbc41990b0dd9cd105f

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

Description: modified (diff)
Report Upstream: Not yet reported upstream; Will do shortly.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 6 years ago by Travis Scrimshaw

Cc: Volker Braun Jeroen Demeyer added
Reviewers: 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 6 years ago by Volker Braun

Make it so

comment:37 Changed 6 years ago by git

Commit: 35c1b6cac5abbcf1ecb7fdbc41990b0dd9cd105f8b3ad56cb3e5d17c52670740c61a7b9e72bf8a91

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

8b3ad56lrslib: Fix up dependencies

comment:38 Changed 6 years ago by Travis Scrimshaw

Description: modified (diff)
Status: needs_reviewpositive_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 6 years ago by Matthias Köppe

Thanks for reviewing, Travis!

comment:40 Changed 6 years ago by Matthias Köppe

Follow-up:

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

comment:41 Changed 6 years ago by Volker Braun

Branch: u/mkoeppe/upgrade_lrslib_to_version_6_2__build_a_shared_library__build_parallel__multicore_mpi__plrs__mplrs8b3ad56cb3e5d17c52670740c61a7b9e72bf8a91
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.