Opened 2 years ago

Closed 2 years 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:

Status badges

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 gh-LaisRast

  • Branch set to public/27700
  • Commit set to 3087931f38c02980664efddec7261f1081b11baf
  • Status changed from new to needs_review

New commits:

3087931fix is_simplicial for non-full-dimensional polytopes

comment:2 Changed 2 years ago by gh-LaisRast

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: Changed 2 years ago by gh-kliem

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 git

  • Commit changed from 3087931f38c02980664efddec7261f1081b11baf to ecc29734ddfde148597ca0c7709d6696f903c854

Branch pushed to git repo; I updated commit sha1. New commits:

ecc2973add an example

comment:5 in reply to: ↑ 3 Changed 2 years ago by gh-LaisRast

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 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:6 Changed 2 years ago by vdelecroix

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 gh-kliem

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 2 years ago by jipilab

  • Reviewers set to Jean-Philippe Labbé
  • Status changed from needs_review to positive_review

Looks good to me.

comment:9 Changed 2 years ago by vbraun

  • Branch changed from public/27700 to ecc29734ddfde148597ca0c7709d6696f903c854
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.