sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.32k stars 453 forks source link

Triangulation.polyhedral_complex, boundary_simplicial_complex, boundary_polyhedral_complex #33586

Closed mkoeppe closed 2 years ago

mkoeppe commented 2 years ago

polyhedral_complex creates a geometric polyhedral complex corresponding to the triangulation.

boundary_simplicial_complex and boundary_polyhedral_complex are combinations of boundary (which gives a set of simplices) with simplicial_complex (which gives an abstract simplicial complex) and polyhedral_complex, respectively

CC: @yuan-zhou @jhpalmieri

Component: geometry

Author: Matthias Koeppe

Branch/Commit: 65134f1

Reviewer: Yuan Zhou, John Palmieri

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

mkoeppe commented 2 years ago

Author: Matthias Koeppe

mkoeppe commented 2 years ago

Description changed:

--- 
+++ 
@@ -1,2 +1,4 @@
 This new method creates a geometric polyhedral complex corresponding to the triangulation.

+Also we change the constructor of `PolyhedralComplex` so that it accepts a `Triangulation` as input.
+
mkoeppe commented 2 years ago

Branch: u/mkoeppe/triangulation_polyhedral_complex

mkoeppe commented 2 years ago

New commits:

65b7cfeTriangulation.polyhedral_complex: New
mkoeppe commented 2 years ago

Description changed:

--- 
+++ 
@@ -1,4 +1,2 @@
 This new method creates a geometric polyhedral complex corresponding to the triangulation.

-Also we change the constructor of `PolyhedralComplex` so that it accepts a `Triangulation` as input.
-
mkoeppe commented 2 years ago

Commit: 65b7cfe

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

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

fff67c3Triangulation.boundary_{simplicial,polyhedral}_complex: New
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

Changed commit from 65b7cfe to fff67c3

mkoeppe commented 2 years ago

Description changed:

--- 
+++ 
@@ -1,2 +1,4 @@
-This new method creates a geometric polyhedral complex corresponding to the triangulation.
+`polyhedral_complex` creates a geometric polyhedral complex corresponding to the triangulation.

+`boundary_simplicial_complex` and `boundary_polyhedral_complex` are combinations of `boundary` (which gives a set of simplices) with `simplicial_complex` (which gives an abstract simplicial complex) and `polyhedral_complex`, respectively
+
jhpalmieri commented 2 years ago
comment:8

The docstring for boundary_simplicial_complex ends with

The boundary of every convex set is a topological sphere::
mkoeppe commented 2 years ago
comment:9

Thanks! I'll add the missing example

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

Changed commit from fff67c3 to 0a09ca4

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

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

0a09ca4src/sage/geometry/triangulation/element.py: Add missing example
yuan-zhou commented 2 years ago
comment:12

Would it be possible to illustrate that boundary_simplicial_complex is a sub complex of simplicial_complex?

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

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

0845f03Triangulation.boundary_simplicial_complex: Expand example
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

Changed commit from 0a09ca4 to 0845f03

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

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

80165baTriangulation.boundary_polyhedral_complex: Expand example
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

Changed commit from 0845f03 to 80165ba

mkoeppe commented 2 years ago
comment:15

Replying to @yuan-zhou:

Would it be possible to illustrate that boundary_simplicial_complex is a sub complex of simplicial_complex?

Good idea, done

mkoeppe commented 2 years ago
comment:16

It looks like abstract simplicial complexes don't have an is_subcomplex method. Should we add one?

mkoeppe commented 2 years ago
comment:17

Cubical complexes also define is_subcomplex.

yuan-zhou commented 2 years ago
comment:18

Defining is_subcomplex for abstract simplicial complexes is probably beyond the scope of the current ticket. I think that polyhedral complex has the method is_subcomplex, so it can at least be added to the doctests of polyhedral_complex or boundary_polyhedral_complex. Replying to @mkoeppe:

It looks like abstract simplicial complexes don't have an is_subcomplex method. Should we add one?

mkoeppe commented 2 years ago
comment:19

Yes, I'm already using it

mkoeppe commented 2 years ago
comment:20

Replying to @yuan-zhou:

Defining is_subcomplex for abstract simplicial complexes is probably beyond the scope of the current ticket.

OK, I've opened #34294 for it

yuan-zhou commented 2 years ago
comment:21

In the docstring of Triangulation.__init__, it says "In the second case, the point indices of the maximal simplices of the triangulation", which needs rephrasing.

yuan-zhou commented 2 years ago
comment:22

It would be nice to point out that the polyhedral complex and boundary polyhedral complex are actually simplicial complexes. The difference is that they are not abstract.

yuan-zhou commented 2 years ago
comment:23

Never mind. Replying to @yuan-zhou:

It would be nice to point out that the polyhedral complex and boundary polyhedral complex are actually simplicial complexes. The difference is that they are not abstract.

yuan-zhou commented 2 years ago
comment:24

Does it make sense to compare Triangulation.boundary_polyhedral_complex and PolyhedralComplex.boundary_subcomplex?

mkoeppe commented 2 years ago
comment:25

Replying to @yuan-zhou:

Does it make sense to compare Triangulation.boundary_polyhedral_complex and PolyhedralComplex.boundary_subcomplex?

I decided to stay away from that for this ticket because there's a bit of confusion regarding "boundary" vs "relative boundary"

yuan-zhou commented 2 years ago
comment:26

The code on this ticket (other than comment:21) looks good to me.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

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

65134f1src/sage/geometry/triangulation/element.py: Docstring improvements
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

Changed commit from 80165ba to 65134f1

mkoeppe commented 2 years ago
comment:28

I've rephrased it, please take a look

yuan-zhou commented 2 years ago

Reviewer: Yuan Zhou, John Palmieri

mkoeppe commented 2 years ago
comment:30

Thanks!

vbraun commented 2 years ago

Changed branch from u/mkoeppe/triangulation_polyhedral_complex to 65134f1