Opened 14 months ago
Closed 11 months ago
#31644 closed enhancement (fixed)
Topological closure of manifold subsets, methods ManifoldSubset.closure, is_closed, declare_closed
Reported by: | mkoeppe | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-9.4 |
Component: | manifolds | Keywords: | |
Cc: | egourgoulhon, tscrim, yzh, gh-mjungmath | 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: |
Description (last modified by )
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 14 months ago by
comment:2 Changed 14 months ago by
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 14 months ago by
- Branch set to u/mkoeppe/topological_closure_of_embedded_submanifolds
comment:4 follow-up: ↓ 6 Changed 14 months ago by
- Commit set to 331aa59f61c065ac9b6237e5d37ae59ba652ece6
New commits:
331aa59 | sage.manifolds.closure_topological_submanifold, TopologicalSubmanifold.closure: New
|
comment:5 Changed 14 months ago by
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 ; follow-up: ↓ 7 Changed 14 months ago by
Replying to mkoeppe:
New commits:
331aa59 sage.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 ; follow-up: ↓ 11 Changed 14 months ago by
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 follow-up: ↓ 9 Changed 14 months ago by
Speaking of boundary; I think we should slowly but surely consider to introduce manifolds with boundary...
comment:9 in reply to: ↑ 8 Changed 14 months ago by
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 14 months ago by
- Dependencies set to #31653
- Summary changed from Topological closure of embedded submanifolds to Topological closure of manifold subsets, embedded submanifolds
comment:11 in reply to: ↑ 7 Changed 14 months ago by
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 14 months ago by
- Commit changed from 331aa59f61c065ac9b6237e5d37ae59ba652ece6 to 0579188638bb4f858976e9e8682f5c854e71eb66
Branch pushed to git repo; I updated commit sha1. New commits:
0579188 | Fixup
|
comment:13 Changed 14 months ago by
- Work issues set to redo on top of #31653
comment:14 Changed 14 months ago by
- Milestone changed from sage-9.3 to sage-9.4
comment:15 Changed 13 months ago by
- Commit changed from 0579188638bb4f858976e9e8682f5c854e71eb66 to 0666fae89c698e792b534c81395bb42deb2b3151
Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:
26c7e56 | src/sage/manifolds/continuous_map_image.py: Update doctests
|
3e273bb | TopologicalSubmanifold.as_subset: New
|
9726d36 | Docstring work
|
19762ae | ImageManifoldSubset: New parameter domain_subset, use it in ContinuousMap.image
|
964f9f7 | src/sage/manifolds/continuous_map.py: Update copyright
|
e711215 | src/sage/manifolds/continuous_map_image.py: Add tests
|
0f3e36d | Link in documentation of sage.manifolds.continuous_map_image
|
218117b | sage.manifolds.closure_topological_submanifold, TopologicalSubmanifold.closure: New
|
3b3662e | Fixup
|
0666fae | Change to ClosureOfManifoldSubset
|
comment:16 Changed 13 months ago by
- Commit changed from 0666fae89c698e792b534c81395bb42deb2b3151 to 72294f04f8a99651a74f8629cdf3ba1278ffc0f4
comment:17 Changed 13 months ago by
- Description modified (diff)
- Summary changed from Topological closure of manifold subsets, embedded submanifolds to Topological closure of manifold subsets
- Work issues redo on top of #31653 deleted
comment:18 Changed 13 months ago by
- Dependencies changed from #31653 to #31653, #31763
comment:19 Changed 13 months ago by
- Summary changed from Topological closure of manifold subsets to Topological closure of manifold subsets, methods ManifoldSubset.closure, is_closed
comment:20 Changed 13 months ago by
- Description modified (diff)
comment:21 Changed 13 months ago by
- Dependencies changed from #31653, #31763 to #31653, #31763, #31798
comment:22 Changed 13 months ago by
- Commit changed from 72294f04f8a99651a74f8629cdf3ba1278ffc0f4 to 0faec94bad521406c4f4f2da04ef851051504ec9
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
6772570 | ManifoldSubset.union: Handle arbitrary unions
|
edfea33 | ManifoldSubset.union: Add example and plots
|
fdd0aba | ManifoldSubset.declare_union: Handle arbitrary unions
|
c2f484c | src/sage/manifolds/subset.py: import itertools
|
a083569 | ManifoldSubset._intersection_subset, _union_subset: Use declare_subset, declare_superset; fix doctest
|
da91653 | ManifoldSubset._union_subset: Fixup
|
3e6a9cf | Merge #31764
|
3786cee | ManifoldSubset.difference, complement: New
|
decd831 | Merge #31798
|
0faec94 | ManifoldSubset.is_closed, declare_closed: New
|
comment:23 Changed 13 months ago by
- Status changed from new to needs_review
comment:24 Changed 13 months ago by
- Description modified (diff)
- Summary changed from Topological closure of manifold subsets, methods ManifoldSubset.closure, is_closed to Topological closure of manifold subsets, methods ManifoldSubset.closure, is_closed, declare_closed
comment:25 Changed 13 months ago by
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 13 months ago by
- Status changed from needs_review to needs_work
Looks nice, but there are several issues reported by the patchbot, including doctest failures.
comment:27 Changed 13 months ago by
Looks like the failures are originated in #31798.
comment:28 Changed 13 months ago by
- Dependencies changed from #31653, #31763, #31798 to #31763, #31798
comment:29 Changed 13 months ago by
- Commit changed from 0faec94bad521406c4f4f2da04ef851051504ec9 to 38c9e96e08560fbf7fa28397bba2e46b16e87e0e
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
94edd68 | ManifoldSubset.declare_superset: Fix documentation
|
41826b4 | ManifoldSubset.declare_{sub,super}set: Expand docstring
|
1c14655 | Merge #31763
|
6370e88 | ManifoldSubset.difference, complement: New
|
73eceac | Merge #31798
|
2028258 | sage.manifolds.subsets.closure, TopologicalSubmanifold.closure: New
|
38c9e96 | ManifoldSubset.is_closed, declare_closed: New
|
comment:30 Changed 13 months ago by
comment:31 Changed 13 months ago by
- Status changed from needs_work to needs_review
comment:32 Changed 13 months ago by
- Commit changed from 38c9e96e08560fbf7fa28397bba2e46b16e87e0e to 9259a2c109ce9ac069e80f2c5aa090fb533e1690
Branch pushed to git repo; I updated commit sha1. New commits:
3ef5141 | Merge #31763
|
79a625b | ManifoldSubset._reduce_intersection_members: Add examples, raise TypeError if input is empty family
|
8e70023 | ManifoldSubset._reduce_union_members: Add examples
|
ee4efc9 | ManifoldSubset._{union,intersection}_subset: Do cache the result here; add examples
|
e7f7a7d | Merge #31764
|
9259a2c | Merge #31798
|
comment:33 Changed 13 months ago by
- Commit changed from 9259a2c109ce9ac069e80f2c5aa090fb533e1690 to ac5398499c314b8a59b5994c2cfa8b5945f3b311
Branch pushed to git repo; I updated commit sha1. New commits:
ac53984 | src/sage/manifolds/subsets/__init__.py: New
|
comment:34 Changed 12 months ago by
- Commit changed from ac5398499c314b8a59b5994c2cfa8b5945f3b311 to 60505b09c0a05e08282020ef1284db5ecff8ce6f
Branch pushed to git repo; I updated commit sha1. New commits:
21739de | src/sage/manifolds/subset.py: Simplify code, make pyflakes happy
|
6bbd4ae | src/sage/manifolds/subset.py: Add coding header
|
2d7fda7 | src/sage/manifolds/subset.py: Simplify code more to make pyflakes happy
|
8d3ad85 | Merge #31764
|
60505b0 | Merge #31798
|
comment:35 Changed 12 months ago by
One doctest has failed. Is that related to this ticket?
comment:36 Changed 12 months ago by
No, it's unrelated, see #31848
comment:37 Changed 12 months ago by
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 12 months ago by
- Commit changed from 60505b09c0a05e08282020ef1284db5ecff8ce6f to b9909c061046a204354341833e3b3fcb4c6d7341
Branch pushed to git repo; I updated commit sha1. New commits:
b9909c0 | ManifoldSubsetClosure: Add examples, fix docstring markup
|
comment:39 Changed 12 months ago by
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 12 months ago by
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 12 months ago by
- Commit changed from b9909c061046a204354341833e3b3fcb4c6d7341 to 16c6a7219848d2045fe92b1ca50eb0195a05d91f
Branch pushed to git repo; I updated commit sha1. New commits:
16c6a72 | ManifoldSubsetClosure, ManifoldSubset.closure: Improve documentation
|
comment:42 Changed 12 months ago by
Thanks for the suggestions, done.
comment:43 Changed 12 months ago by
- Reviewers set to Eric Gourgoulhon
- Status changed from needs_review to positive_review
Thanks!
comment:44 Changed 12 months ago by
Thank you!
comment:45 Changed 11 months ago by
- Commit changed from 16c6a7219848d2045fe92b1ca50eb0195a05d91f to 9abc6174394c85ca400650c814a3d27e7921729d
- Status changed from positive_review to needs_review
Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:
359dde1 | Merge 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
|
fd4506a | Merge #31732
|
7fed9ef | Merge #31736
|
c4acd09 | Merge tag '9.4.beta2' into t/31763/manifoldsubset__new_methods_declare_subset__declare_superset
|
2ba18ab | Merge #31763
|
0e9e5cd | Merge #31732
|
a96f736 | Merge #31736
|
582b58d | Merge #31763
|
2113f78 | Merge #31764
|
9abc617 | Merge #31798
|
comment:46 Changed 11 months ago by
- Status changed from needs_review to positive_review
Trivial merge of updated dependency #31763
comment:47 Changed 11 months ago by
- Branch changed from u/mkoeppe/topological_closure_of_embedded_submanifolds to 9abc6174394c85ca400650c814a3d27e7921729d
- Resolution set to fixed
- Status changed from positive_review to closed
That sounds extremely interesting!
Btw: Do you really mean cell complexes or rather simplicial complexes?