Bug in the associahedron object

[jipilab]

The following lines currently occur:

sage: A = polytopes.associahedron(['A',3])
sage: face = A.faces(2)[3]
sage: face
<6,8,9,10>
sage: face.as_polyhedron()
/home/jplabbe/sage/local/lib/python2.7/site-packages/sage/repl/rich_output/display_manager.py:590: RichReprWarning: Exception in _rich_repr_ while displaying object: 'Associahedra_with_category.element_class' object has no attribute '_cartan_type'
  RichReprWarning,
<repr(<sage.combinat.root_system.associahedron.Associahedra_with_category.element_class at 0x7f48d0d7ad70>) failed: AttributeError: 'Associahedra_with_category.element_class' object has no attribute '_cartan_type'>

The construction as_polyhedron initializes the new (face-)polyhedron via P.__class__(parent, Vrep, None). In the case of the associahedron, this does not give a valid object, as the associahedron requires a cartan type as well.

Same (or similar problem) with:

• minkowski_sum,
• minkowski_difference,
• translation,
• dilation,
• convex_hull,
• intersection (actually even worse as coercion fails),
• polar.

We fix the initialization of the associahedron to now require the cartan type on __init__ (before it was assumed to be set after initialization).

Further we have __new__ return the correct parent class such that e.g. the face of an Associahedron_class_ppl is constructed as Polyhedron_ppl.

We fix _coerce_map_from_ to take into account that no general polyhedron can be coerced to an associahedron.

We manually set the correct pushout of polyhedra over ZZ and associahedra (over QQ) to be polyhedra over QQ.

[gh-kliem]

I think I took care of it in #27798.

[gh-kliem]

This branch should fix the bug.

Feel free to change it, alter it, abonden it, etc.

Maybe there is a better way to obtain the correct parent, i.e. improve the following lines:

156             for typ1 in ancestors_of_associahedron:
157                 if typ1 in mro:
158                     return typ1(parent, Vrep, Hrep, **kwds)

---

New commits:

​2643078	added `backend` to associahedron and flow polytope
​a9b4826	typo
​8716985	corrected docstring
​b873156	associahedron actually uses the claimed backend now
​1f4bb25	comments by tscrim
​2c812c5	should have fixed bug in #25183
​65b11c9	removed redundant line

[gh-kliem]

Once #27798 is done, I'm going to rebase it and solve the merge conflict. At the moment it seems pointless to me.

[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
​e137199	changed default value of `backend`; small changes in documentation
​0e892e7	modify construction and coercion of associahedra to fix bug #25183

[gh-kliem]

I think I got things fixed now. Suggestions for better solutions are welcome.

[jipilab]

That looks good to me. Might be good to have some feedback from Travis or Frédéric on the cartan side of things, if they are done properly.

I would put this to positive review since the error appears to be fixed.

[tscrim]

I don't have any comments on the Cartan side as you are not really doing anything with them. However, I do have one comment about this code block:

            for typ1 in ancestors_of_associahedron:
                if typ1 in mro:
                    return typ1(parent, Vrep, Hrep, **kwds)

My understanding is there is no subclass relations among the classes in ancestors_of_associahedron. So I think it is faster to make that a set object and test

            for typ1 in mro:
                if typ1 in ancestors_of_associahedron:
                    return typ1(parent, Vrep, Hrep, **kwds)

as set containment check is much faster and these classes are likely to be higher in the MRO. It also has a side effect of making all of these classes run in the same time. For example, imagine if you are looking for a Polyhedron_polymake, that means it has to check the through the entire MRO 4 times (and a bit more to actually find it). So this could become really expensive if you create lots of (temporary) associahedra.

[gh-kliem]

New commits:

​cc20d20	modify construction and coercion of associahedra to fix bug #25183
​d9083b7	small change of order of iteration to be more efficient

[gh-kliem]

Changed to order of iteration, such that it only runs once through mro now.

[tscrim]

Thanks. LGTM.