Opened 6 months ago

Closed 4 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: sage-9.4
Component: manifolds Keywords:
Cc: egourgoulhon, gh-mjungmath, 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:

Status badges

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 6 months ago by mkoeppe

  • 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 6 months ago by mkoeppe

  • Branch set to u/mkoeppe/manifoldsubset__new_methods_declare_empty__declare_nonempty__is_empty__has_defined_points__open_cover_family

comment:3 Changed 6 months ago by mkoeppe

  • Commit set to 1b5e5ace5e5cc01820d4e85e08f81d1662e5511d
  • Status changed from new to needs_review

Last 10 new commits:

78cc27aManifoldSubset.open_covers: Change to generator, add optional arg 'trivial'; update uses
e026e7aManifoldSubset.subset_digraph: Use open_covers method
5a072cfMerge #31718
83a9f47ManifoldSubset.open_cover_family: New
a183575ManifoldSubset.{declare_empty,declare_nonempty,is_empty,has_defined_points}: New
4ad562dManifoldSubset.{sub,super}set_{digraph,poset}: Add option 'points'
cec7fa2ManifoldSubset.open_covers: Add option supersets; use it to fix is_empty
c40ec03ManifoldSubset.open_cover_family: Add option supersets
a066387Fix doctests
1b5e5acManifoldSubset.declare_empty: Add plot

comment:4 Changed 6 months ago by gh-mjungmath

I am not sure whether this is a good point to hook in, but long-term 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 6 months ago by mkoeppe

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 6 months ago by git

  • Commit changed from 1b5e5ace5e5cc01820d4e85e08f81d1662e5511d to 48b2700e8e8990dbc3f9b358a6799e3c6b960854

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

48b2700src/sage/manifolds/subset.py: Remove unused import, make doctest stable by using 'sorted'

comment:7 follow-up: Changed 6 months ago by gh-mjungmath

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 6 months ago by egourgoulhon

Replying to gh-mjungmath:

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 follow-up: Changed 6 months ago by gh-mjungmath

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 6 months ago by gh-mjungmath

Mh, for some reason patchbot complains about coverage.

comment:11 Changed 6 months ago by gh-mjungmath

Ah, is_empty, declare_nonempty and has_defined_points do not admit examples/tests.

Last edited 6 months ago by gh-mjungmath (previous) (diff)

comment:12 in reply to: ↑ 9 Changed 6 months ago by egourgoulhon

Replying to gh-mjungmath:

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 6 months ago by mkoeppe

  • Status changed from needs_review to needs_work

comment:14 Changed 6 months ago by git

  • Commit changed from 48b2700e8e8990dbc3f9b358a6799e3c6b960854 to 1e6a05505661e19ad09df7716fc93f5a1a2dde4d

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

1e6a055is_empty, declare_nonempty, has_defined_points: Add doc, examples

comment:15 Changed 6 months ago by mkoeppe

  • Status changed from needs_work to needs_review

comment:16 Changed 6 months ago by mkoeppe

No more coverage warnings.

comment:17 follow-up: Changed 6 months ago by gh-mjungmath

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 ; follow-up: Changed 6 months ago by mkoeppe

Replying to gh-mjungmath:

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 6 months ago by gh-mjungmath

  • Status changed from needs_review to positive_review

A nice and important addition to the current code. Thank you! LGTM.

comment:20 Changed 6 months ago by mkoeppe

  • Reviewers set to Michael Jung

Thanks for the review!

comment:21 Changed 4 months ago by mkoeppe

  • Dependencies changed from #31718 to #31718, #31727

comment:22 Changed 4 months ago by git

  • 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:

3c5555aMerge #31718
52a93f9ManifoldSubset.subset_family: New; deprecate .list_of_subsets
7a22bb0ManifoldSubset.superset_family: New, use it in doctests
bdc07ffMerge #31677
2b47b1bManifoldSubset.open_superset_family: New
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

comment:23 Changed 4 months ago by mkoeppe

  • Status changed from needs_review to positive_review

Merged #31727 to resolve a merge conflict

comment:24 Changed 4 months ago by vbraun

  • 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
Note: See TracTickets for help on using tickets.