Opened 5 years ago

Last modified 3 years ago

#25183 closed defect

Bug in the associahedron object — at Version 7

Reported by: Jean-Philippe Labbé Owned by:
Priority: major Milestone: sage-9.0
Component: geometry Keywords: associahedron
Cc: Frédéric Chapoton, Christian Stump, Travis Scrimshaw, Moritz Firsching, Jean-Philippe Labbé, Laith Rastanawi Merged in:
Authors: Jonathan Kliem Reviewers:
Report Upstream: N/A Work issues:
Branch: public/25183 (Commits, GitHub, GitLab) Commit: 65b11c9de95bb2ecde90c722ca2e9781fdac7cc5
Dependencies: #27798 Stopgaps:

Status badges

Description (last modified by gh-kliem)

The following lines currently occur:

sage: A = polytopes.associahedron(['A',3])
sage: face = A.faces(2)[3]
sage: face
sage: face.as_polyhedron()
/home/jplabbe/sage/local/lib/python2.7/site-packages/sage/repl/rich_output/ RichReprWarning: Exception in _rich_repr_ while displaying object: 'Associahedra_with_category.element_class' object has no attribute '_cartan_type'
<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 as 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.

Change History (7)

comment:1 Changed 5 years ago by Moritz Firsching

Cc: Moritz Firsching added

comment:2 Changed 4 years ago by gh-kliem

I think I took care of it in #27798.

comment:3 Changed 4 years ago by gh-kliem

Authors: Jonathan Kliem
Branch: public/25183
Commit: 65b11c9de95bb2ecde90c722ca2e9781fdac7cc5
Dependencies: 27798
Description: modified (diff)
Status: newneeds_review

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:

2643078added `backend` to associahedron and flow polytope
8716985corrected docstring
b873156associahedron actually uses the claimed backend now
1f4bb25comments by tscrim
2c812c5should have fixed bug in #25183
65b11c9removed redundant line

comment:4 Changed 4 years ago by gh-kliem

Dependencies: 27798#27798

comment:5 Changed 3 years ago by gh-kliem

Milestone: sage-8.2sage-8.9

comment:6 Changed 3 years ago by gh-kliem

Status: needs_reviewneeds_work

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

comment:7 Changed 3 years ago by gh-kliem

Description: modified (diff)
Note: See TracTickets for help on using tickets.