Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
mkoeppe opened this issue Jun 27, 2016 · 46 comments

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Jun 27, 2016

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"

Upstream: Reported upstream. No feedback yet.

CC: @dimpase @videlec @mkoeppe @fchapoton @kiwifb @tscrim @drvinceknight @theref @vbraun @jdemeyer

Component: packages: optional

Keywords: days78

Author: Matthias Koeppe

Branch/Commit: 8b3ad56

Reviewer: Travis Scrimshaw

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

@mkoeppe mkoeppe added this to the sage-7.3 milestone Jun 27, 2016
@mkoeppe

This comment has been minimized.

@mkoeppe mkoeppe changed the title Upgrade lrslib to version 6.2 Upgrade lrslib to version 6.2 and build a shared library Jun 28, 2016
@videlec

This comment has been minimized.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 28, 2016

comment:4

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

@mkoeppe

This comment has been minimized.

@kcrisman
Copy link
Member

comment:8

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

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 28, 2016

comment:9

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

@mkoeppe mkoeppe changed the title Upgrade lrslib to version 6.2 and build a shared library Upgrade lrslib to version 6.2; build a shared library; build parallel (multicore/MPI) plrs, mplrs Jun 28, 2016
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 28, 2016

Upstream: Not yet reported upstream; Will do shortly.

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 28, 2016

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 28, 2016

Commit: 02fcdb9

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 28, 2016

New commits:

02fcdb9Upgrade lrs to version 062 (autotoolized)

@kcrisman
Copy link
Member

comment:13

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.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 28, 2016

comment:14

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

@kcrisman
Copy link
Member

comment:15

Awesome.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 28, 2016

Changed commit from 02fcdb9 to bb8b740

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 28, 2016

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

bb8b740Adjust scripts

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 28, 2016

comment:17

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

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 29, 2016

Author: Matthias Koeppe

@kcrisman
Copy link
Member

Reviewer: Karl-Dieter Crisman

@kcrisman
Copy link
Member

Changed reviewer from Karl-Dieter Crisman to none

@tscrim
Copy link
Collaborator

tscrim commented Jun 29, 2016

comment:21

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).

@tscrim
Copy link
Collaborator

tscrim commented Jun 29, 2016

comment:26

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

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 29, 2016

comment:27

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.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 29, 2016

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

e96fab2Add dependencies

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 29, 2016

Changed commit from bb8b740 to e96fab2

@kcrisman
Copy link
Member

comment:30

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?

@tscrim
Copy link
Collaborator

tscrim commented Jun 30, 2016

comment:31

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).

@tscrim
Copy link
Collaborator

tscrim commented Jul 1, 2016

Changed keywords from none to days78

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 6, 2016

Changed commit from e96fab2 to 35c1b6c

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 6, 2016

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

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 6, 2016

Changed upstream from Not yet reported upstream; Will do shortly. to Reported upstream. No feedback yet.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 6, 2016

comment:34

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.

@tscrim
Copy link
Collaborator

tscrim commented Jul 6, 2016

comment:35

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.

@tscrim
Copy link
Collaborator

tscrim commented Jul 6, 2016

Reviewer: Travis Scrimshaw

@vbraun
Copy link
Member

vbraun commented Jul 6, 2016

comment:36

Make it so

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 6, 2016

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

8b3ad56lrslib: Fix up dependencies

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 6, 2016

Changed commit from 35c1b6c to 8b3ad56

@tscrim

This comment has been minimized.

@tscrim
Copy link
Collaborator

tscrim commented Jul 7, 2016

comment:38

This looks good to me now; thanks.

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

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 7, 2016

comment:39

Thanks for reviewing, Travis!

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 7, 2016

comment:40

Follow-up:

@vbraun
Copy link
Member

vbraun commented Jul 8, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants