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

Fix the dimension of PolyhedronFace #28650

Closed
LaisRast opened this issue Oct 24, 2019 · 14 comments
Closed

Fix the dimension of PolyhedronFace #28650

LaisRast opened this issue Oct 24, 2019 · 14 comments

Comments

@LaisRast
Copy link

In this ticket, I am going to fix the implementation of the method dim of PolyhedronFace. This problem occurs when we have a vertex and a ray (or a line) with the same vector.
See the following example.

sage: P = Polyhedron(vertices=[[1,0]], rays=[[1,0],[0,1]]); P
A 2-dimensional polyhedron in QQ^2 defined as the convex hull of 1 vertex and 2 rays
sage: P.faces(P.dim())[0]
A 1-dimensional face of a Polyhedron in QQ^2 defined as the convex hull of 1 vertex and 2 rays

CC: @jplab @kliem

Component: geometry

Keywords: polytopes, polyhedronface, dimension

Author: Laith Rastanawi

Branch/Commit: 67d3ce0

Reviewer: Jonathan Kliem

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

@LaisRast LaisRast added this to the sage-9.0 milestone Oct 24, 2019
@LaisRast
Copy link
Author

Commit: 4e5912d

@LaisRast
Copy link
Author

New commits:

4e5912dfix PolyhedronFace.dim()

@LaisRast
Copy link
Author

Branch: public/28650

@LaisRast
Copy link
Author

Author: Laith Rastanawi

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 25, 2019

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

ae40381code within 79 columns

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 25, 2019

Changed commit from 4e5912d to ae40381

@kliem
Copy link
Contributor

kliem commented Oct 25, 2019

comment:3

Missing a doctest, which shows that the bug is fixed.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 29, 2019

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

632a2d9add a test in the docstring

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 29, 2019

Changed commit from ae40381 to 632a2d9

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 30, 2019

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

67d3ce0fix doctest syntax

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 30, 2019

Changed commit from 632a2d9 to 67d3ce0

@kliem
Copy link
Contributor

kliem commented Oct 30, 2019

comment:7

Looks good to me.

Now only waiting for that one bot, but I don't except anything to happen.

@kliem
Copy link
Contributor

kliem commented Oct 30, 2019

Reviewer: Jonathan Kliem

@vbraun
Copy link
Member

vbraun commented Oct 31, 2019

Changed branch from public/28650 to 67d3ce0

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