Make "sage_input" for polyhedron objects output the backend used

[jipilab]

Currently, the sage_input function of a Polyhedron object does not provide the backend used.

sage: P1 = Polyhedron(vertices = [[1, 0], [0, 1]], rays = [[1, 1]], backend='normaliz')
sage: sage_input(P1)
Polyhedron(base_ring=ZZ, rays=[(1, 1)], vertices=[(0, 1), (1, 0)])
sage: P2 = Polyhedron(vertices = [[1, 0], [0, 1]], rays = [[1, 1]], backend='ppl')
sage: sage_input(P2)
Polyhedron(base_ring=ZZ, rays=[(1, 1)], vertices=[(0, 1), (1, 0)])

It should be able to give it.

[moritz]

I agree that we need to have this feature! Perhaps it would make sense to implement a function get_backend (like we have in MixedIntegerLinearProgram), that returns the backend first? (Or perhaps simply call this function backend?

So I guess it would be cleanest to store the backend information somehow in P1._backend. Should this be done directly in the init of class Polyhedra_base in parent.py, similar to P1._ambient_dim? Or should this rather be done inside each backend class? Or yet elsewhere?

What do you think?

[jipilab]

Replying to moritz:

I agree that we need to have this feature! Perhaps it would make sense to implement a function get_backend (like we have in MixedIntegerLinearProgram), that returns the backend first? (Or perhaps simply call this function backend?

+1 for getting a function backend. (I would choose backend to make it similar to base_ring and further starting with get_ is annoying when using tab completion. Just to be sure I would look at the naming conventions of Sage, I guess backend is better.)

So I guess it would be cleanest to store the backend information somehow in P1._backend. Should this be done directly in the init of class Polyhedra_base in parent.py, similar to P1._ambient_dim? Or should this rather be done inside each backend class? Or yet elsewhere?

One way to go, since each (backend/base_ring) combination creates a different parent in parent.py would be to create an init function for each of these parents that executes the init of the Polyhedra_base and adds an attribute _backend on top.

Because doing it in Polyhedra_base seems to be messy (how to know the backend information at this stage?).

[moritz]

I looked at it again and now think its best to put it in parent.py. There a treatment that is analogous to ambient_dim and base_ring is possible. Please have a look at the patch!

Once the backend-function is approved it will be easy to enhance the sage_input-function as requested in the description of the ticket.

---

New commits:

​e1e56a7

introduce backend parameter, variable and function

[jipilab]

The backend method looks good. I guess you can proceed with the sage_input part.

Thanks!

[git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

​afa5286	introduce backend parameter, variable and function
​e7f81ec	backend function in base.py
​739af0c	added 'backend' to 'sage_input'

[moritz]

I have not (yet) added doctests polymake and (py)normaliz; do you think this is needed?

[jipilab]

Replying to moritz:

I have not (yet) added doctests polymake and (py)normaliz; do you think this is needed?

Yes, that would be nice! Especially since they are backends and it's about backends...

[git]

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

​3bd35ca

added normaliz and polymake doctests

[moritz]

I have not tested the polymake doctest, since I don't have polymake installed at the moment. Let's see what the bot says..

[jipilab]

I have not tested the polymake doctest, since I don't have polymake installed at the moment. Let's see what the bot says..

All right, I'll have a look!

[jipilab]

For now, I get:

sage -t base.py
**********************************************************************
File "base.py", line 171, in sage.geometry.polyhedron.base.Polyhedron_base._sage_input_
Failed example:
    sage_input(P)                                                                    # optional - polymake
Expected:
    Polyhedron(backend='polymake', base_ring=ZZ, rays=[(1, 1)], vertices=[(0, 1), (1, 0)])
Got:
    Polyhedron(backend='polymake', base_ring=QQ, rays=[(QQ(1), QQ(1))], vertices=[(QQ(1), QQ(0)), (QQ(0), QQ(1))])
**********************************************************************

If you don't want to see the QQ(.), there's the option preparse=False in sage_input.

[jipilab]

Hi Moritz,

All tests pass on beta3. It seems to fix the issue with the backend. I put a TODO in the doctest to add handling of the preparse option of sage_input eventually to get rid of the QQ in the output.

Look at the change and if you are happy, you can set it as positive review.

---

New commits:

​dd3a121

Fixed a failing polymake test

[moritz]

thanks for the review, JP!

[vbraun]

See the patchbot result

[jipilab]

It seems that the creation of the associahedron is broken by this code (!?)

[chapoton]

Maybe the backend must be specified in

        associahedron = super(Associahedra, self)._element_constructor_(None, [inequalities, []])

near the end of src/sage/combinat/root_system/associahedron.py

[jipilab]

Maybe the backend must be specified in

        associahedron = super(Associahedra, self)._element_constructor_(None, [inequalities, []])

near the end of src/sage/combinat/root_system/associahedron.py

That does not seem to be enough. I'm puzzled by how to fix this. Now the __init__ of the polyhedron base class requires 4 arguments. But how to pass this new argument when having a subparent class Associahedra.

I changed also line 33 like this:

33     parent = Associahedra(QQ, cartan_type.rank(),'ppl')

to get the correct backend. But I still get similar errors. Any cues?

[chapoton]

Here is a tentative that works.. Maybe Travis would find a better way ?

---

New commits:

​951735b

trac 22565 fixing associahedra (sort of)

[jipilab]

Oh, I see! But isn't this a bit weird considering the fact that the class Associahedra inherits from Polyhedra_QQ_ppl?

[mkoeppe]

I think it is a bad idea in the first place to have a class such as Associahedron_class inherit from a particular polyhedron backend class.

[jipilab]

The bot seems to like the hack.

Perhaps as a follow-up, it would be good to change the inheritance of the Associahedron_class to Polyhedra ? What do you think?

Otherwise, as of now, I would put this ticket as positive review.

[chapoton]

TODO:: should be .. TODO:: Otherwise, looks good to me

[git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

​79af7fc	introduce backend parameter, variable and function
​a8de4a7	backend function in base.py
​deebcf7	added 'backend' to 'sage_input'
​c10357f	added normaliz and polymake doctests
​435525a	Fixed a failing polymake test
​04eda4d	trac 22565 fixing associahedra (sort of)
​28e2b71	rebase and '.. TODO'

[jipilab]

This looks good to go. The last bot gave a green light.