#31644 closed enhancement (fixed)

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

Reported by: Matthias Köppe Owned by:
Priority: major Milestone: sage-9.4
Component: manifolds Keywords:
Cc: Eric Gourgoulhon, Travis Scrimshaw, Yuan Zhou, Michael Jung Merged in:
Authors: Matthias Koeppe Reviewers: Eric Gourgoulhon
Report Upstream: N/A Work issues:
Branch: 9abc617 (Commits, GitHub, GitLab) Commit: 9abc6174394c85ca400650c814a3d27e7921729d
Dependencies: #31763, #31798 Stopgaps:

Status badges

Description (last modified by Matthias Köppe)

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.

Change History (47)

comment:1 Changed 20 months ago by Michael Jung

That sounds extremely interesting!

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

comment:2 Changed 20 months ago by Matthias Köppe

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.

comment:3 Changed 20 months ago by Matthias Köppe

Branch: u/mkoeppe/topological_closure_of_embedded_submanifolds

comment:4 Changed 20 months ago by Matthias Köppe

Authors: Matthias Koeppe, ...
Commit: 331aa59f61c065ac9b6237e5d37ae59ba652ece6

New commits:

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

comment:5 Changed 20 months ago by Matthias Köppe

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"?

comment:6 in reply to:  4 ; Changed 20 months ago by Michael Jung

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.

comment:7 in reply to:  6 ; Changed 20 months ago by Eric Gourgoulhon

Replying to gh-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.

comment:8 Changed 20 months ago by Michael Jung

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

comment:9 in reply to:  8 Changed 20 months ago by Eric Gourgoulhon

Replying to gh-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/.

comment:10 Changed 20 months ago by Matthias Köppe

Dependencies: #31653
Summary: Topological closure of embedded submanifoldsTopological closure of manifold subsets, embedded submanifolds

comment:11 in reply to:  7 Changed 20 months ago by Matthias Köppe

Replying to egourgoulhon:

Replying to gh-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

comment:12 Changed 20 months ago by git

Commit: 331aa59f61c065ac9b6237e5d37ae59ba652ece60579188638bb4f858976e9e8682f5c854e71eb66

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

0579188Fixup

comment:13 Changed 20 months ago by Matthias Köppe

Work issues: redo on top of #31653

comment:14 Changed 20 months ago by Matthias Köppe

Milestone: sage-9.3sage-9.4

comment:15 Changed 19 months ago by git

Commit: 0579188638bb4f858976e9e8682f5c854e71eb660666fae89c698e792b534c81395bb42deb2b3151

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

comment:16 Changed 19 months ago by git

Commit: 0666fae89c698e792b534c81395bb42deb2b315172294f04f8a99651a74f8629cdf3ba1278ffc0f4

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

426e087ManifoldSubset.declare_{sub,super}set: New
72294f0ManifoldSubset.closure: New

comment:17 Changed 19 months ago by Matthias Köppe

Authors: Matthias Koeppe, ...Matthias Koeppe
Description: modified (diff)
Summary: Topological closure of manifold subsets, embedded submanifoldsTopological closure of manifold subsets
Work issues: redo on top of #31653

comment:18 Changed 19 months ago by Matthias Köppe

Dependencies: #31653#31653, #31763

comment:19 Changed 19 months ago by Matthias Köppe

Summary: Topological closure of manifold subsetsTopological closure of manifold subsets, methods ManifoldSubset.closure, is_closed

comment:20 Changed 19 months ago by Matthias Köppe

Description: modified (diff)

comment:21 Changed 19 months ago by Matthias Köppe

Dependencies: #31653, #31763#31653, #31763, #31798

comment:22 Changed 19 months ago by git

Commit: 72294f04f8a99651a74f8629cdf3ba1278ffc0f40faec94bad521406c4f4f2da04ef851051504ec9

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

comment:23 Changed 19 months ago by Matthias Köppe

Status: newneeds_review

comment:24 Changed 19 months ago by Matthias Köppe

Description: modified (diff)
Summary: Topological closure of manifold subsets, methods ManifoldSubset.closure, is_closedTopological closure of manifold subsets, methods ManifoldSubset.closure, is_closed, declare_closed

comment:25 Changed 19 months ago by Matthias Köppe

I have put the new class in a new subpackage sage.manifolds.subsets (see https://trac.sagemath.org/ticket/30139#comment:33)

comment:26 Changed 19 months ago by Eric Gourgoulhon

Status: needs_reviewneeds_work

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

comment:27 Changed 19 months ago by Michael Jung

Looks like the failures are originated in #31798.

comment:28 Changed 19 months ago by Matthias Köppe

Dependencies: #31653, #31763, #31798#31763, #31798

comment:29 Changed 19 months ago by git

Commit: 0faec94bad521406c4f4f2da04ef851051504ec938c9e96e08560fbf7fa28397bba2e46b16e87e0e

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

comment:30 Changed 19 months ago by Matthias Köppe

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

comment:31 Changed 19 months ago by Matthias Köppe

Status: needs_workneeds_review

comment:32 Changed 19 months ago by git

Commit: 38c9e96e08560fbf7fa28397bba2e46b16e87e0e9259a2c109ce9ac069e80f2c5aa090fb533e1690

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

comment:33 Changed 19 months ago by git

Commit: 9259a2c109ce9ac069e80f2c5aa090fb533e1690ac5398499c314b8a59b5994c2cfa8b5945f3b311

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

ac53984src/sage/manifolds/subsets/__init__.py: New

comment:34 Changed 19 months ago by git

Commit: ac5398499c314b8a59b5994c2cfa8b5945f3b31160505b09c0a05e08282020ef1284db5ecff8ce6f

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

comment:35 Changed 18 months ago by Michael Jung

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

comment:36 Changed 18 months ago by Matthias Köppe

No, it's unrelated, see #31848

comment:37 Changed 18 months ago by Eric Gourgoulhon

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.

comment:38 Changed 18 months ago by git

Commit: 60505b09c0a05e08282020ef1284db5ecff8ce6fb9909c061046a204354341833e3b3fcb4c6d7341

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

b9909c0ManifoldSubsetClosure: Add examples, fix docstring markup

comment:39 Changed 18 months ago by Eric Gourgoulhon

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

comment:40 Changed 18 months ago by Eric Gourgoulhon

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.

comment:41 Changed 18 months ago by git

Commit: b9909c061046a204354341833e3b3fcb4c6d734116c6a7219848d2045fe92b1ca50eb0195a05d91f

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

16c6a72ManifoldSubsetClosure, ManifoldSubset.closure: Improve documentation

comment:42 Changed 18 months ago by Matthias Köppe

Thanks for the suggestions, done.

comment:43 Changed 18 months ago by Eric Gourgoulhon

Reviewers: Eric Gourgoulhon
Status: needs_reviewpositive_review

Thanks!

comment:44 Changed 18 months ago by Matthias Köppe

Thank you!

comment:45 Changed 18 months ago by git

Commit: 16c6a7219848d2045fe92b1ca50eb0195a05d91f9abc6174394c85ca400650c814a3d27e7921729d
Status: positive_reviewneeds_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

359dde1Merge branch 't/31727/manifoldsubset__add_methods_subset_family__superset_family__equal_subset_family__deprecate_method_list_of_subsets' into t/31732/manifoldsubset__new_methods_declare_empty__declare_nonempty__is_empty__has_defined_points__open_cover_family
fd4506aMerge #31732
7fed9efMerge #31736
c4acd09Merge tag '9.4.beta2' into t/31763/manifoldsubset__new_methods_declare_subset__declare_superset
2ba18abMerge #31763
0e9e5cdMerge #31732
a96f736Merge #31736
582b58dMerge #31763
2113f78Merge #31764
9abc617Merge #31798

comment:46 Changed 18 months ago by Matthias Köppe

Status: needs_reviewpositive_review

Trivial merge of updated dependency #31763

comment:47 Changed 17 months ago by Volker Braun

Branch: u/mkoeppe/topological_closure_of_embedded_submanifolds9abc6174394c85ca400650c814a3d27e7921729d
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.