Opened 8 years ago

Closed 4 years ago

#14116 closed defect (wontfix)

update polymake interface to 2.14-rc1

Reported by: tkluck Owned by: mhampton
Priority: major Milestone: sage-duplicate/invalid/wontfix
Component: geometry Keywords:
Cc: was, mkoeppe Merged in:
Authors: Timo Kluck Reviewers: Karl-Dieter Crisman, Frédéric Chapoton, Matthias Köppe
Report Upstream: N/A Work issues: source tar link, fix doctests, info for how to install, add doctests
Branch: public/ticket/14416 (Commits, GitHub, GitLab) Commit: 50f6da95b155a9739c546b2f0fcd5f6edd09546f
Dependencies: #13768, #13767 Stopgaps:

Status badges

Description (last modified by chapoton)

I've written a new interface for polymake using its shared library interface, while trying to stay backward compatible with what there was before.

I've classified this as a defect, since the current interface doesn't work anymore.

http://www.polymake.org/lib/exe/fetch.php/download/polymake-2.14r1-minimal.tar.bz2

Attachments (2)

trac_14116_polymake_upgrade.patch (41.0 KB) - added by tkluck 8 years ago.
trac_14116-polymake-upgrade-rebased.patch (41.0 KB) - added by kcrisman 8 years ago.

Download all attachments as: .zip

Change History (60)

Changed 8 years ago by tkluck

comment:1 Changed 8 years ago by tkluck

comment:2 Changed 8 years ago by kcrisman

Needs to be rebased to #13432.

comment:3 Changed 8 years ago by kcrisman

Uploading rebased patch next. But:

Compiling sage/geometry/polymake/polymake.pyx because it changed.
Cythonizing sage/geometry/polymake/polymake.pyx

Error compiling Cython file:
------------------------------------------------------------
...
    def save(self, filename):
        self.thisptr.save(<string><char*>filename)

    def __getattr__(self, propertyname):
        propertyname = propertyname.upper()
        return <str>get_property_wrapper(self.thisptr[0], <string><char*>propertyname).c_str()
                                                                                           ^
------------------------------------------------------------

sage/geometry/polymake/polymake.pyx:247:92: default encoding required for conversion from 'char const *' to 'str object'

This is presumably related to your comment about getting rid of std::string cpp_string = <string><char*>python_string stuff; the Cython in Sage is now 5.19.

Error compiling Cython file:
------------------------------------------------------------
...
                    p = i.numerator() if callable(i.numerator) else i.numerator
                    q = i.denominator() if callable(i.denominator) else i.denominator
                    entries.push_back(Rational(p,q))
                else:
                    entries.push_back(<Rational><int>i)
            matrix = new Matrix[Rational](rows, cols, entries.begin())
                                                                  ^
------------------------------------------------------------

sage/geometry/polymake/polymake.pyx:268:67: Cannot assign type 'iterator' to 'vector[Rational]'

No idea, though the message seems like it would be clear to someone who knew Cython.

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

Changed 8 years ago by kcrisman

comment:4 Changed 8 years ago by tkluck

kcrisman: I'm considering replacing my own work by what Burcin has done with pypolymake. He works around some cython issues in a much nicer way. Have you looked at that, too?

comment:5 Changed 8 years ago by kcrisman

  • Milestone changed from sage-5.11 to sage-5.10

Have you looked at that, too?

No. It's not exactly clear to me what to do (though this seems likely), and the forums suggest that it doesn't have as much functionality as the interface here, though I may be misinterpreting that. It would be nice to have it in an spkg form, though I understand that is different from the lmonade philosophy.

Also note this polymake user's attempt, though apparently they later just used pypolymake. I assume that user is not yourself?

comment:6 Changed 8 years ago by kcrisman

In particular, the following things seems like hacks:

ln -s /usr/local/sage-4.7.1/devel/sage/c_lib/include /usr/local/sage-4.7.1/local/include/sage

and this apparently required patch

  • sage-env

    diff --git a/sage-env b/sage-env
    a b  
    219219    mkdir -p "$MPLCONFIGDIR"
    220220fi
    221221
     222LD_PRELOAD="$SAGE_LOCAL/lib/libpolymake.so"
     223export LD_PRELOAD
     224
    222225LD_LIBRARY_PATH="$SAGE_ROOT/local/lib/:$LD_LIBRARY_PATH" && export LD_LIBRARY_PATH
    223226# The following is needed for openmpi:
    224227LD_LIBRARY_PATH="$SAGE_ROOT/local/lib/openmpi:$LD_LIBRARY_PATH" && export LD_LIBRARY_PATH

which I'll point out would no longer apply directly, as the LD_LIBRARY_PATH code has changed, though one could just add the lines, of course. And of course sage-env doesn't live in local/bin any more, but in spkg/bin.

comment:7 Changed 8 years ago by tkluck

I just pushed some initial work to

https://github.com/tkluck/sage/tree/polymake-upgrade

It integrates pypolymake with Sage, while keeping backwards compatibility with the old interface. Additionally, it adds an interface allowing you to specify polytopes by Sage expressions.

I'm having some trouble getting the doctests to work, since polymake has a habit of "spamming" stderr. (Actually, it outputs citation information, which is worthwhile. We do need to deal with it, though.) Also, trying to be backwards compatible both with Sage and with pypolymake leads to ugliness such as having both n_points and num_points methods.

I'm doing development in the new integrated git repository, which is supposed to replace the mercurial repositories sometime this Summer. Here are instructions for reviewing my branch. They include cloning the repository and building Sage from source, so this takes a lot of time. I will also make mercurial patches available for review at some point.

$ # clone the repository
$ git clone git://github.com/tkluck/sage.git
$ cd sage
$ git checkout -t polymake-upgrade
$ # get the upstream package for polymake
$ cd upstream
$ wget http://polymake.org/lib/exe/fetch.php/download/polymake-2.12-rc3.tar.bz2
$ mv polymake-2.12{-rc3,}.tar.bz2
$ cd ..
$ # build sage
$ MAKE="make -j4" make
$ # install the polymake package
$ ./sage -i polymake
$ # build the polymake interface, now that the package is available
$ ./sage -b

comment:8 Changed 8 years ago by vbraun

How about

  • Move the whole stuff to sage/libs/polymake
  • Don't care about backward compatibility, the polymake library interface (like the PPL library interface) is not supposed to be the user-facing part
  • Integrate polymake as one of the backends for polyhedra in Sage (and only this will be in sage/geometry)
  • the PolymakeProxy can go away, the polyhedron backend will raise an error if polymake is not installed.

comment:9 Changed 8 years ago by vbraun

Also, why do we need to hardcode the RPATH? This will break during relocation, fyi.

comment:10 Changed 8 years ago by tkluck

Thanks for your comments, Volker. I removed hardcoding the RPATH, which seems to have been a leftover from early debugging. I also agree with your idea about eventually making polymake just a backend for the polyhedron class.

In anticipation, I did as you suggested and moved the library interface code to sage/libs/polymake. I have not made steps to integrate it with the existing polyhedron class and I have also not removed the backward compatibility, including the PolymakeProxy thing.

I'm not sure if we need to do all this integration work before upgrading polymake and merging this into Sage. Currently, the object polymake is part of the global namespace on startup, and it is broken. This fixes it.

comment:11 Changed 8 years ago by vbraun

Can you actually attach your patch? ;-)

There is also a note about Cython 0.17pre that is useless now. Can you remove it while you are at it?

I agree that we don't have to do everything on this ticket. But which base rings does polymake support, and which ones are wrapped here? The polymake web page says something about quadratic field extensions which might be interesting...

comment:12 Changed 8 years ago by tkluck

The code is up at github: https://github.com/tkluck/sage/tree/polymake-upgrade

I'll make this into a mercurial patch at some point, but I haven't yet. These kind of updates (simultaneously updating a package and its library interface) are exactly the kind of thing the integrated repository was setup to do.

comment:13 Changed 8 years ago by tkluck

  • Branch set to u/tkluck/ticket/14116
  • Dependencies #13768, #13767 deleted

comment:14 Changed 8 years ago by tkluck

  • Dependencies set to #13768, #13767

reinstate dependencies after they got lost by using the new dev-scripts.

comment:15 Changed 7 years ago by chapoton

  • Commit set to eebd8ec51e4082905710325aeb3c09d019838bb9
  • Milestone changed from sage-5.10 to sage-6.4

Last 10 new commits:

6cbe8aeFix wrong doctests
e511e06Fix failing of doctests due to credits
b3fda3bsome initial work on the GRAPH property
315bd12Some initial work to make pickling possible
a30b030move polymake interface to sage.libs.polymake
f049b0bname of module is sage.libs.polymake.polymake
7ab0dddcatch some possible c++ exceptions
7c20845make load work, and thereby pickling
e2ebcf4coordinates should be a tuple, not a generator
eebd8ecshould have tracked __init__.py file for polymake

comment:16 Changed 7 years ago by chapoton

  • Branch changed from u/tkluck/ticket/14116 to public/ticket/14416
  • Commit changed from eebd8ec51e4082905710325aeb3c09d019838bb9 to e14a9f2e18d3e091ceaf7a922ca5e2ff91effb7a

merge with 6.4.beta2


New commits:

e14a9f2Merging with 6.4.beta2

comment:17 Changed 6 years ago by kcrisman

Frédéric, have you actually successfully installed polymake and used this, or did you just do community service in updating this to a git branch? Just curious.

comment:18 Changed 6 years ago by kcrisman

Timo, did you want to update to a new-style spkg, or was that Frédéric? In which case someone would have to provide a link here to what to put in the upstream/ folder.

comment:19 Changed 6 years ago by kcrisman

I am trying the new interface, but can't get Sage to build the pyx file.

$ touch src/sage/libs/polymake/polymake.pyx 
$ ./sage -br
<stuff not involving that file>
sage:

and of course that causes problems with

sage: P = polymake.convex_hull([[1,0,0,0], [1,0,0,1], [1,0,1,0], [1,0,1,1],  [1,1,0,0], [1,1,0,1], [1,1,1,0], [1,1,1,1]])

or

sage: from sage.libs.polymake import polymake

which don't work for this reason.

comment:20 Changed 6 years ago by kcrisman

Ah, I think the problem is that I had to do it from the command line and sage -sh.

sage: sage.misc.package.is_package_installed('polymake')
False

Fixing that 'by hand', it WORKS!!!

comment:21 Changed 6 years ago by kcrisman

  • Reviewers set to Karl-Dieter Crisman, Frédéric Chapoton
  • Status changed from new to needs_review

Two concrete things:

  • Please make ALL TESTS here optional. Because they pretty much all fail if you don't have polymake, not just the part that imports polymake.
  • Exactly one (!) test failure.
    **********************************************************************
    File "src/sage/libs/polymake/polymake.pyx", line 76, in sage.libs.polymake.polymake
    Failed example:
        n.graph()                         # optional - polymake
    Exception raised:
        ValueError: invalid value for an input string property
    
    I don't know what this is about, however.

comment:22 Changed 6 years ago by kcrisman

  • Status changed from needs_review to needs_work
  • Work issues set to source tar link, fix doctests, info for how to install, add doctests

I think that we need the following for a positive review.

  • Fix the doctest issues in the previous comment.
  • Provide a source tarball.
  • Provide explicit instructions for how to acquire any prereqs for polymake (notably any Perl ones)
  • Lots and lots of functions have no doctests. :-(

This also definitely should close #5488.

comment:23 Changed 6 years ago by kcrisman

  • Cc was added

Try the source package at http://sage.math.washington.edu/home/kcrisman/polymake-2.12.tar.gz - possibly with Mac tar annoying warnings, sorry. If putting this in upstream works then we are in good shape. I'll note I do get a different set of checksums, so that may have to be fixed. Where did those come from? Someone must have had the src for that!

By the way - to William - this should enable polymake installation within Sage easy in the cloud (modulo a Perl module or two).

comment:24 Changed 6 years ago by git

  • Commit changed from e14a9f2e18d3e091ceaf7a922ca5e2ff91effb7a to faa22c6b6e5cd42bd138d8fca06cd8f03d8a601c

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

16f7856Merge branch 'public/ticket/14416' into 6.5.b1
faa22c6trac #14116 some work on documentation

comment:25 Changed 6 years ago by chapoton

How can I manually install the package to test it ?

Looking at the developer manual, I have copied the tar in my upstream dir and then tried sage -i polymake, but that fails by trying to upload the sources from the web.

What should I do ?

comment:26 Changed 6 years ago by kcrisman

I think if you put the tar in the upstream and then do sage -f polymake-x.y.z it should work. the sage -i will try the 'official' (and broken) optional package.

comment:27 Changed 6 years ago by chapoton

Thanks, but alas, no, this does not work..

sage -i  polymake-2.12
Found local metadata for polymake-2.12
Attempting to download package polymake-2.12
...
IOError: [Errno 404] Not Found: '//www.sagemath.org/packages/upstream/polymake/'
Error: failed to download package polymake-2.12

comment:28 Changed 6 years ago by kcrisman

Read carefully. Put the tar file I uploaded in comment:23 in $SAGE_ROOT/upstream, then

sage -f polymake-2.12

At least, I hope this will work. See also #13768.

comment:29 Changed 6 years ago by chapoton

Ooops, sorry for misreading the -f.

Still not working. I have got your polymake-2.12.tar.gz in my $SAGE_ROOT/upstream

and nothing among sage (-i or -f) (polymake or polymake-2.12) works

I have tried both on a clean develop and on the branch here.

comment:30 Changed 6 years ago by kcrisman

Can you give me the error message when you do sage -f polymake-2.12? Maybe something is corrupted in the upload.

comment:31 Changed 6 years ago by chapoton

ls -lg upstream/po*
-rw-r--r-- 1 users  1565069 oct.  17  2013 upstream/polybori-0.8.3.tar.bz2
-rw-r--r-- 1 users 14504274 nov.  27 13:27 upstream/polymake-2.12.tar.gz
-rw-r--r-- 1 users    41376 oct.  17  2013 upstream/polytopes_db-20120220.tar.bz2
sage -f polymake-2.12
Found local metadata for polymake-2.12
Attempting to download package polymake-2.12
>>> Trying to download http://www.sagemath.org/packages/upstream/polymake/
[Traceback (most recent call last):
  File "<stdin>", line 35, in <module>
  File "/homes/doua/chapoton/sage/local/lib/python/urllib.py", line 240, in retrieve
    fp = self.open(url, data)
  File "/homes/doua/chapoton/sage/local/lib/python/urllib.py", line 208, in open
    return getattr(self, name)(url)
  File "/homes/doua/chapoton/sage/local/lib/python/urllib.py", line 359, in open_http
    return self.http_error(url, fp, errcode, errmsg, headers)
  File "/homes/doua/chapoton/sage/local/lib/python/urllib.py", line 376, in http_error
    return self.http_error_default(url, fp, errcode, errmsg, headers)
  File "<stdin>", line 17, in http_error_default
IOError: [Errno 404] Not Found: '//www.sagemath.org/packages/upstream/polymake/'
Error: failed to download package polymake-2.12

comment:32 Changed 6 years ago by kcrisman

Huh. It really shouldn't try to do that. Did you use the branch on this ticket? Otherwise it would assume it's an old-style spkg, I believe, in which case this error message would make sense.

comment:33 Changed 6 years ago by chapoton

Yes, I am doing that sitting on the branch here, having built this branch.

comment:34 Changed 6 years ago by kcrisman

Ok, I'll see if I can find some time to try this on sage.math or some other system. I suspect it's due to the packaging etc.

comment:35 Changed 6 years ago by kcrisman

Okay, I can confirm this problem. Looking into it.

comment:36 Changed 6 years ago by kcrisman

I think the problem is that now the checksum file includes a line about where the tarball lives - see also here.

For now, try applying

  • build/pkgs/polymake/checksums.ini

    diff --git a/build/pkgs/polymake/checksums.ini b/build/pkgs/polymake/checksums.i
    index e219804..75aba08 100644
    a b  
    1 sha1=a990ea31a68740cbf1aba02d49ec23ef589c173d
    2 md5=f07ecafc7a946001c38cdd20e6434b09
    3 cksum=1918099647
     1tarball=polymake-VERSION.tar.gz
     2sha1=6488bde98cc4f50f4c327ee55fae1140e2f1c1be
     3md5=b3351c8b772177a198ac0e905f07de99
     4cksum=1659608855

As noted, there are various odd Perl dependencies, see here for some Linux details.

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

comment:37 Changed 6 years ago by chapoton

Thanks. This now starts the installation, which fails later because of some Perl warnings. As I am not root on the machine on which I tried, I will have to try later on my own machine.

comment:38 Changed 6 years ago by kcrisman

Okay. You should be able to ignore the

WARNING: perl module Term::ReadLine::Gnu required for polymake not found on your machine.

warning, we just won't be able to guarantee using polymake as a command-line option without that (and on Mac that is very tricky, probably much less so on Linux). Something like

sudo perl -MCPAN -e 'install XML::LibXSLT'

should suffice for the xml stuff, assuming you have the other prereqs they recommend, I guess.

comment:39 Changed 6 years ago by chapoton

I get the same build failure on my own machine (after installing all of polymake dependencies) and on the machine of my department, namely:

In file included from /usr/lib/perl/5.18/CORE/perl.h:3482:0,
                 from /home/chapoton/sage/local/var/tmp/sage/build/polymake-2.12/src/include/core/polymake/perl/Ext.h:22,
                 from Ext.xs:18:
Ext.xs: In function 'pm_perl_get_cx_curpad':
/usr/lib/perl/5.18/CORE/av.h:62:26: error: 'PADLIST' has no member named 'sv_u'
 #define AvARRAY(av) ((av)->sv_u.svu_array)
                          ^
Ext.xs:45:17: note: in expansion of macro 'AvARRAY'
    return (SV**)AvARRAY(((AV**)AvARRAY(CvPADLIST(cv)))[d+1]);
                 ^
Ext.xs:45:32: note: in expansion of macro 'AvARRAY'
    return (SV**)AvARRAY(((AV**)AvARRAY(CvPADLIST(cv)))[d+1]);
                                ^
make[2]: *** [Ext.o] Error 1

comment:40 Changed 6 years ago by kcrisman

This is almost certainly because you have too new (!) of a Perl - see this report. See also e.g. http://forum.polymake.org/viewtopic.php?f=8&p=942#p942 which is really annoying, these things happen a lot.

It seems pretty likely that the 2.13 version should fix this. I will see if that still builds on my 10.7 Mac.

See also http://www.polymake.org/doku.php/workshop1214 which points out that Mac 10.10 compatibility is not yet achieved, probably partly for some related reason. Basically, polymake is definitely still in "optional" territory, if not experimental still!

comment:41 Changed 6 years ago by kcrisman

Try this instead of the previous, and the package at ​http://sage.math.washington.edu/home/kcrisman/polymake-2.13.tar.gz . I'm currently seeing how/whether this performs right on my machine.

  • build/pkgs/polymake/checksums.ini

    diff --git a/build/pkgs/polymake/checksums.ini b/build/pkgs/polymake/checksums.i
    index e219804..a45c75e 100644
    a b  
    1 sha1=a990ea31a68740cbf1aba02d49ec23ef589c173d
    2 md5=f07ecafc7a946001c38cdd20e6434b09
    3 cksum=1918099647
     1tarball=polymake-VERSION.tar.gz
     2sha1=72f32a1e0b8a9f8d8e12ef7563c481de1d44cd67
     3md5=92da21355405790c6c816304569a7b12
     4cksum=890996601
  • build/pkgs/polymake/package-version.txt

    diff --git a/build/pkgs/polymake/package-version.txt b/build/pkgs/polymake/packa
    index 3e162f0..ae656d4 100644
    a b  
    1 2.12
     12.13
  • build/pkgs/polymake/spkg-install

    diff --git a/build/pkgs/polymake/spkg-install b/build/pkgs/polymake/spkg-install
    index ce6723d..c7a52d1 100755
    a b fi 
    99cd src
    1010
    1111./configure --prefix="$SAGE_LOCAL" --with-boost="$SAGE_LOCAL" --with-gmp="$SAGE
    12             --with-mpfr="$SAGE_LOCAL" --with-readline="$SAGE_LOCAL" --with-fink
     12            --with-mpfr="$SAGE_LOCAL" --with-readline="$SAGE_LOCAL" --without-f
    1313if [ $? -ne 0 ]; then
    1414    echo "Failed to configure."
    1515    exit 1
Last edited 6 years ago by kcrisman (previous) (diff)

comment:42 Changed 6 years ago by kcrisman

This doesn't work for me, though. Aargh.

comment:43 Changed 6 years ago by chapoton

on my department's machine, I got that:

Unrecognized option --with-readline=/homes/doua/cha/sage/local;
Try ./configure --help for detailed information
Failed to configure.

and the configure is still beleaving it is in 6.4.rc2

comment:44 Changed 6 years ago by git

  • Commit changed from faa22c6b6e5cd42bd138d8fca06cd8f03d8a601c to c52e7981f859febc7ca6eca2eff51d36450532bb

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

c52e798Merge branch 'public/ticket/14416' into 6.7

comment:45 Changed 6 years ago by chapoton

  • Milestone changed from sage-6.4 to sage-6.8

comment:46 Changed 5 years ago by git

  • Commit changed from c52e7981f859febc7ca6eca2eff51d36450532bb to 50f6da95b155a9739c546b2f0fcd5f6edd09546f

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

50f6da9Merge branch 'public/ticket/14416' into 6.10.b1

comment:47 Changed 5 years ago by chapoton

  • Description modified (diff)

comment:48 Changed 5 years ago by chapoton

  • Milestone changed from sage-6.8 to sage-6.10
  • Summary changed from update polymake interface to 2.12-rc3 to update polymake interface to 2.14-rc1

comment:49 Changed 5 years ago by chapoton

  • Milestone changed from sage-6.10 to sage-7.0

comment:50 Changed 5 years ago by vdelecroix

New tentative at #20892 for polymake-3.0

comment:51 Changed 5 years ago by kcrisman

  • Cc mkoeppe added

Matthias, are you going to repurpose this and use some of it for the updated interface now that you have things working in #20892 for upgrading polymake?

comment:52 Changed 5 years ago by mkoeppe

I think Vincent may be working on it; and it sounds like it will be on a new ticket rather than this one here.

comment:53 Changed 5 years ago by vdelecroix

I am indeed working on the Cython interface to polymake. I should come out with a minimal version by the end of the week.

comment:54 Changed 4 years ago by mkoeppe

  • Milestone changed from sage-7.0 to sage-duplicate/invalid/wontfix
  • Status changed from needs_work to needs_review

I'm proposing to close this outdated ticket because newer efforts have made it obsolete.

comment:55 Changed 4 years ago by kcrisman

Sounds reasonable - just be sure to point to the tickets which overtake it here :)

comment:56 Changed 4 years ago by mkoeppe

That's:

  • #22452: Create a Polymake pexpect interface
  • #22658: Support interface coercion polymake(X) for Polyhedron
  • #22683: backend_polymake for Polyhedron

and, in a separate effort,

comment:57 Changed 4 years ago by kcrisman

  • Reviewers changed from Karl-Dieter Crisman, Frédéric Chapoton to Karl-Dieter Crisman, Frédéric Chapoton, Matthias Köppe
  • Status changed from needs_review to positive_review

comment:58 Changed 4 years ago by embray

  • Resolution set to wontfix
  • Status changed from positive_review to closed

Closing tickets in the sage-duplicate/invalid/wontfix module with positive_review (i.e. someone has confirmed they should be closed).

Note: See TracTickets for help on using tickets.