Opened 5 years ago
Closed 5 years ago
#22658 closed enhancement (fixed)
Support interface coercion polymake(X) for Polyhedron
Reported by: | mkoeppe | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-8.0 |
Component: | geometry | Keywords: | days84 |
Cc: | SimonKing, jipilab, vdelecroix, tscrim, yzh, dimpase | Merged in: | |
Authors: | Matthias Koeppe, Simon King | Reviewers: | Jean-Philippe Labbé, Travis Scrimshaw |
Report Upstream: | N/A | Work issues: | |
Branch: | d7d4234 (Commits, GitHub, GitLab) | Commit: | d7d4234f660f8b154a1d3b28cc9a8d0883f9a38c |
Dependencies: | #22452 | Stopgaps: |
Description (last modified by )
Following up on the Polymake pexpect interface (#22452), we make it easy to make a polymake object corresponding to a given Polyhedron
.
We use the standard interface coercion protocol by defining SageObject._polymake_
and a default method SageObject._polymake_init_
, which is overridden for Polyhedron_base
and for several rings.
Basic example (see Polyhedron_base._polymake_init_
for more):
sage: P = polytopes.cube() sage: PP = polymake(P) # optional - polymake sage: PP.N_VERTICES # optional - polymake 8
Example of an algebraic polyhedron:
sage: P = polytopes.dodecahedron(); P A 3-dimensional polyhedron in (Number Field in sqrt5 with defining polynomial x^2 - 5)^3 defined as the convex hull of 20 vertices sage: print("There may be a recompilation warning"); PP = polymake(P); PP There may be a recompilation warning... Polytope<QuadraticExtension<Rational>>[...] sage: sorted(PP.VERTICES[:])[0] 1 1-1r5 4-2r5 0
The branch is on top of #22452; and it also contains a quick fix for the polymake interface regarding the way that files are used to send long commands.
Change History (85)
comment:1 Changed 5 years ago by
comment:2 Changed 5 years ago by
- Cc jipilab added; jpilab removed
comment:3 Changed 5 years ago by
The problem arises when converting the list of lists of integers:
sage: polymake([[12, -2, -3, -5, -8, -13, -21, -34, -55], ....: [0, 1, 0, 0, 0, 0, 0, 0, 0], ....: [0, 0, 0, 0, 0, 0, 0, 0, 1], ....: [0, 0, 0, 0, 0, 0, 0, 1, 0], ....: [0, 0, 0, 0, 0, 0, 1, 0, 0], ....: [0, 0, 0, 0, 0, 1, 0, 0, 0], ....: [0, 0, 0, 0, 1, 0, 0, 0, 0], ....: [0, 0, 0, 1, 0, 0, 0, 0, 0], ....: [0, 0, 1, 0, 0, 0, 0, 0, 0]]) Traceback (most recent call last): ... TypeError: Polymake terminated unexpectedly while reading in a large line: reference to an undeclared variable @SAGE9 at /home/king/.sage/temp/king-ThinkPad-T430/4038/interface/tmp4212 line 1.
comment:4 Changed 5 years ago by
The interesting thing is that in the file there is no reference to a variable. Instead, a variable (with a different name) is defined in the file.
comment:5 Changed 5 years ago by
Is that a bug in polymake, perhaps? The content of the file is
@SAGE47=([[12, -2, -3, -5, -8, -13, -21, -34, -55], [0, 1, 0, 0, 0, 0, 0, 0, 0], [0, 0, 0, 0, 0, 0, 0, 0, 1], [0, 0, 0, 0, 0, 0, 0, 1, 0], [0, 0, 0, 0, 0, 0, 1, 0, 0], [0, 0, 0, 0, 0, 1, 0, 0, 0], [0, 0, 0, 0, 1, 0, 0, 0, 0], [0, 0, 0, 1, 0, 0, 0, 0, 0], [0, 0, 1, 0, 0, 0, 0, 0, 0]]);
When I feed this directly into polymake, all is fine. But when I read it as a script, I get
polytope > script "/home/king/.sage/temp/king-ThinkPad-T430/4038/interface/tmp4212"; polymake: ERROR: reference to an undeclared variable @SAGE47 at /home/king/.sage/temp/king-ThinkPad-T430/4038/interface/tmp4212 line 1.
Question to polymake experts: Is polymake interpreting a script differently from a user interaction?
comment:6 Changed 5 years ago by
I posted the question in the polymake forum
comment:7 Changed 5 years ago by
Apparently it is safer to use declare @SAGE47=...;
. I wonder why so far it has worked without "declare", but I was told that when reading a script, variables need to be declared explicitly, either by my
or by declare
, depending on whether they are only declared in the script or shall still be available later.
That said, after changing the set()
method, I get a different error:
polymake.new_object("Polytope", FACETS=[[12, -2, -3, -5, -8, -13, -21, -34, -55], ....: [0, 1, 0, 0, 0, 0, 0, 0, 0], ....: [0, 0, 0, 0, 0, 0, 0, 0, 1], ....: [0, 0, 0, 0, 0, 0, 0, 1, 0], ....: [0, 0, 0, 0, 0, 0, 1, 0, 0], ....: [0, 0, 0, 0, 0, 1, 0, 0, 0], ....: [0, 0, 0, 0, 1, 0, 0, 0, 0], ....: [0, 0, 0, 1, 0, 0, 0, 0, 0], ....: [0, 0, 1, 0, 0, 0, 0, 0, 0]]) Traceback (most recent call last): ... TypeError: multiple declaration of a variable at input line 1.
Also, the following is not what I was hoping for:
sage: polymake([[12, -2, -3, -5, -8, -13, -21, -34, -55], ....: [0, 1, 0, 0, 0, 0, 0, 0, 0], ....: [0, 0, 0, 0, 0, 0, 0, 0, 1], ....: [0, 0, 0, 0, 0, 0, 0, 1, 0], ....: [0, 0, 0, 0, 0, 0, 1, 0, 0], ....: [0, 0, 0, 0, 0, 1, 0, 0, 0], ....: [0, 0, 0, 0, 1, 0, 0, 0, 0], ....: [0, 0, 0, 1, 0, 0, 0, 0, 0], ....: [0, 0, 1, 0, 0, 0, 0, 0, 0]]) 0
comment:8 Changed 5 years ago by
Probably the reason for the new error is that on the command line, a variable can be declared only once. But certainly Perl/polymake allows to explicitly un-declare a variable. So, as soon I have found out how to un-declare, I can fix that.
comment:9 follow-up: ↓ 11 Changed 5 years ago by
It REALLY SUCKS that trac keeps logging me out automatically after short time!
What I wanted to say: At polymake forum I was told that scripts somehow are cached - which basically means that the use of scripts in order to send commands to polymake is leaking memory.
So, perhaps better disallow using files. When I do it, the definition of the polytope initially seems to work:
sage: P = polymake.new_object("Polytope", FACETS=[[12, -2, -3, -5, -8, -13, -21, -34, -55], ....: [0, 1, 0, 0, 0, 0, 0, 0, 0], ....: [0, 0, 0, 0, 0, 0, 0, 0, 1], ....: [0, 0, 0, 0, 0, 0, 0, 1, 0], ....: [0, 0, 0, 0, 0, 0, 1, 0, 0], ....: [0, 0, 0, 0, 0, 1, 0, 0, 0], ....: [0, 0, 0, 0, 1, 0, 0, 0, 0], ....: [0, 0, 0, 1, 0, 0, 0, 0, 0], ....: [0, 0, 1, 0, 0, 0, 0, 0, 0]]) sage: P Polytope<Rational>[SAGE185] sage: P.N_FACETS 9
but alas, something goes foobar:
sage: P.VERTICES Traceback (most recent call last): ... AttributeError:
whereas in a polymake session I get
polytope > $P = new Polytope(FACETS=>[[12, -2, -3, -5, -8, -13, -21, -34, -55], [0, 1, 0, 0, 0, 0, 0, 0, 0], [0, 0, 0, 0, 0, 0, 0, 0, 1], [0, 0, 0, 0, 0, 0, 0, 1, 0], [0, 0, 0, 0, 0, 0, 1, 0, 0], [0, 0, 0, 0, 0, 1, 0, 0, 0], [0, 0, 0, 0, 1, 0, 0, 0, 0], [0, 0, 0, 1, 0, 0, 0, 0, 0], [0, 0, 1, 0, 0, 0, 0, 0, 0]]); polytope > print $P->VERTICES; polymake: used package ppl The Parma Polyhedra Library (PPL): A C++ library for convex polyhedra and other numerical abstractions. http://www.cs.unipr.it/ppl/ 1 6 0 0 0 0 0 0 0 1 0 4 0 0 0 0 0 0 1 0 0 0 0 0 0 0 0 1 0 0 12/5 0 0 0 0 0 1 0 0 0 0 0 0 0 12/55 1 0 0 0 0 0 0 6/17 0 1 0 0 0 0 0 4/7 0 0 1 0 0 0 0 12/13 0 0 0 1 0 0 0 3/2 0 0 0 0
comment:10 follow-up: ↓ 15 Changed 5 years ago by
The interface seems to be out of sync in the example above:
sage: P = polymake.new_object("Polytope", FACETS=[[12, -2, -3, -5, -8, -13, -21, -34, -55], ....: [0, 1, 0, 0, 0, 0, 0, 0, 0], ....: [0, 0, 0, 0, 0, 0, 0, 0, 1], ....: [0, 0, 0, 0, 0, 0, 0, 1, 0], ....: [0, 0, 0, 0, 0, 0, 1, 0, 0], ....: [0, 0, 0, 0, 0, 1, 0, 0, 0], ....: [0, 0, 0, 0, 1, 0, 0, 0, 0], ....: [0, 0, 0, 1, 0, 0, 0, 0, 0], ....: [0, 0, 1, 0, 0, 0, 0, 0, 0]]) sage: P._name '$SAGE91[0]' sage: polymake.eval("print $SAGE91[0]->VERTICES;") '' sage: polymake.eval("print $SAGE91[0]->VERTICES;") '1 6 0 0 0 0 0 0 0\n1 0 4 0 0 0 0 0 0\n1 0 0 0 0 0 0 0 0\n1 0 0 12/5 0 0 0 0 0\n1 0 0 0 0 0 0 0 12/55\n1 0 0 0 0 0 0 6/17 0\n1 0 0 0 0 0 4/7 0 0\n1 0 0 0 0 12/13 0 0 0\n1 0 0 0 3/2 0 0 0 0\n' sage: P.VERTICES 1 6 0 0 0 0 0 0 0 1 0 4 0 0 0 0 0 0 1 0 0 0 0 0 0 0 0 1 0 0 12/5 0 0 0 0 0 1 0 0 0 0 0 0 0 12/55 1 0 0 0 0 0 0 6/17 0 1 0 0 0 0 0 4/7 0 0 1 0 0 0 0 12/13 0 0 0 1 0 0 0 3/2 0 0 0 0
Or perhaps there is a different reason:
sage: polymake.eval("print $SAGE91[0]->F_VECTOR;") '9 36 84 126 126 84 36 9' sage: P.F_VECTOR Traceback (most recent call last): ... AttributeError:
So, sometimes the attribute error is stable, sometimes it vanishes.
comment:11 in reply to: ↑ 9 ; follow-up: ↓ 12 Changed 5 years ago by
Replying to SimonKing:
It REALLY SUCKS that trac keeps logging me out automatically after short time!
Maybe because your IP address changes automatically?
comment:12 in reply to: ↑ 11 ; follow-up: ↓ 17 Changed 5 years ago by
comment:13 Changed 5 years ago by
- Branch set to u/SimonKing/polyhedron_methods_using_the_polymake_pexpect_interface
comment:14 follow-up: ↓ 20 Changed 5 years ago by
- Commit set to 2c63f8da05b2caba5cdf7ff17c632146fc64ee18
By the way, would it make it easier for you to stay synced if there was no terminal echo?
Then consider adding stty -echo;
just before the "env". We did this in #20173 to work around a problem in the Mathematica pexpect interface.
Last 10 new commits:
f90647d | Replace some tests by more realistic use cases of Polymake, and mark some others as 'random'
|
bcbabe5 | Change Input to INPUT
|
1238261 | Fix a doc link. Fix dealing with timeouts. Demonstrate how to set timeouts
|
94460ae | Add further tests, fix doc formatting
|
643d6b4 | Make it clearer that 'set()' is an internal function. Make reaction on interrupt more stable
|
a8157a4 | Mark three tests as optional and create a proper WARNING block.
|
52226e2 | Do not _check_valid in _repr_, since it is done in __repr__
|
726d3fd | Merge branch 't/22501/make_it_easier_to_customize_pexpect_interfaces_new' into t/22452/create_a_polymake_pexpect_interface
|
185430f | Use print function, not print statement, in doctests
|
2c63f8d | Do not use file to send commands to polymake
|
comment:15 in reply to: ↑ 10 Changed 5 years ago by
- Branch u/SimonKing/polyhedron_methods_using_the_polymake_pexpect_interface deleted
- Commit 2c63f8da05b2caba5cdf7ff17c632146fc64ee18 deleted
Replying to SimonKing:
The interface seems to be out of sync in the example above:
But it does work if I simply disallow the use of files and do not change the declaration of variables. So, I provided a quick fix plus doctest.
comment:16 Changed 5 years ago by
- Dependencies set to #22452
comment:17 in reply to: ↑ 12 ; follow-ups: ↓ 19 ↓ 23 Changed 5 years ago by
- Branch set to u/SimonKing/polyhedron_methods_using_the_polymake_pexpect_interface
- Commit set to 2c63f8da05b2caba5cdf7ff17c632146fc64ee18
- Dependencies #22452 deleted
You mean my IP address could change while being connected to eduroam, or also while being connected to ethernet?
Yes, actually, the same just happened to me now to come and write this comment and I'm using eduroam (without a lan connection at the same time) ...and this never happened to me in Olot for example.
It is explains anything at all...
comment:18 Changed 5 years ago by
- Dependencies set to #22452
comment:19 in reply to: ↑ 17 ; follow-up: ↓ 21 Changed 5 years ago by
- Dependencies #22452 deleted
Replying to jipilab:
You mean my IP address could change while being connected to eduroam, or also while being connected to ethernet?
Yes, actually, the same just happened to me now to come and write this comment and I'm using eduroam (without a lan connection at the same time) ...and this never happened to me in Olot for example.
It is explains anything at all...
It also sucks that some changes that I just did have automatically been deleted by the simple fact that you posted something.
comment:20 in reply to: ↑ 14 Changed 5 years ago by
Replying to mkoeppe:
By the way, would it make it easier for you to stay synced if there was no terminal echo? Then consider adding
stty -echo;
just before the "env". We did this in #20173 to work around a problem in the Mathematica pexpect interface.
I will try. At least it would slightly reduce traffic in the interface.
comment:21 in reply to: ↑ 19 Changed 5 years ago by
Replying to SimonKing:
Replying to jipilab:
You mean my IP address could change while being connected to eduroam, or also while being connected to ethernet?
Yes, actually, the same just happened to me now to come and write this comment and I'm using eduroam (without a lan connection at the same time) ...and this never happened to me in Olot for example.
It is explains anything at all...
It also sucks that some changes that I just did have automatically been deleted by the simple fact that you posted something.
Yes, I'm sorry for that... Trac does suck for that as well(!)
comment:22 Changed 5 years ago by
One needs to be careful to click "revert" when there's a red conflict marker.
comment:23 in reply to: ↑ 17 Changed 5 years ago by
Replying to jipilab:
You mean my IP address could change while being connected to eduroam, or also while being connected to ethernet?
Yes, actually, the same just happened to me now to come and write this comment and I'm using eduroam (without a lan connection at the same time) ...and this never happened to me in Olot for example.
IIRC, it happened in Olot to me all the time.
I tried to change things so that there is no terminal echo, but then I immediately get an error as soon as I do anything with polymake (and yes, I did set terminal_echo=False
in the initialisation of the pexpect interface).
comment:24 Changed 5 years ago by
The way that some interfaces are written seems to assume that there is echo, and important lines are discarded as echo. I don't remember the details
comment:25 Changed 5 years ago by
Thanks for the patch, this is progress.
comment:26 Changed 5 years ago by
- Branch changed from u/SimonKing/polyhedron_methods_using_the_polymake_pexpect_interface to u/mkoeppe/polyhedron_methods_using_the_polymake_pexpect_interface
comment:27 follow-up: ↓ 28 Changed 5 years ago by
- Commit changed from 2c63f8da05b2caba5cdf7ff17c632146fc64ee18 to ecd99ea780ef26e49c716cc977bb637cba640eda
So here's a Polyhedron._polymake_
method.
I guess the next step would be to provide a _sage_
method for PolymakeElement
which can convert Perl arrays(?) to Sage lists (ideally, based on calls to __len__
and __getitem__
, not parsing!).
The inherited method already knows how to convert numbers:
sage: P = polytopes.cube() sage: PP = P._polymake_() sage: PP.N_VERTICES 8 sage: _._sage_() 8 sage: type(_) <type 'sage.rings.integer.Integer'>
Simon, is this something that you're planning to work on?
New commits:
ecd99ea | Polyhedron_base._polymake_: New
|
comment:28 in reply to: ↑ 27 Changed 5 years ago by
Replying to mkoeppe:
Simon, is this something that you're planning to work on?
Currently Sage has a relatively low priority, as I have to set up a written exam on differential equations, to grade two 40 student's written exams, to edit a book chapter and to prepare three lectures courses that will start in 10 days.
comment:29 Changed 5 years ago by
It may be reasonable/faster/more stable to not rely on automatic conversion of lists of lists of integers to Polymake, but to do the conversion explicitly, in the _polymake_
method.
comment:30 Changed 5 years ago by
- Commit changed from ecd99ea780ef26e49c716cc977bb637cba640eda to 7d1187c084d0179d0e09e29ecab7aa78ae536bbe
Branch pushed to git repo; I updated commit sha1. New commits:
7d1187c | PolymakeElement.__getitem__: Support slices
|
comment:31 follow-ups: ↓ 37 ↓ 49 Changed 5 years ago by
I think the _polymake_
method doesn't work how it is supposed to.
Have a look at sage.structure.sage_object
: There, all conversion methods _foobar_
are defined, and are customised with a _foobar_init_
.
So, I guess one should implement _polymake_
for SageObject
, and also a default version of _polymake_init_
(which defaults to the string representation and thus works out of the box for numbers and strings), similar to the other conversion methods. And then, you can override _polymake_init_
(but not _polymake_
!) for Polyhedron_base
, returning a string rather than a polymake element.
comment:32 follow-ups: ↓ 36 ↓ 58 Changed 5 years ago by
A question about workflow: My commits so far didn't concern polyhedron methods --- they are just fixes to the polymake interface (namely: "communicate with polymake without files"). I have further ideas (namely: "do communicate with polymake with files, but this time declare the variables properly"), and you as well ("avoid terminal echo"). Should we really do these fixes here, or open another ticket, which would be another dependency?
comment:33 Changed 5 years ago by
PS: The following should work, but doesn't, with the current approach.
sage: P = polytopes.cube() sage: polymake(P) --------------------------------------------------------------------------- TypeError Traceback (most recent call last) <ipython-input-2-d5e7b6594a93> in <module>() ----> 1 polymake(P) /home/king/Sage/git/sage/local/lib/python2.7/site-packages/sage/interfaces/interface.pyc in __call__(self, x, name) 259 return cls(self, x, name=name) 260 try: --> 261 return self._coerce_from_special_method(x) 262 except TypeError: 263 raise /home/king/Sage/git/sage/local/lib/python2.7/site-packages/sage/interfaces/interface.pyc in _coerce_from_special_method(self, x) 285 s = '_gp_' 286 try: --> 287 return (x.__getattribute__(s))(self) 288 except AttributeError: 289 return self(x._interface_init_()) TypeError: __call__() takes exactly 0 positional arguments (1 given)
So, better implement _polymake_
analogously to _singular_
, _gap_
, ... for SageObject
.
EDIT: _coerce_from_special_method_
results in a call to _name_of_the_interface_()
.
comment:34 follow-up: ↓ 35 Changed 5 years ago by
PPS: Is it really the case that we only want to define a polyhedron using FACETS and AFFINE_HULL? Why not via VERTICES?
comment:35 in reply to: ↑ 34 ; follow-ups: ↓ 38 ↓ 39 Changed 5 years ago by
Replying to SimonKing:
PPS: Is it really the case that we only want to define a polyhedron using FACETS and AFFINE_HULL? Why not via VERTICES?
We probably want to pass both the H- and V-description to polymake because both are computed already.
However, for unbounded, nonpointed polyhedra, I think it's more complicated to get it right, because what a Sage Polyhedron
calls "vertices" are not vertices. So plenty of tests are needed to get it right.
comment:36 in reply to: ↑ 32 Changed 5 years ago by
Replying to SimonKing:
A question about workflow: My commits so far didn't concern polyhedron methods --- they are just fixes to the polymake interface (namely: "communicate with polymake without files"). I have further ideas (namely: "do communicate with polymake with files, but this time declare the variables properly"), and you as well ("avoid terminal echo"). Should we really do these fixes here, or open another ticket, which would be another dependency?
A separate ticket would be fine. In fact, since there is very little code on Polyhedron methods as of now, it's perhaps easiest to just rename this ticket and make a fresh ticket for the Polyhedron methods.
comment:37 in reply to: ↑ 31 ; follow-up: ↓ 40 Changed 5 years ago by
Replying to SimonKing:
I think the
_polymake_
method doesn't work how it is supposed to.Have a look at
sage.structure.sage_object
: There, all conversion methods_foobar_
are defined, and are customised with a_foobar_init_
.So, I guess one should implement
_polymake_
forSageObject
, and also a default version of_polymake_init_
(which defaults to the string representation and thus works out of the box for numbers and strings), similar to the other conversion methods. And then, you can override_polymake_init_
(but not_polymake_
!) forPolyhedron_base
, returning a string rather than a polymake element.
Thanks a lot! I'll look into it.
It seems to me that I do want a Polyhedron
method that does what my _polymake_
method did. What should I call it?
comment:38 in reply to: ↑ 35 ; follow-up: ↓ 42 Changed 5 years ago by
We probably want to pass both the H- and V-description to polymake because both are computed already. However, for unbounded, nonpointed polyhedra, I think it's more complicated to get it right, because what a Sage
Polyhedron
calls "vertices" are not vertices. So plenty of tests are needed to get it right.
I am not 100% sure, but I guess this only happens when the polyhedron has a lineality space (if it is not pointed but does not contain a line, then I guess vertices are still the correct vertices), for which sometimes the choice is not directly predictable (one should open the hood and checkout how it is done...)
I just put examples here for later test cases.
Examples:
sage: Q = Polyhedron(lines = [[-1, 1]], vertices = [[1, 1]]) sage: Q.vertices() (A vertex at (2, 0),) sage: R = Polyhedron(rays = [[0, 1, 0], [1, 0, 0]], vertices = [[0, 1, 0], [1, 0 ....: , 0]], lines = [[0, 0, 1]]) sage: R.vertices() (A vertex at (0, 1, 0), A vertex at (1, 0, 0))
comment:39 in reply to: ↑ 35 Changed 5 years ago by
Replying to mkoeppe:
Replying to SimonKing:
PPS: Is it really the case that we only want to define a polyhedron using FACETS and AFFINE_HULL? Why not via VERTICES?
We probably want to pass both the H- and V-description to polymake because both are computed already. However, for unbounded, nonpointed polyhedra, I think it's more complicated to get it right, because what a Sage
Polyhedron
calls "vertices" are not vertices. So plenty of tests are needed to get it right.
But certainly there are cases where a Sage polyhedron does know its vertices, and thus they could be provided to the polymake constructor. I am not saying that this should *always* be done.
comment:40 in reply to: ↑ 37 Changed 5 years ago by
Replying to mkoeppe:
It seems to me that I do want a
Polyhedron
method that does what my_polymake_
method did. What should I call it?
I don't understand that. Why do you want P.Polyhedron()
, if polymake(P)
works? Moreover, it isn't clear from the name that P.Polyhedron()
returns anything that is related with polymake.
comment:41 follow-up: ↓ 43 Changed 5 years ago by
What I meant is a method of the Polyhedron
classes.
comment:42 in reply to: ↑ 38 ; follow-up: ↓ 45 Changed 5 years ago by
Replying to jipilab:
We probably want to pass both the H- and V-description to polymake because both are computed already. However, for unbounded, nonpointed polyhedra, I think it's more complicated to get it right, because what a Sage
Polyhedron
calls "vertices" are not vertices. So plenty of tests are needed to get it right.I am not 100% sure, but I guess this only happens when the polyhedron has a lineality space
Yes, exactly. We may be used to different terminologies, but in my book, pointed = trivial lineality space = minimal faces are vertices.
comment:43 in reply to: ↑ 41 ; follow-up: ↓ 44 Changed 5 years ago by
Replying to mkoeppe:
What I meant is a method of the
Polyhedron
classes.
I still don't see why there should be such method, if polymake(P)
works. The implementation via _polymake_init_
would be for Polyhedron_base
, I suppose.
comment:44 in reply to: ↑ 43 Changed 5 years ago by
Replying to SimonKing:
I still don't see why there should be such method, if
polymake(P)
works. The implementation via_polymake_init_
would be forPolyhedron_base
, I suppose.
OK, I'll try to get polymake(P)
working. Thanks for the discussion!
comment:45 in reply to: ↑ 42 ; follow-up: ↓ 50 Changed 5 years ago by
Yes, exactly. We may be used to different terminologies, but in my book, pointed = trivial lineality space = minimal faces are vertices.
Okay! Yes, sounds good! I guess we are now on the same page ... of the same book.
comment:46 Changed 5 years ago by
Another observation about the interface:
sage: polymake.show_preferences() 1
This should instead print something (like it does in polymake:)
polytope > show_preferences; "default" (#) "tikz.lattice" (#) "*.convex_hull ppl, cdd, lrs, beneath_beyond" (#) "projection.integer_points" Preferences marked with (#) are in effect by default, as specified in the rule files
comment:47 Changed 5 years ago by
- Commit changed from 7d1187c084d0179d0e09e29ecab7aa78ae536bbe to 65988239e463e512f11d759eb8c01f8c71d812f7
Branch pushed to git repo; I updated commit sha1. New commits:
65151e7 | SageObject: Add _polymake_, _polymake_init_ methods
|
7588b62 | Polyhedron_base: Provide _polymake_init_ instead of @cached_method _polymake_
|
07270d4 | Polyhedron_base._polymake_init_: Pass both representations to polymake, add tests
|
5fadbcd | Add _polymake_init_ for some rings
|
3d3cef2 | Polymake: fix typo in docstring
|
6598823 | Polyhedron_base._polymake_init_: Support polyhedra over quadratic extensions
|
comment:48 Changed 5 years ago by
- Commit changed from 65988239e463e512f11d759eb8c01f8c71d812f7 to ef23af4ada9587307f001690662e3c4d715fb6e6
Branch pushed to git repo; I updated commit sha1. New commits:
ef23af4 | Polyhedron_base._polymake_init_: Add test for RDF polyhedra
|
comment:49 in reply to: ↑ 31 Changed 5 years ago by
Replying to SimonKing:
So, I guess one should implement
_polymake_
forSageObject
, and also a default version of_polymake_init_
(which defaults to the string representation and thus works out of the box for numbers and strings), similar to the other conversion methods. And then, you can override_polymake_init_
(but not_polymake_
!) forPolyhedron_base
, returning a string rather than a polymake element.
I have now reworked this along these lines. The default _polymake_init
(in SageObject
) works for some basic things such as integers and rationals.
I provide a specialized _polymake_init_
for some rings (the parents). For example, polymake(QQ)
gives the polymake type name"Rational"
.
Note, in Polyhedron.base._polymake_init_
, I am still using polymake.new_object
rather than putting together a complicated string. This seems to work fine, and seems more natural.
polymake(P)
now works when P
is a Polyhedron
over the rationals, over RDF
, or over a quadratic extension of the rationals.
comment:50 in reply to: ↑ 45 Changed 5 years ago by
Replying to jipilab:
Yes, exactly. We may be used to different terminologies, but in my book, pointed = trivial lineality space = minimal faces are vertices.
Okay! Yes, sounds good! I guess we are now on the same page ... of the same book.
Turns out that polymake does a similar thing as Sage does, so it's less tricky than I thought it would be. I'm now passing both the H- and the V-rep to polymake. I've added tests for the various cases.
comment:51 Changed 5 years ago by
- Dependencies set to #22452
- Description modified (diff)
- Status changed from new to needs_review
- Summary changed from Polyhedron methods using the polymake pexpect interface to Support interface coercion polymake(X) for Polyhedron
comment:52 Changed 5 years ago by
- Description modified (diff)
comment:53 Changed 5 years ago by
- Commit changed from ef23af4ada9587307f001690662e3c4d715fb6e6 to 205879fb118e1c7c3fd954e42022b7fe0c6af119
Branch pushed to git repo; I updated commit sha1. New commits:
205879f | Matrix, MatrixSpace: Add coercion to polymake interface
|
comment:54 Changed 5 years ago by
- Cc tscrim added
comment:55 Changed 5 years ago by
- Cc yzh added
comment:56 Changed 5 years ago by
- Cc dimpase added
comment:57 Changed 5 years ago by
- Commit changed from 205879fb118e1c7c3fd954e42022b7fe0c6af119 to e49765eaa94319320c1a40e820d45010b32ea94d
Branch pushed to git repo; I updated commit sha1. New commits:
e49765e | Polymake: Fix tests whose error messages changed because we do not use files
|
comment:58 in reply to: ↑ 32 Changed 5 years ago by
Replying to SimonKing:
I have further ideas (namely: "do communicate with polymake with files, but this time declare the variables properly")
Yes, this is needed; I just got bitten by the 4096-characters-per-line limitation...
comment:59 Changed 5 years ago by
- Commit changed from e49765eaa94319320c1a40e820d45010b32ea94d to 5c5948c802d498a2313dac022e7bea175cb0ec5c
Branch pushed to git repo; I updated commit sha1. New commits:
5c5948c | Polymake: Use files again, using 'eval read_file' instead of 'script'
|
comment:60 follow-up: ↓ 64 Changed 5 years ago by
It seems I found a simple solution for communicating with files.
comment:61 Changed 5 years ago by
Do you want it to be 100 characters (IIRC that is what the 100 specifies), or do you want to go higher so files are written less frequently, like 1024 or 2048?
comment:62 Changed 5 years ago by
1024 works for me, larger values don't. I'll make a change and add tests.
comment:63 Changed 5 years ago by
- Commit changed from 5c5948c802d498a2313dac022e7bea175cb0ec5c to 5b6a921419f3d4efd20d7d57b79675e861cca9fc
Branch pushed to git repo; I updated commit sha1. New commits:
5b6a921 | Polymake: Change file cut off to 1024 and add tests
|
comment:64 in reply to: ↑ 60 ; follow-up: ↓ 65 Changed 5 years ago by
Replying to mkoeppe:
It seems I found a simple solution for communicating with files.
Sigh. Why didn't the polymake developers point me to that solution when I asked in the polymake forum? I explicitly asked for a way to read commands from a file and mentioned that the only solution I found was script
, but I made it clear that I do certainly not insist on using script
. Their answer was to not point out alternatives, but to explain oddities that come with script
, such as different conventions on declaring variables, and caching.
Do I see correctly that eval read_file
will not cache what it read? That would be another advantage over script
and would have been yet another reason for the polymake forum to mention that solution.
In singular.py, there is
eval_using_file_cutoff=100 if os.uname()[0]=="SunOS" else 1000
and in gap it is unconditionally 100. So, perhaps 1024 won't work on some systems?
comment:65 in reply to: ↑ 64 Changed 5 years ago by
Replying to SimonKing:
Do I see correctly that
eval read_file
will not cache what it read?
No caching. It's just Perl. eval
executes a given string of code.
In singular.py, there is
eval_using_file_cutoff=100 if os.uname()[0]=="SunOS" else 1000and in gap it is unconditionally 100. So, perhaps 1024 won't work on some systems?
I'm not very surprised to see code like this for Solaris based on past experience with its pty driver, but I don't think anyone has built Sage on Solaris in a long time, so we can't know. I would be hesitant to add special-casing code like this based on guessing.
comment:66 Changed 5 years ago by
- Commit changed from 5b6a921419f3d4efd20d7d57b79675e861cca9fc to f10637ee3bcf88f6f8d6cda44881183a8fecf926
Branch pushed to git repo; I updated commit sha1. New commits:
f10637e | Polyhedron_base: Fix doctests to avoid vector comparisons, prep for polymake 3.1
|
comment:67 Changed 5 years ago by
- Commit changed from f10637ee3bcf88f6f8d6cda44881183a8fecf926 to 3b49a54dc04adec084b7f77f1c70328e608eb967
Branch pushed to git repo; I updated commit sha1. New commits:
8812e9c | Merge tag '7.6' into t/22658/polyhedron_methods_using_the_polymake_pexpect_interface
|
b47f6f9 | Polyhedron_base._polymake_init_: Add work around for empty polytopes with Polymake 3.0
|
3b49a54 | Polymake: Fix another test whose error message depends on whether we use files
|
comment:68 Changed 5 years ago by
I wouldn't even worry about Solaris. If someone does want this, then they should be able to provide a fix.
comment:69 Changed 5 years ago by
Also, in the interfaces, there are other similar large values, e.g.:
src/sage/interfaces/gap3.py: eval_using_file_cutoff=100, src/sage/interfaces/scilab.py: eval_using_file_cutoff=100) src/sage/interfaces/r.py: eval_using_file_cutoff=1024) src/sage/interfaces/octave.py: eval_using_file_cutoff=100) src/sage/interfaces/gap3.py: eval_using_file_cutoff=100, src/sage/interfaces/scilab.py: eval_using_file_cutoff=100) src/sage/interfaces/r.py: eval_using_file_cutoff=1024) src/sage/interfaces/octave.py: eval_using_file_cutoff=100) src/sage/interfaces/kash.py: eval_using_file_cutoff=100, src/sage/interfaces/mathematica.py: eval_using_file_cutoff=50) src/sage/interfaces/lie.py: eval_using_file_cutoff=1024) src/sage/interfaces/macaulay2.py: eval_using_file_cutoff=500) src/sage/interfaces/gp.py: eval_using_file_cutoff=1024) src/sage/interfaces/maxima.py: eval_using_file_cutoff = 256
comment:70 Changed 5 years ago by
Yes, no point worrying about these numbers. I guess some are based on historical testing, others based on cut and paste.
In any case, the current value in the polymake interface works both on Mac OS X and Linux; and there are tests for it.
comment:71 Changed 5 years ago by
So what else needs to be done for reviewing this ticket? Is anyone currently actively finishing the review for this ticket?
comment:72 Changed 5 years ago by
- Commit changed from 3b49a54dc04adec084b7f77f1c70328e608eb967 to 07a17571db553f2aa91f87184fd96492317a4d93
Branch pushed to git repo; I updated commit sha1. New commits:
07a1757 | Polyhedron_base._polymake_init_: Fix comment
|
comment:73 Changed 5 years ago by
Travis, I'm not aware of anyone else reviewing this ticket.
If you'd like to take a closer look at how I use _polymake_
and _polymake_init_
in this ticket, that could be helpful. I've in part followed Simon's advice regarding these methods and in part copied what other methods of the same classes did, but it's first time I've written interface coercion methods.
comment:74 Changed 5 years ago by
LGTM as far as I can tell. However, there are 2 minor issues from the patchbot:
- you didn't mark some tests as
# optional
insrc/sage/geometry/polyhedron/base.py
, - you have trivial doctest failures in
src/doc/en/thematic_tutorials/coercion_and_categories.rst
.
comment:75 Changed 5 years ago by
- Description modified (diff)
comment:76 Changed 5 years ago by
- Commit changed from 07a17571db553f2aa91f87184fd96492317a4d93 to 19d05f1b1053dac87c1a21c98a4f3844e0e6a7e3
comment:77 Changed 5 years ago by
- Reviewers set to Jean-Philippe Labbé, Travis Scrimshaw
I think this one is unnecessary:
sage: P = polytopes.dodecahedron(exact=False); P # optional - polymake
Once that is reverted, then you can set a positive review on my behalf.
comment:78 Changed 5 years ago by
- Commit changed from 19d05f1b1053dac87c1a21c98a4f3844e0e6a7e3 to cd583e2934e499211c8c8a6addf7dddeb36314cb
Branch pushed to git repo; I updated commit sha1. New commits:
cd583e2 | Remove one unneccessary 'optional - polymake
|
comment:80 Changed 5 years ago by
- Status changed from positive_review to needs_work
sage -t --long src/sage/interfaces/polymake.py ********************************************************************** File "src/sage/interfaces/polymake.py", line 397, in sage.interfaces.polymake.Polymake._read_in_file_command Failed example: L = polymake([42] * 400) Exception raised: Traceback (most recent call last): File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 498, in _run self.compile_and_execute(example, compiler, test.globs) File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 861, in compile_and_execute exec(compiled, globs) File "<doctest sage.interfaces.polymake.Polymake._read_in_file_command[1]>", line 1, in <module> L = polymake([Integer(42)] * Integer(400)) File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/interfaces/interface.py", line 273, in __call__ raise TypeError(msg) TypeError: unable to start polymake ********************************************************************** File "src/sage/interfaces/polymake.py", line 398, in sage.interfaces.polymake.Polymake._read_in_file_command Failed example: len(L) Exception raised: Traceback (most recent call last): File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 498, in _run self.compile_and_execute(example, compiler, test.globs) File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 861, in compile_and_execute exec(compiled, globs) File "<doctest sage.interfaces.polymake.Polymake._read_in_file_command[2]>", line 1, in <module> len(L) NameError: name 'L' is not defined ********************************************************************** File "src/sage/interfaces/polymake.py", line 403, in sage.interfaces.polymake.Polymake._read_in_file_command Failed example: L = polymake([42] * 84) Exception raised: Traceback (most recent call last): File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 498, in _run self.compile_and_execute(example, compiler, test.globs) File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 861, in compile_and_execute exec(compiled, globs) File "<doctest sage.interfaces.polymake.Polymake._read_in_file_command[3]>", line 1, in <module> L = polymake([Integer(42)] * Integer(84)) File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/interfaces/interface.py", line 273, in __call__ raise TypeError(msg) TypeError: unable to start polymake ********************************************************************** File "src/sage/interfaces/polymake.py", line 404, in sage.interfaces.polymake.Polymake._read_in_file_command Failed example: len(L) Exception raised: Traceback (most recent call last): File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 498, in _run self.compile_and_execute(example, compiler, test.globs) File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 861, in compile_and_execute exec(compiled, globs) File "<doctest sage.interfaces.polymake.Polymake._read_in_file_command[4]>", line 1, in <module> len(L) NameError: name 'L' is not defined ********************************************************************** 1 item had failures: 4 of 6 in sage.interfaces.polymake.Polymake._read_in_file_command [25 tests, 4 failures, 0.24 s]
comment:81 Changed 5 years ago by
Looks like they just need # optional - polymake
.
comment:82 Changed 5 years ago by
- Commit changed from cd583e2934e499211c8c8a6addf7dddeb36314cb to d7d4234f660f8b154a1d3b28cc9a8d0883f9a38c
comment:83 Changed 5 years ago by
- Status changed from needs_work to needs_review
comment:84 Changed 5 years ago by
- Status changed from needs_review to positive_review
comment:85 Changed 5 years ago by
- Branch changed from u/mkoeppe/polyhedron_methods_using_the_polymake_pexpect_interface to d7d4234f660f8b154a1d3b28cc9a8d0883f9a38c
- Resolution set to fixed
- Status changed from positive_review to closed
I immediately ran into a problem:
When I enter the same a second time, I get this interesting "recompilation failure"...