Opened 5 years ago
Last modified 3 years ago
#25183 closed defect
Bug in the associahedron object — at Version 7
Reported by:  JeanPhilippe Labbé  Owned by:  

Priority:  major  Milestone:  sage9.0 
Component:  geometry  Keywords:  associahedron 
Cc:  Frédéric Chapoton, Christian Stump, Travis Scrimshaw, Moritz Firsching, JeanPhilippe 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: 
Description (last modified by )
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/sitepackages/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 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
Cc:  Moritz Firsching added 

comment:2 Changed 4 years ago by
comment:3 Changed 4 years ago by
Authors:  → Jonathan Kliem 

Branch:  → public/25183 
Commit:  → 65b11c9de95bb2ecde90c722ca2e9781fdac7cc5 
Dependencies:  → 27798 
Description:  modified (diff) 
Status:  new → needs_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:
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

comment:4 Changed 4 years ago by
Dependencies:  27798 → #27798 

comment:5 Changed 3 years ago by
Milestone:  sage8.2 → sage8.9 

comment:6 Changed 3 years ago by
Status:  needs_review → needs_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
Description:  modified (diff) 

I think I took care of it in #27798.