Opened 3 years ago

Closed 11 months ago

Last modified 8 months ago

#22704 closed enhancement (fixed)

Create a variant of the polymake interface using polymake's callable library (polymake::Main) via JuPyMake

Reported by: mkoeppe Owned by:
Priority: major Milestone: sage-8.8
Component: interfaces Keywords: days84
Cc: SimonKing, jipilab, gh-sophiasage, gh-LaisRast, gh-kliem, gh-sebasguts, vdelecroix, tscrim Merged in:
Authors: Matthias Koeppe Reviewers: Dima Pasechnik
Report Upstream: Reported upstream. No feedback yet. Work issues:
Branch: 8d9e2c2 (Commits) Commit:
Dependencies: #24905 Stopgaps:

Description (last modified by mkoeppe)

Following up on #22452 (Create a Polymake pexpect interface):

As suggested by Gawrilow in https://forum.polymake.org/viewtopic.php?f=8&t=544, we could create a variant of the polymake pexpect interface that does not go through pty's but rather replaces the relevant send/receive methods of pexpect by calls to simulate_shell_input (linking to libpolymake).

Recent polymake versions have developed this further and now provide a class polymake::Main with convenient access to the polymake interpreter. We use the (partial) Python bindings for this, https://github.com/polymake/JuPyMake which are used for Polymake's jupyter.

Upstream package:

  wget -P upstream https://files.pythonhosted.org/packages/32/dd/0a48546daa2461e57207d31eeb64a3ad77fd9fb339d90e1e067ea25991a5/JuPyMake-0.9.tar.gz

We do want to keep the current pty-based pexpect interface, though, because it may be handy for running polymake on a remote server; or for guarding against a crashing polymake (sigh).

Remaining issue: #27807

See also: #22710: Meta-ticket: polymake

Change History (59)

comment:1 Changed 3 years ago by mkoeppe

  • Dependencies set to #22452
  • Description modified (diff)

comment:2 Changed 19 months ago by mkoeppe

  • Description modified (diff)
  • Milestone changed from sage-8.0 to sage-8.4
  • Summary changed from Create a variant of the polymake pexpect interface using polymake's simulate_shell_input to Create a variant of the polymake pexpect interface using polymake's simulate_shell_input / JuPyMake

comment:3 Changed 19 months ago by mkoeppe

  • Dependencies changed from #22452 to #22452, #26333

comment:4 Changed 11 months ago by mkoeppe

  • Branch set to public/22704

comment:5 Changed 11 months ago by mkoeppe

  • Cc jipilab gh-sophiasage gh-LaisRast gh-kliem gh-sebasguts vdelecroix added
  • Commit set to 2f855b0298f80080013d7c9b1a9f58b98876d1c8
  • Description modified (diff)

Here's a first version.


Last 10 new commits:

7018013polymake doctest: Mark a doctest random that depends on installed packages
78c10fbPolymake._tab_completion: Fix for new apropos format in Polymake 3.4
1699019Doctest fix for polymake 3.4: false/true instead of 0/1
f595909Doctest fix for polymake 3.4: Use different function from the tropical application
76701f6Doctest fix for polymake 3.4: Differences in available properties
83b51dePolymake 3.4: Fix for typeof small objects
8510124Polymake: Remove copy/paste error handling for code that cannot raise
3c6ae95polymake: Make normaliz a dependency
b6eeaa1polymake: Add a patch to polymake's libnormaliz configure
2f855b0Refactor interface Polymake through PolymakeAbstract, add PolymakeJuPyMake

comment:6 Changed 11 months ago by git

  • Commit changed from 2f855b0298f80080013d7c9b1a9f58b98876d1c8 to 78b5036ac44d171568aa15774190ed3e20cb7c17

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

78b5036More doctests etc.

comment:7 Changed 11 months ago by mkoeppe

  • Description modified (diff)

comment:8 Changed 11 months ago by mkoeppe

  • Description modified (diff)

comment:9 Changed 11 months ago by git

  • Commit changed from 78b5036ac44d171568aa15774190ed3e20cb7c17 to 92274e0fd34e47c1db95bd45a6620755362e9e43

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

92274e0Add package jupymake

comment:10 Changed 11 months ago by mkoeppe

Some polymake errors are not properly signalled by Polymake: Only the stderr slot [2] is filled but the error slot [3] is empty

sage: from JuPyMake import ExecuteCommand
sage: ExecuteCommand('help("Triangulation");')
(True, '', "polymake:  ERROR: unknown help topic 'Triangulation'\n", '')
Last edited 11 months ago by mkoeppe (previous) (diff)

comment:11 Changed 11 months ago by git

  • Commit changed from 92274e0fd34e47c1db95bd45a6620755362e9e43 to 6bd4971d19129c9edbba0087f8dc452f9908ae3f

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

b40a1cbNo need for File::Slurp in jupymake interface
6bd4971Add doc

comment:12 Changed 11 months ago by git

  • Commit changed from 6bd4971d19129c9edbba0087f8dc452f9908ae3f to 0bad10c608fa88f230b4244470207b1ef2de61a3

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

0bad10cFix jupymake help

comment:13 Changed 11 months ago by git

  • Commit changed from 0bad10c608fa88f230b4244470207b1ef2de61a3 to 67086632c142d1dc4df8517ff38c55eb5c1174ca

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

624ac01polymake: Also provide core functions
6708663Merge branch 't/24905/public/upgrade_polymake_to_version_3_2r4' into t/22704/public/22704

comment:14 Changed 11 months ago by git

  • Commit changed from 67086632c142d1dc4df8517ff38c55eb5c1174ca to a614876d3b8981641c7acdd08a3612ff27bc8319

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

a5c8eb8Add optional - polymake
ad2b976Can only quit polymake in pexpect interface
7ec6c74Mark jupymake tests optional - jupymake
8b7ffbdSelect polymake interface based on feature test
a614876Fixup polymake doctests that depend on 'used package' messages

comment:15 Changed 11 months ago by mkoeppe

  • Dependencies changed from #22452, #26333 to #26333
  • Description modified (diff)

comment:16 Changed 11 months ago by mkoeppe

Signal handling issues are solved in new version of JuPyMake? - waiting for a new release

comment:17 Changed 11 months ago by jipilab

This is coming from #24905, which is already set as positive review.

Could you correct the following pyflakes warnings:

+src/sage/geometry/polyhedron/backend_polymake.py:29: 'sage.rings.all.QQ' imported but unused
+src/sage/geometry/polyhedron/backend_polymake.py:29: 'sage.rings.all.ZZ' imported but unused

comment:18 Changed 11 months ago by git

  • Commit changed from a614876d3b8981641c7acdd08a3612ff27bc8319 to bcc7c6b88bb6c622988bf3b06579a5560c0181ee

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

bcc7c6bAdd polymake patch for thread correctness

comment:19 Changed 11 months ago by mkoeppe

  • Description modified (diff)

Crashes are fixed.

comment:20 Changed 11 months ago by mkoeppe

  • Dependencies #26333 deleted

comment:21 Changed 11 months ago by mkoeppe

  • Description modified (diff)

comment:22 Changed 11 months ago by git

  • Commit changed from bcc7c6b88bb6c622988bf3b06579a5560c0181ee to 1b8e2a580423e3b215930b98d065d4514af02af9

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

34900a0backend_polymake.py: Remove unused imports
1b8e2a5Update jupymake to 0.9

comment:23 Changed 11 months ago by mkoeppe

  • Dependencies set to #24905
  • Description modified (diff)

Signal handling fixed by jupymake update

comment:24 Changed 11 months ago by mkoeppe

  • Description modified (diff)
  • Report Upstream changed from N/A to Reported upstream. No feedback yet.

polymake's error handling bug is reported upstream.

comment:25 Changed 11 months ago by mkoeppe

  • Authors set to Matthias Koeppe
  • Cc tscrim added
  • Status changed from new to needs_review

Apart from 1 remaining doctest regarding the form of some error messages, this is ready for review!

comment:26 Changed 11 months ago by mkoeppe

  • Description modified (diff)
  • Milestone changed from sage-8.4 to sage-8.8
  • Summary changed from Create a variant of the polymake pexpect interface using polymake's simulate_shell_input / JuPyMake to Create a variant of the polymake interface using polymake's callable library (polymake::Main) via JuPyMake

comment:27 Changed 11 months ago by git

  • Commit changed from 1b8e2a580423e3b215930b98d065d4514af02af9 to e46cd380e9645b0e7e6a26474aa2f69999f7cdc3

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

666db96Added two missing optional tags
4e91dccChange format of a 'not tested' doc test annotation
05f3e61update docs, as discussed on #24905
53c8ee6Mention libterm-readkey-perl
b613e57Mention libterm-readline-gnu-perl
e5c3249polymake: Mention MOngoDB for polydb
b89376apolymake: Update info on macOS
0c1b044polymake: update MongoDB information for linux distros
e46cd38Merge branch 't/24905/public/upgrade_polymake_to_version_3_2r4' into t/22704/public/22704

comment:28 Changed 11 months ago by dimpase

does this need #24905 ?

comment:29 Changed 11 months ago by mkoeppe

I did this work on top of the #24905 branch because otherwise there would be big merge conflicts.

comment:30 follow-up: Changed 11 months ago by dimpase

that's what I get after trying this on Linux:

$ ./sage -tp --optional=bliss,build,dochtml,e_antic,gap_packages,jupymake,lidia,lrslib,memlimit,ninja_build,normaliz,python2,polymake,sage src/sage/interfaces/polymake.py 
too many failed tests, not using stored timings
Running doctests with ID 2019-05-08-22-08-17-11941b32.
Git branch: testwip
Using --optional=bliss,build,dochtml,e_antic,gap_packages,jupymake,lidia,lrslib,memlimit,ninja_build,normaliz,polymake,python2,sage
Doctesting 1 file using 4 threads.
sage -t src/sage/interfaces/polymake.py
**********************************************************************
File "src/sage/interfaces/polymake.py", line 765, in sage.interfaces.polymake.PolymakeAbstract.application
Failed example:
    polymake.eval('application "killerapp";')         # optional - polymake
Expected:
    Traceback (most recent call last):
    ...
    PolymakeError: Unknown application killerapp
Got:
    <BLANKLINE>
    Traceback (most recent call last):
      File "/mnt/opt/Sage/sage-dev/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 671, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/mnt/opt/Sage/sage-dev/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 1095, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.interfaces.polymake.PolymakeAbstract.application[15]>", line 1, in <module>
        polymake.eval('application "killerapp";')         # optional - polymake
      File "/mnt/opt/Sage/sage-dev/local/lib/python2.7/site-packages/sage/interfaces/polymake.py", line 2520, in eval
        raise PolymakeError(error)
    PolymakeError: Can't locate object method "compile_scope" via package "Polymake::Core::Shell::Mock" at /mnt/opt/Sage/sage-dev/local/share/polymake/perllib/Polymake/utils.pl line 247.
    <BLANKLINE>
**********************************************************************
1 item had failures:
   1 of  17 in sage.interfaces.polymake.PolymakeAbstract.application
    [317 tests, 1 failure, 9.54 s]
----------------------------------------------------------------------
sage -t src/sage/interfaces/polymake.py  # 1 doctest failed
----------------------------------------------------------------------

comment:31 follow-up: Changed 11 months ago by dimpase

and also

**********************************************************************
File "src/sage/geometry/polyhedron/base.py", line 175, in sage.geometry.polyhedron.base.Polyhedron_base._sage_input_
Failed example:
    P = Polyhedron(vertices = [[1, 0], [0, 1]], rays = [[1, 1]], backend='polymake') # optional - polymake
Expected nothing
Got:
     gmp v.6.1
...
File "src/sage/geometry/polyhedron/backend_polymake.py", line 59, in sage.geometry.polyhedron.backend_polymake.Polyhedron_polymake
Failed example:
    p = Polyhedron(vertices=[(0,0),(1,0),(0,1)], rays=[(1,1)],   # optional - polymake
                   lines=[], backend='polymake')
Expected nothing
Got:
     gmp v.6.1

comment:32 in reply to: ↑ 30 Changed 11 months ago by mkoeppe

Replying to dimpase:

that's what I get after trying this on Linux:

$ ./sage -tp --optional=bliss,build,dochtml,e_antic,gap_packages,jupymake,lidia,lrslib,memlimit,ninja_build,normaliz,python2,polymake,sage src/sage/interfaces/polymake.py 
....
       raise PolymakeError(error)
    PolymakeError: Can't locate object method "compile_scope" via package "Polymake::Core::Shell::Mock" at /mnt/opt/Sage/sage-dev/local/share/polymake/perllib/Polymake/utils.pl line 247.

Yes, I know this one - see ticket description. I have reported it upstream.

comment:33 in reply to: ↑ 31 ; follow-up: Changed 11 months ago by mkoeppe

Replying to dimpase:

and also

**********************************************************************
File "src/sage/geometry/polyhedron/base.py", line 175, in sage.geometry.polyhedron.base.Polyhedron_base._sage_input_
Failed example:
    P = Polyhedron(vertices = [[1, 0], [0, 1]], rays = [[1, 1]], backend='polymake') # optional - polymake
Expected nothing
Got:
     gmp v.6.1
...
File "src/sage/geometry/polyhedron/backend_polymake.py", line 59, in sage.geometry.polyhedron.backend_polymake.Polyhedron_polymake
Failed example:
    p = Polyhedron(vertices=[(0,0),(1,0),(0,1)], rays=[(1,1)],   # optional - polymake
                   lines=[], backend='polymake')
Expected nothing
Got:
     gmp v.6.1

Looks like this message comes from lrslib, I'll investigate

comment:34 follow-up: Changed 11 months ago by mkoeppe

Actually on my machine polymake does not find the installed lrslib (it's looking for liblrs but we only have liblrsgmp.)

comment:35 Changed 11 months ago by dimpase

I also only have liblrsgmp. Not sure if the following helps, but still:

$ ldd local/lib/libpolymake.so
	linux-vdso.so.1 (0x00007fff97bbb000)
	libperl.so.5.28 => /usr/lib64/libperl.so.5.28 (0x00007fcae6d94000)
	libxml2.so.2 => /usr/lib64/libxml2.so.2 (0x00007fcae6bef000)
	libdl.so.2 => /lib64/libdl.so.2 (0x00007fcae6be9000)
	libmpfr.so.6 => /mnt/opt/Sage/sage-dev/local/lib/libmpfr.so.6 (0x00007fcae6b6a000)
	libgmp.so.10 => /usr/lib64/libgmp.so.10 (0x00007fcae68df000)
	libpthread.so.0 => /lib64/libpthread.so.0 (0x00007fcae68bc000)
	libstdc++.so.6 => /usr/lib/gcc/x86_64-pc-linux-gnu/9.1.0/libstdc++.so.6 (0x00007fcae6642000)
	libm.so.6 => /lib64/libm.so.6 (0x00007fcae6505000)
	libgcc_s.so.1 => /usr/lib/gcc/x86_64-pc-linux-gnu/9.1.0/libgcc_s.so.1 (0x00007fcae64eb000)
	libc.so.6 => /lib64/libc.so.6 (0x00007fcae631b000)
	libcrypt.so.1 => /lib64/libcrypt.so.1 (0x00007fcae62e2000)
	libicuuc.so.64 => /usr/lib64/libicuuc.so.64 (0x00007fcae60f1000)
	libz.so.1 => /lib64/libz.so.1 (0x00007fcae5ecf000)
	/lib64/ld-linux-x86-64.so.2 (0x00007fcae71bd000)
	libicudata.so.64 => /usr/lib64/libicudata.so.64 (0x00007fcae4489000)

the rest of polymake libs are statically linked, so it's not as easy to gather whether lrs has anything to do with it.

comment:36 in reply to: ↑ 34 Changed 11 months ago by mkoeppe

Replying to mkoeppe:

Actually on my machine polymake does not find the installed lrslib (it's looking for liblrs but we only have liblrsgmp.)

This belongs to the polymake 3.4 upgrade ticket (#24905).

comment:37 in reply to: ↑ 33 Changed 11 months ago by dimpase

Replying to mkoeppe:

Replying to dimpase:

and also

**********************************************************************
File "src/sage/geometry/polyhedron/base.py", line 175, in sage.geometry.polyhedron.base.Polyhedron_base._sage_input_
Failed example:
    P = Polyhedron(vertices = [[1, 0], [0, 1]], rays = [[1, 1]], backend='polymake') # optional - polymake
Expected nothing
Got:
     gmp v.6.1
...
File "src/sage/geometry/polyhedron/backend_polymake.py", line 59, in sage.geometry.polyhedron.backend_polymake.Polyhedron_polymake
Failed example:
    p = Polyhedron(vertices=[(0,0),(1,0),(0,1)], rays=[(1,1)],   # optional - polymake
                   lines=[], backend='polymake')
Expected nothing
Got:
     gmp v.6.1

Looks like this message comes from lrslib, I'll investigate

After the lrs fix on #27803, one of these 2 errors is still present:

File "src/sage/geometry/polyhedron/base.py", line 175, in sage.geometry.polyhedron.base.Polyhedron_base._sage_input_
Failed example:
    P = Polyhedron(vertices = [[1, 0], [0, 1]], rays = [[1, 1]], backend='polymake') # optional - polymake
Expected nothing
Got:
     gmp v.6.1

comment:38 follow-up: Changed 11 months ago by mkoeppe

Yes, and with #27803 I can reproduce it. I think these errors appear non-deterministicaly.

comment:39 in reply to: ↑ 38 ; follow-up: Changed 11 months ago by dimpase

Replying to mkoeppe:

Yes, and with #27803 I can reproduce it. I think these errors appear non-deterministicaly.

I think it is deterministic, and occurs on the 1st invocation of the polymake interface. Try starting Sage and running

P = Polyhedron(vertices = [[1, 0], [0, 1]], rays = [[1, 1]], backend='polymake')

repeatedly.

comment:40 Changed 11 months ago by mkoeppe

  • Dependencies changed from #24905 to #24905, #27803

comment:41 in reply to: ↑ 39 Changed 11 months ago by mkoeppe

Replying to dimpase:

I think it is deterministic, and occurs on the 1st invocation of the polymake interface.

You are right.

comment:42 follow-up: Changed 11 months ago by mkoeppe

The new behavior from this ticket comes from the fact that with the new in-process polymake, lrslib prints directly to stdout, so this output is not captured. Perhaps lrslib should be configured with LRS_QUIET.

comment:43 in reply to: ↑ 42 Changed 11 months ago by mkoeppe

Replying to mkoeppe:

Perhaps lrslib should be configured with LRS_QUIET.

lrslib 070 removes this output unconditionally.

comment:44 Changed 11 months ago by mkoeppe

  • Dependencies changed from #24905, #27803 to #24905, #27803, #27805

comment:45 Changed 11 months ago by git

  • Commit changed from e46cd380e9645b0e7e6a26474aa2f69999f7cdc3 to c84d89d552396a83638ee7a95de77c1a01c54e58

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

2eda6e1polymake: add to fedora instructions
5916141Fix polymake lrslib detection
086bb2dMerge branch 't/27803/public/27803-polymake-lrslib-detection' into t/22704/public/22704
1b47911Configure lrslib with LRS_QUIET
c84d89dMerge branch 't/27805/configure_lrslib_with_lrs_quiet' into t/22704/public/22704

comment:46 Changed 11 months ago by mkoeppe

The lrslib 070 update (#27745) is too much work for today. Instead I configure lrslib with LRS_QUIET (#27805).

comment:47 Changed 11 months ago by dimpase

  • Dependencies changed from #24905, #27803, #27805 to #24905
  • Reviewers set to Dima Pasechnik
  • Status changed from needs_review to positive_review

OK, this is fixed by #27805 indeed.


New commits:

2eda6e1polymake: add to fedora instructions
5916141Fix polymake lrslib detection
086bb2dMerge branch 't/27803/public/27803-polymake-lrslib-detection' into t/22704/public/22704
1b47911Configure lrslib with LRS_QUIET
c84d89dMerge branch 't/27805/configure_lrslib_with_lrs_quiet' into t/22704/public/22704

comment:48 Changed 11 months ago by mkoeppe

  • Description modified (diff)

I've put the remaining minor issue to a follow-up ticket, #27807 (Polymake-jupymake interface: Fix polymake's error handling in Shell::Mock)

comment:49 Changed 11 months ago by mkoeppe

Should I make that last failing doctest less specific regarding the error message for this ticket? (Before it's fixed fully in #27807)

comment:50 Changed 11 months ago by dimpase

If you like, you can tag it "known error".

comment:51 Changed 11 months ago by git

  • Commit changed from c84d89d552396a83638ee7a95de77c1a01c54e58 to 58aa1ad052d7c2901b919cac9863d8d867d574c1
  • Status changed from positive_review to needs_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

58aa1adMark issue of #27807 as known bug

comment:52 Changed 11 months ago by mkoeppe

  • Status changed from needs_review to positive_review

comment:53 Changed 11 months ago by vbraun

  • Status changed from positive_review to needs_work

Merge conflict

comment:54 Changed 11 months ago by git

  • Commit changed from 58aa1ad052d7c2901b919cac9863d8d867d574c1 to 8d9e2c26686c4bbb0fac7d9b30fd60a11ef37446

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

8d9e2c2Merge tag '8.8.beta5' into t/22704/public/22704

comment:55 Changed 11 months ago by mkoeppe

  • Status changed from needs_work to positive_review

comment:56 Changed 11 months ago by vbraun

  • Branch changed from public/22704 to 8d9e2c26686c4bbb0fac7d9b30fd60a11ef37446
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:57 Changed 9 months ago by mkoeppe

  • Commit 8d9e2c26686c4bbb0fac7d9b30fd60a11ef37446 deleted

The upstream package jupymake seems to be missing on the sage mirrors

comment:58 follow-up: Changed 8 months ago by jipilab

Do I understand correctly that in the current version jupymake is necessary for polymake to function properly? Shouldn't jupymake become a dependancy of polymake?

comment:59 in reply to: ↑ 58 Changed 8 months ago by mkoeppe

Replying to jipilab:

Do I understand correctly that in the current version jupymake is necessary for polymake to function properly? Shouldn't jupymake become a dependancy of polymake?

No, there are two interface classes, which both work. PolymakeExpect (aliases to Polymake) and PolymakeJupyMake. Depending on whether you have jupymake, polymake is set to an instance of one or the other.

Note: See TracTickets for help on using tickets.