Add backend option to associahedron and flow polytope

[kliem]

The option backend was missing for polytopes.associahedron and polytopes.flow_polytope.

CC: @jplab

Component: geometry

Keywords: polytopes, associahedron, flow polytopes, backend, normaliz

Author: Jonathan Kliem

Branch/Commit: e137199

Reviewer: Jean-Philippe Labbé, Travis Scrimshaw

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

[kliem]

New commits:

2643078

added `backend` to associahedron and flow polytope

[kliem]

Branch: public/27798

[kliem]

Commit: 2643078

[sagetrac-git]

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

a9b4826

typo

[sagetrac-git]

Changed commit from 2643078 to a9b4826

[seblabbe]

comment:3

The doc should include in the description of backend input of both functions:

If ``None`` then ``backend='ppl'`` is used.

[sagetrac-git]

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

8716985

corrected docstring

[sagetrac-git]

Changed commit from a9b4826 to 8716985

[kliem]

Author: Jonathan Kliem

[kliem]

Changed keywords from none to polytopes, associahedron, flow polytopes, backend, normaliz

[kliem]

comment:6

Replying to @seblabbe:

The doc should include in the description of backend input of both functions:

If ``None`` then ``backend='ppl'`` is used.

Thanks for remarking that.
Is the current even a good way of doing it?
Maybe I should just have 'ppl' instead of None (but this somehow looks more written in stone, does it)?
In this ticket I didn't want any changes to the default option.

[kliem]

comment:7

Actually, I don't think the construction with associahedron works. The ._backend string is correct, but the object does not have e.g. normaliz functionality.

[sagetrac-git]

Changed commit from 8716985 to b873156

[sagetrac-git]

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

b873156

associahedron actually uses the claimed backend now

[kliem]

comment:9

Seems to work now. Flow polytope anyway and also the associahedron. E.g. I can construct an associahedron with backend='normaliz' and access (and use) the underlying normaliz cone.

[tscrim]

comment:10

Three little comments from a quick look:

Your INPUT: block is over-indented.

It is better to have new-style classes:

-class Associahedron_class_base():
+class Associahedron_class_base(object):

-class Associahedra_base:
+class Associahedra_base(object):

I do not see why the _poly_super is needed. You could just look at the MRO or have the Polyhedra_* class be a class-level attribute (similar to Element). Are you also sure that this doesn't work:

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

as my understanding is that it goes to the next class up in the MRO after Associahedra_base, which should be the Polyhedra_* class.

[kliem]

comment:11

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

(with self and Associahedra_base interchanged) lead to

TypeError: super() argument 1 must be type, not classobj

However, this seems to be solved by inheriting from object.

Replying to @tscrim:

Three little comments from a quick look:

Your INPUT: block is over-indented.

It is better to have new-style classes:

-class Associahedron_class_base():
+class Associahedron_class_base(object):

-class Associahedra_base:
+class Associahedra_base(object):

I do not see why the _poly_super is needed. You could just look at the MRO or have the Polyhedra_* class be a class-level attribute (similar to Element). Are you also sure that this doesn't work:

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

as my understanding is that it goes to the next class up in the MRO after Associahedra_base, which should be the Polyhedra_* class.

[sagetrac-git]

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

1f4bb25

comments by tscrim

[sagetrac-git]

Changed commit from b873156 to 1f4bb25

[jplab]

comment:13

You may want to have a look at #25183 while you are playing around with the associahedron.

[sagetrac-git]

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

d9a7e6b

periods to ends of sentences

[kliem]

comment:31

Replying to @sagetrac-git:

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

d9a7e6b

periods to ends of sentences

Took care of the periods.

[fchapoton]

comment:32

red branch, meaning that it needs to be rebased on the latest beta

[sagetrac-git]

Changed commit from d9a7e6b to 6cffe8d

[sagetrac-git]

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

6cffe8d

added `backend` to associahedron and flow polytope

[tscrim]

comment:35

It seems like you have some extra stuff in digraph that has snuck in with the all_paths_iterator.

[sagetrac-git]

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

949b0a9

added `backend` to associahedron and flow polytope

[sagetrac-git]

Changed commit from 6cffe8d to 949b0a9

[kliem]

comment:37

Replying to @tscrim:

It seems like you have some extra stuff in digraph that has snuck in with the all_paths_iterator.

Done.

[tscrim]

comment:38

Okay, I am pretty sure you now break Associahedra when the backend is not specified. I also do not see why you need this in Associahedron:

    if backend is None:
        backend = 'ppl'

Just set backend='ppl' as the default argument instead of None.
You should also update the documentation there too:

    - ``backend`` -- string (``'ppl'``); the backend to use;
      see :meth:`sage.geometry.polyhedron.constructor.Polyhedron`

Also, in flow_polytope:

        - ``backend`` -- string or ``None`` (default); the backend to use;
          see :meth:`sage.geometry.polyhedron.constructor.Polyhedron`

(By convention in Sage, the INPUT: block points are not considered to be full sentences, so no period/full-stop.)

[tscrim]

Reviewer: Travis Scrimshaw

[sagetrac-git]

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

e137199

changed default value of `backend`; small changes in documentation

[sagetrac-git]

Changed commit from 949b0a9 to e137199

[kliem]

comment:40

Replying to @tscrim:

You should also update the documentation there too:

    - ``backend`` -- string (``'ppl'``); the backend to use;
      see :meth:`sage.geometry.polyhedron.constructor.Polyhedron`

Also, in flow_polytope:

        - ``backend`` -- string or ``None`` (default); the backend to use;
          see :meth:`sage.geometry.polyhedron.constructor.Polyhedron`

(By convention in Sage, the INPUT: block points are not considered to be full sentences, so no period/full-stop.)

We had the discussion before. I guess its fine now, as we got rid of the complete sentences.

Btw, in many (if not all) polytopes in the library the INPUT: block is a complete sentence with period/full-stop.

[tscrim]

comment:42

Thank you. Yea, well, there have been some fairly inconsistent things done with the documentation over the years. :/ I am at least trying to move towards what the standard in the dev guide

[tscrim]

Changed reviewer from Travis Scrimshaw to Jean-Philippe Labbé, Travis Scrimshaw

[vbraun]

Changed branch from public/27798 to e137199