Implement star and stellar subdivision of a face of simplicial complex

[jplab]

This ticket provides the star and the stellar subdivision of a face of a simplicial complex.

The star of a face is the union of the faces that contains that face. The star of the empty face is the whole complex.

Given a simplicial complex C, the stellar subdivision of a face f of C is a new simplicial complex obtained as the barycentric subdivision of the face with respect to its star.

CC: @mo271 @videlec

Component: algebraic topology

Keywords: days84, simplicial complex, star, stellar subdivision

Author: Jean-Philippe Labbé

Branch/Commit: 1b54fba

Reviewer: Thierry Monteil, Frédéric Chapoton

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

[jplab]

Branch: u/jipilab/22466

[jplab]

Description changed:

--- 
+++ 
@@ -1,6 +1,4 @@
-This ticket provides the deletion, the star and the stellar subdivision of a face of a simplicial complex.
-
-For the deletion, there are different definitions which give different results. The difference lies in removing **or not** the vertices of the face. This should be an optional parameter.
+This ticket provides the star and the stellar subdivision of a face of a simplicial complex.

 The star of a face is the union of the faces that contains that face. The star of the empty face is the whole complex.

[jplab]

Commit: 2dfd035

[jplab]

New commits:

2dfd035

First implementation of star

[jplab]

Changed keywords from days84, simplicial complex, star, stellar subdivision, deletion to days84, simplicial complex, star, stellar subdivision

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

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

04bcbef

Added first implementation of stellar subdivision

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 2dfd035 to 04bcbef

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 04bcbef to e948253

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

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

c6b3853	Corrected indentation
e948253	pep8 conventions

[jplab]

Author: Jean-Philippe Labbé

[edd8e884-f507-429a-b577-5d554626c0fe]

Reviewer: Thierry Monteil

[edd8e884-f507-429a-b577-5d554626c0fe]

comment:7

First few comments/questions:

• would it be pertinent to check that the provided simplex is actually a simplex in the considered simplicial complex ?
• You could provide non-working examples/tests to check such inconsistent user inputs.
• in stellar_subdivision, the mutable option is not taken into account (does not appear in the code).
• what is the difference between is_mutable and mutable option ?
• you should add an INPUT section in stellar_subdivision.

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

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

fae5016

Review and made tests pass

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from e948253 to fae5016

[fchapoton]

comment:9

missing empty line here:

+        EXAMPLES::
+            sage: SC = SimplicialComplex([[0,1,2],[1,2,3]])

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from fae5016 to eb4dfac

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

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

eb4dfac

pep8 conventions

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from eb4dfac to 9ed5260

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

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

9ed5260

Corrected a silly typo

[jhpalmieri]

comment:12

You should avoid lines longer than 80 characters. I see one here:

- `is_mutable` -- (optional) boolean, determines if the output is mutable, default ``True``

In a line like

def stellar_subdivision(self,simplex,inplace=False,is_mutable=True):

the style is to put a space after each comma.

In docstrings, use single backquotes or dollar signs – `F`, $F$ – for math, but use double backquotes – ``simplex``, ``is_mutable`` – for code.

In doctests, you need to put a double colon :: before each indented line, not just a single colon. For example, you need two colons at the end of this line

The simplex to subdivide should be a face of self::

I'm assuming that you haven't actually looked at the built documentation or you would have realized that there were problems. It's good practice to look at the html documentation to make sure it is okay.

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

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

2c1c6a8

pep8 conventions and fixed doc

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 9ed5260 to 2c1c6a8

[jplab]

comment:14

Dear jhpalmieri,

Thank you for the comments. Indeed, the double ticks slipped out of my mind for the code part.

I fixed the documentation, hopefully it should be okay now.

[jhpalmieri]

comment:16

Okay, better, but now the indentation doesn't match in the input and output blocks. You need to make changes like

diff --git a/src/sage/homology/simplicial_complex.py b/src/sage/homology/simplicial_complex.py
index 2f2e997..0c77839 100644
--- a/src/sage/homology/simplicial_complex.py
+++ b/src/sage/homology/simplicial_complex.py
@@ -2776,7 +2776,7 @@ class SimplicialComplex(Parent, GenericCellComplex):

         - ``simplex`` -- a simplex in this simplicial complex
         - ``is_mutable`` -- (default: ``True``) boolean; determines if the output
-            is mutable
+          is mutable

         EXAMPLES::

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 2c1c6a8 to d68a1a2

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

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

d68a1a2

Corrected indentation of input blocks

[jplab]

comment:18

Oh! Okay, my bad! Should be okay now.

Thanks a lot for the help.

[fchapoton]

comment:19

1) The first line of the 2 new methods should rather be

+        """
+        Return the star of a simplex in this simplicial complex.

and

+        """
+        Return the stellar subdivision of a simplex in this simplicial complex.

2) in the raise statements the convention is not to start with a capital and not to end with a dot, so something like

+            raise ValueError("this simplicial complex is not mutable")

and

+            raise ValueError("the face to subdivide is not a face of self")

and then change the doctests accordingly.

3) I would add "or on a copy" here:

+        - ``inplace`` -- (default: ``False``) boolean; determines if the
+          operation is done on ``self`` or on a copy

Otherwise, this looks good.

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from d68a1a2 to 1b54fba

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

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

1b54fba

corrected conventions

[fchapoton]

comment:21

ok, let it be

[fchapoton]

Changed reviewer from Thierry Monteil to Thierry Monteil, Frédéric Chapoton

[vbraun]

Changed branch from u/jipilab/22466 to 1b54fba