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

PolyhedronFace.affine_tangent_cone returning the polyhedron for the empty face #32658

Closed
louisng114 mannequin opened this issue Oct 9, 2021 · 16 comments
Closed

PolyhedronFace.affine_tangent_cone returning the polyhedron for the empty face #32658

louisng114 mannequin opened this issue Oct 9, 2021 · 16 comments

Comments

@louisng114
Copy link
Mannequin

louisng114 mannequin commented Oct 9, 2021

When using the empty face for #29811, it should be undefined, but instead, it returns the original polyhedron.

Example:

sage: P = polytopes.hypercube(2)
sage: P.Hrepresentation()
(An inequality (-1, 0) x + 1 >= 0,
 An inequality (0, -1) x + 1 >= 0,
 An inequality (1, 0) x + 1 >= 0,
 An inequality (0, 1) x + 1 >= 0)
sage: P.face_lattice()[0]
A -1-dimensional face of a Polyhedron in ZZ^2
sage: P.face_lattice()[0].affine_tangent_cone().Hrepresentation()
(An inequality (-1, 0) x + 1 >= 0,
 An inequality (0, -1) x + 1 >= 0,
 An inequality (1, 0) x + 1 >= 0,
 An inequality (0, 1) x + 1 >= 0)

CC: @mkoeppe @jplab @kliem @yuan-zhou

Component: geometry

Author: Louis Ng, Jonathan Kliem

Branch/Commit: 4340762

Reviewer: Matthias Koeppe

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

@louisng114 louisng114 mannequin added this to the sage-9.5 milestone Oct 9, 2021
@louisng114 louisng114 mannequin added c: geometry labels Oct 9, 2021
@louisng114

This comment has been minimized.

@kliem
Copy link
Contributor

kliem commented Oct 11, 2021

Commit: 73a4ca6

@kliem

This comment has been minimized.

@kliem
Copy link
Contributor

kliem commented Oct 11, 2021

Branch: public/32658

@kliem
Copy link
Contributor

kliem commented Oct 11, 2021

comment:3

I do not agree that it should be undefined. It should just be empty. This is also what the documentation claims and I think the definition makes sense:

    It is equal to the sum of ``self`` and the cone of feasible directions
    at any point of the relative interior of ``self``.

New commits:

73a4ca6the affine tangent cone of the empty face should be empty

@kliem
Copy link
Contributor

kliem commented Oct 11, 2021

Changed author from Louis Ng to Louis Ng, Jonathan Kliem

@mkoeppe
Copy link
Contributor

mkoeppe commented Oct 11, 2021

comment:4

But can an "affine cone" be empty? An ordinary cone cannot - it always contains 0

@mkoeppe
Copy link
Contributor

mkoeppe commented Oct 11, 2021

comment:5

Replying to @kliem:

It is equal to the sum of self and the cone of feasible directions
at any point of the relative interior of self.

I would say that this sentence does not define anything for the empty case, because "the" cone at "any" point of the empty set is undefined.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 11, 2021

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

4340762throw an error, because it is not a cone

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 11, 2021

Changed commit from 73a4ca6 to 4340762

@kliem

This comment has been minimized.

@kliem
Copy link
Contributor

kliem commented Oct 11, 2021

New commits:

4340762throw an error, because it is not a cone

@kliem
Copy link
Contributor

kliem commented Oct 11, 2021

comment:8

Replying to @mkoeppe:

Replying to @kliem:

It is equal to the sum of self and the cone of feasible directions
at any point of the relative interior of self.

I would say that this sentence does not define anything for the empty case, because "the" cone at "any" point of the empty set is undefined.

Agreed.

@mkoeppe
Copy link
Contributor

mkoeppe commented Oct 11, 2021

Reviewer: Matthias Koeppe

@kliem
Copy link
Contributor

kliem commented Oct 11, 2021

comment:10

Thank you.

@vbraun
Copy link
Member

vbraun commented Oct 19, 2021

Changed branch from public/32658 to 4340762

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