sagemath / sage

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

Topological closure of manifold subsets, methods ManifoldSubset.closure, is_closed, declare_closed #31644

Closed mkoeppe closed 3 years ago

mkoeppe commented 3 years ago

We define a subclass of ManifoldSubset whose instances represents the topological closure of given subset in the manifold.

Subsets provide a method closure to construct it. When the subset is already closed, as detected by the new method is_closed, it just returns the input.

We also add a method declare_closed. It just sets up an open disjoint union with an open complement. (This is exactly what is_closed tests.)

The purpose of this is to build a connection of manifolds to cell complexes and convex polyhedra: In a separate ticket, we will define embedded submanifolds of euclidean spaces that arise as interiors of polyhedra or relative interiors of their faces.

Depends on #31763 Depends on #31798

CC: @egourgoulhon @tscrim @yuan-zhou @mjungmath

Component: manifolds

Author: Matthias Koeppe

Branch/Commit: 9abc617

Reviewer: Eric Gourgoulhon

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

mjungmath commented 3 years ago
comment:1

That sounds extremely interesting!

Btw: Do you really mean cell complexes or rather simplicial complexes?

mkoeppe commented 3 years ago
comment:2

Simplicial is too narrow - that would not be enough (without triangulating) to represent general convex polyhedra and their boundary structure. So need something slightly more general.

mkoeppe commented 3 years ago

Branch: u/mkoeppe/topological_closure_of_embedded_submanifolds

mkoeppe commented 3 years ago

Author: Matthias Koeppe, ...

mkoeppe commented 3 years ago

Commit: 331aa59

mkoeppe commented 3 years ago

New commits:

331aa59sage.manifolds.closure_topological_submanifold, TopologicalSubmanifold.closure: New
mkoeppe commented 3 years ago
comment:5

Here's a beginning.

Currently, is there already a way to get the image of the embedding of the submanifold in the ambient manifold as a "subset"?

mjungmath commented 3 years ago
comment:6

Replying to @mkoeppe:

New commits:

331aa59sage.manifolds.closure_topological_submanifold, TopologicalSubmanifold.closure: New

Looks already nice!

Replying to @mkoeppe:

Simplicial is too narrow - that would not be enough (without triangulating) to represent general convex polyhedra and their boundary structure. So need something slightly more general.

Right, the cube is already a counter-example.

Replying to @mkoeppe:

Here's a beginning.

Currently, is there already a way to get the image of the embedding of the submanifold in the ambient manifold as a "subset"?

No, I don't think so.

egourgoulhon commented 3 years ago
comment:7

Replying to @mjungmath:

Replying to @mkoeppe:

Here's a beginning.

Currently, is there already a way to get the image of the embedding of the submanifold in the ambient manifold as a "subset"?

No, I don't think so.

I confirm; more generally, there is no such functionality for continuous maps.

mjungmath commented 3 years ago
comment:8

Speaking of boundary; I think we should slowly but surely consider to introduce manifolds with boundary...

egourgoulhon commented 3 years ago
comment:9

Replying to @mjungmath:

Speaking of boundary; I think we should slowly but surely consider to introduce manifolds with boundary...

Indeed. There is even some demand for it: https://ask.sagemath.org/question/56532/.

mkoeppe commented 3 years ago

Dependencies: #31653

mkoeppe commented 3 years ago
comment:11

Replying to @egourgoulhon:

Replying to @mjungmath:

Replying to @mkoeppe:

Currently, is there already a way to get the image of the embedding of the submanifold in the ambient manifold as a "subset"?

No, I don't think so.

I confirm; more generally, there is no such functionality for continuous maps.

OK, I have opened #31653 for this

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

Changed commit from 331aa59 to 0579188

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

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

0579188Fixup
mkoeppe commented 3 years ago

Work Issues: redo on top of #31653

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

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

26c7e56src/sage/manifolds/continuous_map_image.py: Update doctests
3e273bbTopologicalSubmanifold.as_subset: New
9726d36Docstring work
19762aeImageManifoldSubset: New parameter domain_subset, use it in ContinuousMap.image
964f9f7src/sage/manifolds/continuous_map.py: Update copyright
e711215src/sage/manifolds/continuous_map_image.py: Add tests
0f3e36dLink in documentation of sage.manifolds.continuous_map_image
218117bsage.manifolds.closure_topological_submanifold, TopologicalSubmanifold.closure: New
3b3662eFixup
0666faeChange to ClosureOfManifoldSubset
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 0579188 to 0666fae

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

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

426e087ManifoldSubset.declare_{sub,super}set: New
72294f0ManifoldSubset.closure: New
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 0666fae to 72294f0

mkoeppe commented 3 years ago

Changed author from Matthias Koeppe, ... to Matthias Koeppe

mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -1,5 +1,6 @@
-We define a class for objects in the category of topological spaces that represents the topological closure of an embedded submanifold. 
-Embedded submanifolds will provide a method `closure` to construct it. As far as this ticket is concerned, this object is abstract: it only knows what object it is the closure of and the ambient space.
+We define a subclass of `ManifoldSubset` whose instances represents the topological closure of given subset in the manifold. 
+
+Subsets provide a method `closure` to construct it.

 The purpose of this is to build a connection of manifolds to cell complexes and convex polyhedra: In a separate ticket, we will define embedded submanifolds of euclidean spaces that arise as interiors of polyhedra or relative interiors of their faces.
mkoeppe commented 3 years ago

Changed work issues from redo on top of #31653 to none

mkoeppe commented 3 years ago

Changed dependencies from #31653 to #31653, #31763

mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -1,6 +1,6 @@
 We define a subclass of `ManifoldSubset` whose instances represents the topological closure of given subset in the manifold. 

-Subsets provide a method `closure` to construct it.
+Subsets provide a method `closure` to construct it.  When the subset is already closed, as detected by the new method `is_closed`, it just returns the input.

 The purpose of this is to build a connection of manifolds to cell complexes and convex polyhedra: In a separate ticket, we will define embedded submanifolds of euclidean spaces that arise as interiors of polyhedra or relative interiors of their faces.
mkoeppe commented 3 years ago

Changed dependencies from #31653, #31763 to #31653, #31763, #31798

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

Changed commit from 72294f0 to 0faec94

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

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

6772570ManifoldSubset.union: Handle arbitrary unions
edfea33ManifoldSubset.union: Add example and plots
fdd0abaManifoldSubset.declare_union: Handle arbitrary unions
c2f484csrc/sage/manifolds/subset.py: import itertools
a083569ManifoldSubset._intersection_subset, _union_subset: Use declare_subset, declare_superset; fix doctest
da91653ManifoldSubset._union_subset: Fixup
3e6a9cfMerge #31764
3786ceeManifoldSubset.difference, complement: New
decd831Merge #31798
0faec94ManifoldSubset.is_closed, declare_closed: New
mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -2,5 +2,7 @@

 Subsets provide a method `closure` to construct it.  When the subset is already closed, as detected by the new method `is_closed`, it just returns the input.

+We also add a method `declare_closed`. It just sets up an open disjoint union with an open complement. (This is exactly what `is_closed` tests.)
+
 The purpose of this is to build a connection of manifolds to cell complexes and convex polyhedra: In a separate ticket, we will define embedded submanifolds of euclidean spaces that arise as interiors of polyhedra or relative interiors of their faces.
mkoeppe commented 3 years ago
comment:25

I have put the new class in a new subpackage sage.manifolds.subsets (see #30139 comment:33)

egourgoulhon commented 3 years ago
comment:26

Looks nice, but there are several issues reported by the patchbot, including doctest failures.

mjungmath commented 3 years ago
comment:27

Looks like the failures are originated in #31798.

mkoeppe commented 3 years ago

Changed dependencies from #31653, #31763, #31798 to #31763, #31798

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

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

94edd68ManifoldSubset.declare_superset: Fix documentation
41826b4ManifoldSubset.declare_{sub,super}set: Expand docstring
1c14655Merge #31763
6370e88ManifoldSubset.difference, complement: New
73eceacMerge #31798
2028258sage.manifolds.subsets.closure, TopologicalSubmanifold.closure: New
38c9e96ManifoldSubset.is_closed, declare_closed: New
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 0faec94 to 38c9e96

mkoeppe commented 3 years ago
comment:30

Redid the branch on top of only #31763, #31798

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

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

3ef5141Merge #31763
79a625bManifoldSubset._reduce_intersection_members: Add examples, raise TypeError if input is empty family
8e70023ManifoldSubset._reduce_union_members: Add examples
ee4efc9ManifoldSubset._{union,intersection}_subset: Do cache the result here; add examples
e7f7a7dMerge #31764
9259a2cMerge #31798
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 38c9e96 to 9259a2c

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

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

ac53984src/sage/manifolds/subsets/__init__.py: New
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 9259a2c to ac53984

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

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

21739desrc/sage/manifolds/subset.py: Simplify code, make pyflakes happy
6bbd4aesrc/sage/manifolds/subset.py: Add coding header
2d7fda7src/sage/manifolds/subset.py: Simplify code more to make pyflakes happy
8d3ad85Merge #31764
60505b0Merge #31798
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from ac53984 to 60505b0

mjungmath commented 3 years ago
comment:35

One doctest has failed. Is that related to this ticket?

mkoeppe commented 3 years ago
comment:36

No, it's unrelated, see #31848

egourgoulhon commented 3 years ago
comment:37

Looks nice!

There is a spurious line feed in the html documentation of ManifoldSubsetClosure, which can be fixed with

     - ``name`` -- (default: computed from the name of the subset)
-       string; name (symbol) given to the closure
+      string; name (symbol) given to the closure

Besides, there should be some EXAMPLES section in the main docstring of ManifoldSubsetClosure, and the latter should start with r""", instead of """, I think.

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

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

b9909c0ManifoldSubsetClosure: Add examples, fix docstring markup
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 60505b0 to b9909c0

egourgoulhon commented 3 years ago
comment:39

I would not perform the import of ManifoldSubsetClosure in the EXAMPLES section, but rather construct the closure via the dedicated method closure(), since this is what the end user is supposed to do. In other words, I would rewrite the first part of the example as something like

sage: M = Manifold(2, 'R^2', structure='topological')
sage: c_cart.<x,y> = M.chart() # Cartesian coordinates on R^2
sage: D = M.open_subset('D', coord_def={c_cart: x^2+y^2<1}); D
Open subset D of the 2-dimensional topological manifold R^2
sage: cl_D = D.closure()
sage: cl_D
Topological closure cl_D of the Open subset D of the 2-dimensional
 topological manifold R^2
sage: latex(cl_D)
\mathop{\mathrm{cl}}(D)
sage: type(cl_D)
<class 'sage.manifolds.subsets.closure.ManifoldSubsetClosure_with_category'>
sage: cl_D.category()
Category of subobjects of sets
egourgoulhon commented 3 years ago
comment:40

Also, in the docstring of ManifoldSubset.closure(), it would be nice to add an OUTPUT field as follows:

OUTPUT:

- an instance of :class:`~sage.manifolds.subsets.closure.ManifoldSubsetClosure`

This allows one to easily access to the documentation of class ManifoldSubsetClosure from that of closure(). For completeness, one may also add an INPUT field describing the arguments name and latex_name and their default values.

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

Changed commit from b9909c0 to 16c6a72