Opened 8 years ago
Closed 4 years ago
#15253 closed defect (fixed)
Cartesian product of polyhedra with different dimension fails
Reported by:  vbraun  Owned by:  

Priority:  major  Milestone:  sage8.0 
Component:  geometry  Keywords:  
Cc:  dimpase, jipilab, mforets, vdelecroix  Merged in:  
Authors:  Marcelo Forets  Reviewers:  Travis Scrimshaw 
Report Upstream:  N/A  Work issues:  
Branch:  a95ca7f (Commits, GitHub, GitLab)  Commit:  a95ca7f209ee78d2e873210e939e16031dd47a62 
Dependencies:  Stopgaps: 
Description
sage: polytopes.n_cube(1) * polytopes.n_cube(2) ... TypeError: no common canonical parent for objects with parents: 'Polyhedra in ZZ^1' and 'Polyhedra in ZZ^2'
probably shouldn't use @coerce_binop
on Polyhedra.product
Change History (15)
comment:1 Changed 8 years ago by
 Cc dimpase added
comment:2 Changed 7 years ago by
 Milestone changed from sage6.1 to sage6.2
comment:3 Changed 7 years ago by
 Milestone changed from sage6.2 to sage6.3
comment:4 Changed 7 years ago by
 Milestone changed from sage6.3 to sage6.4
comment:5 Changed 4 years ago by
 Cc jipilab mforets added
comment:6 Changed 4 years ago by
comment:7 Changed 4 years ago by
would it be ok if it just performs a has_coerce_map_from
for the base rings?
comment:8 Changed 4 years ago by
Hi, this is not a solution! i'm posting a commented session where i learned something new about this issue..
first, the problem:
sage: P = polytopes.hypercube(1) sage: Q = polytopes.hypercube(2) sage: P * Q ... TypeError: no common canonical parent for objects with parents: 'Polyhedra in ZZ^1' and 'Polyhedra in ZZ^2'
it arises from using the @coerce_binop
decorator at the definition of (Cartesian) product
. indeed,
sage: P.parent() Polyhedra in ZZ^1 sage: Q.parent() Polyhedra in ZZ^2 sage: P.parent().parent() <class 'sage.geometry.polyhedron.parent.Polyhedra_ZZ_ppl_with_category'> sage: Q.parent().parent() <class 'sage.geometry.polyhedron.parent.Polyhedra_ZZ_ppl_with_category'>
in consequence:
sage: P.parent() is Q.parent() # ok False sage: P.parent().parent() is Q.parent().parent() # ok True
on the other side, in def coerce_binop(method)
it uses have_same_parent
:
sage: from sage.structure.element import have_same_parent sage: have_same_parent(P, Q) # ok False sage: have_same_parent(P.parent(), Q.parent()) # why False? False
so already at the level of coerce_binop
there is this a bit strange thing one would say (?). also related is the Warning block in the definition of cpdef inline bint have_same_parent(left, right):
, which says
This function assumes that at least one of the arguments is a
Sage :class:Element
. When in doubt, use the slower
parent(left) is parent(right)
instead.
To sum up:
 perhaps one could define a custom
@coerce_binop_polyhedron
decorator which checks for grandparents, and which does not usehave_same_parent
.  alternatively, if it is indeed ok that
if have_same_parent(self, other)
gives False in this situation, then one should define a "canonical coercion" so that the methodcoercion_model.canonical_coercion(self, other)
doesn't fail. but where?
comment:10 Changed 4 years ago by
Disclaimer: I am not an expert in polytopes. I feel like this is the correct error as there is not a canonical way to embed a Z polytope into a Z^{2} polytope.
However, that is not the issue with the ticket as the product is still welldefined, but what actually should be tested is that the base rings can be made into a common parent:
P.base_ring().has_coerce_map_from(Q.base_ring())
So if that is False
, then an error should be raised. Otherwise it should change the base ring of both polytopes to this common ring and proceed.
comment:11 Changed 4 years ago by
 Branch set to u/mforets/15253
 Commit set to a95ca7f209ee78d2e873210e939e16031dd47a62
 Milestone changed from sage6.4 to sage8.0
 Status changed from new to needs_review
Thanks Travis. I'm uploading an attempt based on the previous observations.
My disclaimer is that I need this operation to substitute 1 piece of my Matlab workflow :)
New commits:
a95ca7f  change decorator to typeerror exception

comment:12 Changed 4 years ago by
 Reviewers set to Travis Scrimshaw
Positive review once you set the author field.
comment:13 Changed 4 years ago by
 Status changed from needs_review to positive_review
comment:14 Changed 4 years ago by
Thanks!
comment:15 Changed 4 years ago by
 Branch changed from u/mforets/15253 to a95ca7f209ee78d2e873210e939e16031dd47a62
 Resolution set to fixed
 Status changed from positive_review to closed
i read
coerce_binop
atsage/structure/element.pyx
, but do not understand what is the use case for Polyhedron, can someone give some hints?when you remove the @coerce_binop decorator, the example in
product
(/polyhedron/base.py) still transforms toQQ^2
when asked forP1 * P2
.