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: | sage-8.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 sage-6.1 to sage-6.2
comment:3 Changed 7 years ago by
- Milestone changed from sage-6.2 to sage-6.3
comment:4 Changed 7 years ago by
- Milestone changed from sage-6.3 to sage-6.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 Z2 polytope.
However, that is not the issue with the ticket as the product is still well-defined, 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 sage-6.4 to sage-8.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
.