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

Dual for combinatorial polyhedron #29190

Closed
kliem opened this issue Feb 13, 2020 · 17 comments
Closed

Dual for combinatorial polyhedron #29190

kliem opened this issue Feb 13, 2020 · 17 comments

Comments

@kliem
Copy link
Contributor

kliem commented Feb 13, 2020

We implement a method of CombinatorialPolyhedron that obtains the polar/dual. We name it dual and create an alias polar to be consistent with Polyhedron_base.

This is obtained by simply copying and interchanging the bitrepresentation of facets and vertices. The object is purely combinatorial and does not have vertex or facet names. A ValueError is raised for the unbounded case.

Along the way we implement a __copy__ method for ListOfFaces and allow initializing CombinatorialPolyhedron from a tuple consisting of two ListOfFaces.

CC: @jplab @LaisRast

Component: geometry

Keywords: polar, dual, combinatorial polyhedron

Author: Jonathan Kliem

Branch/Commit: 73cf39f

Reviewer: Laith Rastanawi

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

@kliem kliem added this to the sage-9.1 milestone Feb 13, 2020
@kliem
Copy link
Contributor Author

kliem commented Feb 13, 2020

Branch: public/29190

@kliem
Copy link
Contributor Author

kliem commented Feb 13, 2020

New commits:

bc51430copy for ListOfFaces
5078b6eadd polar for combinatorial polyhedron

@kliem
Copy link
Contributor Author

kliem commented Feb 13, 2020

Commit: 5078b6e

@LaisRast
Copy link

comment:2
-       * or a tuple consting of facets and vertices as two
+       * or a tuple consisting of facets and vertices as two

@kliem
Copy link
Contributor Author

kliem commented Feb 14, 2020

Dependencies: #28608

@kliem
Copy link
Contributor Author

kliem commented Feb 14, 2020

comment:4

I'm just going to wait until #28608 is merged, otherwise it's a pain to review.

@kliem
Copy link
Contributor Author

kliem commented Feb 24, 2020

Changed commit from 5078b6e to 8beef7c

@kliem
Copy link
Contributor Author

kliem commented Feb 24, 2020

Changed branch from public/29190 to public/29190-reb

@kliem
Copy link
Contributor Author

kliem commented Feb 24, 2020

New commits:

328c16dcopy for ListOfFaces
8beef7cadd polar for combinatorial polyhedron

@kliem
Copy link
Contributor Author

kliem commented Feb 24, 2020

Changed dependencies from #28608 to none

@LaisRast
Copy link

comment:6
  • I would suggest renaming the method name to dual (since this is purely combinatorial) and put polar as an alias to dual to be consistent with Polyhedron_base.

  • Add

        .. SEEALSO::

            :meth:`~sage.geometry.polyhedron.base.Polyhedron_base.polar`.

Otherwise, the ticket is good to go.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 27, 2020

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

23b87edcopy for ListOfFaces
309150badd polar for combinatorial polyhedron
586bc82renamed polar to dual
73cf39ffixing doctest

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 27, 2020

Changed commit from 8beef7c to 73cf39f

@kliem

This comment has been minimized.

@kliem
Copy link
Contributor Author

kliem commented Mar 27, 2020

Reviewer: Laith Rastanawi

@kliem kliem changed the title Polar for combinatorial polyhedron Dual for combinatorial polyhedron Mar 27, 2020
@LaisRast
Copy link

comment:9

I think it is good to go.

@vbraun
Copy link
Member

vbraun commented Apr 5, 2020

Changed branch from public/29190-reb to 73cf39f

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