Opened 13 months ago
Closed 11 months ago
#31732 closed enhancement (fixed)
ManifoldSubset: New methods declare_empty, declare_nonempty, is_empty, has_defined_points, open_cover_family
Reported by:  mkoeppe  Owned by:  

Priority:  major  Milestone:  sage9.4 
Component:  manifolds  Keywords:  
Cc:  egourgoulhon, ghmjungmath, tscrim  Merged in:  
Authors:  Matthias Koeppe  Reviewers:  Michael Jung 
Report Upstream:  N/A  Work issues:  
Branch:  359dde1 (Commits, GitHub, GitLab)  Commit:  359dde1acb2c64a04341bb8d5ffeaa58c3d51dae 
Dependencies:  #31718, #31727  Stopgaps: 
Description
A subset is declared empty by giving it (or any superset) an open cover that is an empty family.
A subset is declared nonempty by defining a point on it (or on any subset).
Change History (24)
comment:1 Changed 13 months ago by
 Dependencies changed from #31727 to #31718
 Summary changed from ManifoldSubset: New methods declare_empty, declare_nonempty, is_empty, has_defined_points to ManifoldSubset: New methods declare_empty, declare_nonempty, is_empty, has_defined_points, open_cover_family
comment:2 Changed 13 months ago by
 Branch set to u/mkoeppe/manifoldsubset__new_methods_declare_empty__declare_nonempty__is_empty__has_defined_points__open_cover_family
comment:3 Changed 13 months ago by
 Commit set to 1b5e5ace5e5cc01820d4e85e08f81d1662e5511d
 Status changed from new to needs_review
comment:4 Changed 13 months ago by
I am not sure whether this is a good point to hook in, but longterm it would be nice to things like Cech cohomology. This construction needs "good covers" which are basically given by contractible intersections. Currently, we cannot determine a subset being contractible (or even connected). But perhaps this is something to consider at this point already?
comment:5 Changed 13 months ago by
An idea to do things like this would be to try to break complicated domains into "simple" subsets (for which we can algorithmically verify that they are simply connected) and to then use algebraic topology (e.g., simplicial complexes) on the subsets. At this point, I don't know what properties other than emptiness/nonemptiness really needs to be encoded on the level of a single subset.
comment:6 Changed 13 months ago by
 Commit changed from 1b5e5ace5e5cc01820d4e85e08f81d1662e5511d to 48b2700e8e8990dbc3f9b358a6799e3c6b960854
Branch pushed to git repo; I updated commit sha1. New commits:
48b2700  src/sage/manifolds/subset.py: Remove unused import, make doctest stable by using 'sorted'

comment:7 followup: ↓ 8 Changed 13 months ago by
Could you please open a new metaticket that collects the rough ideas and updates your progress regarding subsets? I think it would make it easier to keep track, at least for me.
comment:8 in reply to: ↑ 7 Changed 13 months ago by
Replying to ghmjungmath:
Could you please open a new metaticket that collects the rough ideas and updates your progress regarding subsets? I think it would make it easier to keep track, at least for me.
If you don't plan a lot of such tickets, maybe the metaticket #30525 is sufficient.
comment:9 followup: ↓ 12 Changed 13 months ago by
Matthias already opened #31740 for that. It's a nice overview and as you can see, there are plenty of tickets concerning subsets already. Thanks Matthias!
comment:10 Changed 13 months ago by
Mh, for some reason patchbot complains about coverage.
comment:11 Changed 13 months ago by
Ah, is_empty
, declare_nonempty
and has_defined_points
do not admit examples/tests.
comment:12 in reply to: ↑ 9 Changed 13 months ago by
Replying to ghmjungmath:
Matthias already opened #31740 for that.
Ah yes!
It's a nice overview and as you can see, there are plenty of tickets concerning subsets already.
Indeed!
Thanks Matthias!
+1
comment:13 Changed 13 months ago by
 Status changed from needs_review to needs_work
comment:14 Changed 13 months ago by
 Commit changed from 48b2700e8e8990dbc3f9b358a6799e3c6b960854 to 1e6a05505661e19ad09df7716fc93f5a1a2dde4d
Branch pushed to git repo; I updated commit sha1. New commits:
1e6a055  is_empty, declare_nonempty, has_defined_points: Add doc, examples

comment:15 Changed 13 months ago by
 Status changed from needs_work to needs_review
comment:16 Changed 13 months ago by
No more coverage warnings.
comment:17 followup: ↓ 18 Changed 13 months ago by
Thanks! Patchbot is also green.
How exactly is this ticket related to #31743? What exactly are the dependencies you have in mind?
Other than that, looks good to me. Eric, what do you say?
comment:18 in reply to: ↑ 17 ; followup: ↓ 19 Changed 13 months ago by
Replying to ghmjungmath:
How exactly is this ticket related to #31743? What exactly are the dependencies you have in mind?
#31743 will certainly depend on the present ticket.
comment:19 in reply to: ↑ 18 Changed 13 months ago by
 Status changed from needs_review to positive_review
A nice and important addition to the current code. Thank you! LGTM.
comment:21 Changed 11 months ago by
 Dependencies changed from #31718 to #31718, #31727
comment:22 Changed 11 months ago by
 Commit changed from 1e6a05505661e19ad09df7716fc93f5a1a2dde4d to 359dde1acb2c64a04341bb8d5ffeaa58c3d51dae
 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:
3c5555a  Merge #31718

52a93f9  ManifoldSubset.subset_family: New; deprecate .list_of_subsets

7a22bb0  ManifoldSubset.superset_family: New, use it in doctests

bdc07ff  Merge #31677

2b47b1b  ManifoldSubset.open_superset_family: New

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

comment:23 Changed 11 months ago by
 Status changed from needs_review to positive_review
Merged #31727 to resolve a merge conflict
comment:24 Changed 11 months ago by
 Branch changed from u/mkoeppe/manifoldsubset__new_methods_declare_empty__declare_nonempty__is_empty__has_defined_points__open_cover_family to 359dde1acb2c64a04341bb8d5ffeaa58c3d51dae
 Resolution set to fixed
 Status changed from positive_review to closed
Last 10 new commits:
ManifoldSubset.open_covers: Change to generator, add optional arg 'trivial'; update uses
ManifoldSubset.subset_digraph: Use open_covers method
Merge #31718
ManifoldSubset.open_cover_family: New
ManifoldSubset.{declare_empty,declare_nonempty,is_empty,has_defined_points}: New
ManifoldSubset.{sub,super}set_{digraph,poset}: Add option 'points'
ManifoldSubset.open_covers: Add option supersets; use it to fix is_empty
ManifoldSubset.open_cover_family: Add option supersets
Fix doctests
ManifoldSubset.declare_empty: Add plot