Opened 2 years ago
Closed 23 months ago
#27700 closed defect (fixed)
fix is_simplicial for non-full-dimensional polytopes
Reported by: | gh-LaisRast | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-8.8 |
Component: | geometry | Keywords: | polytopes, simplicial |
Cc: | gh-kliem, jipilab | Merged in: | |
Authors: | Laith Rastanawi | Reviewers: | Jean-Philippe Labbé |
Report Upstream: | N/A | Work issues: | |
Branch: | ecc2973 (Commits, GitHub, GitLab) | Commit: | ecc29734ddfde148597ca0c7709d6696f903c854 |
Dependencies: | Stopgaps: |
Description
The current is_simplicial
method returns False
if the polytope is not full-dimensional.
sage: P = polytopes.simplex() sage: P A 3-dimensional polyhedron in ZZ^4 defined as the convex hull of 4 vertices sage: P.is_simplicial() False sage: P.affine_hull().is_simplicial() True
Change History (9)
comment:1 Changed 2 years ago by
- Branch set to public/27700
- Commit set to 3087931f38c02980664efddec7261f1081b11baf
- Status changed from new to needs_review
comment:2 Changed 2 years ago by
On a similar topic:
The method is_simple
returns False
when the polyhedron is not a polytope.
I would suggest to fix that since simpleness is defined for polyhedra, or just raise NotImplementedError
like in is_simplicial
.
comment:3 follow-up: ↓ 5 Changed 2 years ago by
I guess you could just delete if not self.is_compact(): return False
in is_simple
.
But for non-pointed polyhedra we should raise an NotImplementedError
as it is really unclear what is intended in this case.
Then it just checks if every vertex is adjacent to d facets.
Add an example which shows what works correct now (the example from the description should be fine).
comment:4 Changed 2 years ago by
- Commit changed from 3087931f38c02980664efddec7261f1081b11baf to ecc29734ddfde148597ca0c7709d6696f903c854
Branch pushed to git repo; I updated commit sha1. New commits:
ecc2973 | add an example
|
comment:5 in reply to: ↑ 3 Changed 2 years ago by
In any case, the current behavior of is_simple
with polyhedra is not correct. This could be done in another ticket.
I added the simplex example in is_simplicial
.
Replying to gh-kliem:
I guess you could just delete
if not self.is_compact(): return False
inis_simple
. But for non-pointed polyhedra we should raise anNotImplementedError
as it is really unclear what is intended in this case.Then it just checks if every vertex is adjacent to d facets.
Add an example which shows what works correct now (the example from the description should be fine).
comment:6 Changed 2 years ago by
Wouldn't the implementation be simpler and faster with
M = self.incidence_matrix() for face in self.Hrepresentation(): if facet.is_inequality() and sum(M.column(face.index()) != d: return False return True
comment:7 Changed 2 years ago by
If we use the incidence matrix anyway, than we can just restrict to using it. Let n be the number of vertices. Then a polytope is simplicial if and only if
not any(d < sum(M.column(i)) < n for i in M.ncols())
I guess the current solution was intended to change as little as possible.
comment:8 Changed 23 months ago by
- Reviewers set to Jean-Philippe Labbé
- Status changed from needs_review to positive_review
Looks good to me.
comment:9 Changed 23 months ago by
- Branch changed from public/27700 to ecc29734ddfde148597ca0c7709d6696f903c854
- Resolution set to fixed
- Status changed from positive_review to closed
New commits:
fix is_simplicial for non-full-dimensional polytopes