Opened 4 months ago

Closed 4 months ago

#30292 closed defect (fixed)

is_pyramid returns a wrong certificate

Reported by: gh-LaisRast Owned by:
Priority: critical Milestone: sage-9.2
Component: geometry Keywords: polytope, combinatorialpolyhedron, is_pyramid
Cc: jipilab, gh-kliem Merged in:
Authors: Jonathan Kliem Reviewers: Matthias Koeppe
Report Upstream: N/A Work issues:
Branch: 4c5c730 (Commits) Commit: 4c5c730e216d0d160408d2474adcc51b33d28075
Dependencies: Stopgaps:

Description

The method is_pyramid of CombinatorialPolyhedron returns a wrong certificate: 

sage: Polyhedron([[0, -1, -1], [0, -1, 1], [0, 1, -1], [0, 1, 1], [1, 0, 0]]).is_pyramid(certificate=True)
(True, A vertex at (0, -1, -1))

This bug introduced in #29189; the indexing in _face_iter(True, 0) is different from the indexing in Vrepresentation.

Change History (8)

comment:1 Changed 4 months ago by gh-kliem

Thanks for catching this. Stupid mistake.

comment:2 Changed 4 months ago by gh-kliem

  • Authors set to Jonathan Kliem
  • Branch set to public/30292
  • Commit set to fb22b485e74152bc740eeb577eb1f90413eca342
  • Status changed from new to needs_review

New commits:

​fb22b48fix certificate for pyramid

comment:3 Changed 4 months ago by mkoeppe

Perhaps there should be a _test... method that checks the certificate?

comment:4 Changed 4 months ago by git

  • Commit changed from fb22b485e74152bc740eeb577eb1f90413eca342 to 4c5c730e216d0d160408d2474adcc51b33d28075

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

​4c5c730add method `_test_pyramid`

comment:5 Changed 4 months ago by mkoeppe

  • Reviewers set to Matthias Koeppe
  • Status changed from needs_review to positive_review

comment:6 Changed 4 months ago by mkoeppe

  • Priority changed from major to critical

comment:7 Changed 4 months ago by gh-kliem

Thank you.

comment:8 Changed 4 months ago by vbraun

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