Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug in incidence_matrix of CombinatorialPolyhedron #29455

Closed
LaisRast opened this issue Apr 3, 2020 · 14 comments
Closed

Bug in incidence_matrix of CombinatorialPolyhedron #29455

LaisRast opened this issue Apr 3, 2020 · 14 comments

Comments

@LaisRast
Copy link

LaisRast commented Apr 3, 2020

The incidence_matrix method of a 0-dimensional CombinatorialPolyhedron returns an error:

sage: P = Polyhedron([[0]]); P
A 0-dimensional polyhedron in ZZ^1 defined as the convex hull of 1 vertex
sage: C = P.combinatorial_polyhedron()
sage: C.incidence_matrix()
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-7-dab311ba238f> in <module>()
----> 1 C.incidence_matrix()
...
ValueError: ``output_dimension`` must be the dimension of proper faces

We fix this and make the 0-dimensional case consistent with Polyhedron_base.

Also we make incidence_matrix a chached_method.

CC: @jplab @kliem

Component: geometry

Keywords: polytope, combinatorialpolyhedron, incidence_matrix

Author: Jonathan Kliem

Branch/Commit: 34f3254

Reviewer: Laith Rastanawi

Issue created by migration from https://trac.sagemath.org/ticket/29455

@LaisRast LaisRast added this to the sage-9.1 milestone Apr 3, 2020
@kliem
Copy link
Contributor

kliem commented Apr 3, 2020

New commits:

4aa8b28fix incidence matrix for small combinatorial polyhedra

@kliem
Copy link
Contributor

kliem commented Apr 3, 2020

Author: Jonathan Kliem

@kliem

This comment has been minimized.

@kliem
Copy link
Contributor

kliem commented Apr 3, 2020

Commit: 4aa8b28

@kliem
Copy link
Contributor

kliem commented Apr 3, 2020

Branch: public/29455

@LaisRast
Copy link
Author

LaisRast commented Apr 3, 2020

comment:2

This fixes the problem.

@LaisRast
Copy link
Author

LaisRast commented Apr 3, 2020

Reviewer: Laith Rastanawi

@kliem
Copy link
Contributor

kliem commented Apr 3, 2020

comment:3

Thanks

@vbraun
Copy link
Member

vbraun commented Apr 7, 2020

comment:4

See patchbots:

sage -t --long src/sage/geometry/polyhedron/combinatorial_polyhedron/base.pyx  # 2 doctests failed

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 7, 2020

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

34f3254typos in doctests

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 7, 2020

Changed commit from 4aa8b28 to 34f3254

@kliem
Copy link
Contributor

kliem commented Apr 7, 2020

comment:6

I'm sorry. I really don't know how that could have happened. It didn't even work for me.

@LaisRast
Copy link
Author

LaisRast commented Apr 7, 2020

comment:7

Sorry, it is my mistake since I put it on "positive review".
Now the tests look fine. So I will but it again on "positive review".

@vbraun
Copy link
Member

vbraun commented Apr 12, 2020

Changed branch from public/29455 to 34f3254

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants