Opened 12 months ago

Closed 4 months ago

#30832 closed enhancement (fixed)

Connect RealSet to sage.manifolds

Reported by: mkoeppe Owned by:
Priority: major Milestone: sage-9.4
Component: manifolds Keywords:
Cc: gh-mjungmath, egourgoulhon, tscrim Merged in:
Authors: Matthias Koeppe Reviewers: Michael Jung
Report Upstream: N/A Work issues:
Branch: 81b43f4 (Commits, GitHub, GitLab) Commit: 81b43f4bb4d337f3cb1cf59c2d866fddab51b6b2
Dependencies: #31644, #31877 Stopgaps:

Status badges

Description (last modified by mkoeppe)

Instances of RealSet that are finite unions of open intervals should be connected in some way to sage.manifolds, in particular to OpenInterval and RealLine (which are both in the global namespace).

In this ticket, we add functionality to initialize a RealSet from instances of OpenInterval and RealLine, as well as from their topological closures (#31644)

Change History (26)

comment:1 Changed 9 months ago by gh-mjungmath

  • Cc tscrim added

comment:2 Changed 8 months ago by mkoeppe

  • Milestone changed from sage-9.3 to sage-9.4

Setting new milestone based on a cursory review of ticket status, priority, and last modification date.

comment:3 Changed 5 months ago by mkoeppe

  • Dependencies set to #31644
  • Description modified (diff)

comment:4 Changed 5 months ago by mkoeppe

  • Branch set to u/mkoeppe/connect_realset_to_sage_manifolds

comment:5 Changed 5 months ago by mkoeppe

  • Authors set to Matthias Koeppe
  • Commit set to fb33c0f4f26e907ba58a75224efaf263f245d69d
  • Status changed from new to needs_review

Last 10 new commits:

e7f7a7dMerge #31764
9259a2cMerge #31798
ac53984src/sage/manifolds/subsets/__init__.py: New
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
9dd9f90Merge #31644
fb33c0fRealSet: Allow initialization from RealLine and OpenInterval instances and OpenInterval closures

comment:6 Changed 5 months ago by gh-mjungmath

Restricted to open intervals (not closures), this is nothing but the forgetful functor from manifolds into sets. Perhaps it is a good idea to add this functor somehow?

Other than that it looks good! Let's wait for the patchbot.

comment:7 follow-up: Changed 5 months ago by mkoeppe

... or rather from manifolds to topological spaces (see #31877).

Do we have forgetful functors anywhere already?

comment:8 follow-up: Changed 5 months ago by mkoeppe

We may also want to consider

  • adding a method RealSet.as_manifold_subset
  • extending the RealSet constructors (default constructor and RealSet.open etc.) so that they can build manifold objects; for example if structure or name is passed; then perhaps the global bindings RealLine and OpenInterval can be deprecated.

comment:9 follow-up: Changed 5 months ago by gh-mjungmath

Notice that RealSet supports unions of intervals whereas OpenInterval does not.

comment:10 follow-up: Changed 5 months ago by gh-mjungmath

Moreover, you represented closed or clopen intervals by a topological closure, which is fine for now. But as soon as we have manifolds with boundary, we probably want them to be represented by those objects/instances instead (or as well).

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

comment:11 in reply to: ↑ 8 Changed 5 months ago by gh-mjungmath

  • Reviewers set to Michael Jung
  • Status changed from needs_review to positive_review

LGTM

comment:12 in reply to: ↑ 9 ; follow-up: Changed 5 months ago by mkoeppe

Replying to gh-mjungmath:

Notice that RealSet supports unions of intervals whereas OpenInterval does not.

Yes, it does. You can initialize a RealSet using several OpenInterval instances, and it will take the union.

comment:13 in reply to: ↑ 10 Changed 5 months ago by mkoeppe

Thanks for the review!

comment:14 in reply to: ↑ 12 ; follow-up: Changed 5 months ago by gh-mjungmath

Replying to mkoeppe:

Replying to gh-mjungmath:

Notice that RealSet supports unions of intervals whereas OpenInterval does not.

Yes, it does. You can initialize a RealSet using several OpenInterval instances, and it will take the union.

That is not what I mean. There are no unions of OpenInterval on the manifold level yet. This is, for example, important for a potential RealSet.as_manifold_subset.

comment:15 in reply to: ↑ 14 Changed 5 months ago by mkoeppe

Replying to gh-mjungmath:

There are no unions of OpenInterval on the manifold level yet. This is, for example, important for a potential RealSet.as_manifold_subset.

If you create OpenInterval instances with the same ambient_interval set, then you can take unions.

sage: I02 = OpenInterval(0, 2, ambient_interval=RealLine())
sage: I13 = OpenInterval(1, 3, ambient_interval=RealLine())
sage: I02.union(I13)
Open subset (0, 2)_union_(1, 3) of the Real number line R
Last edited 5 months ago by mkoeppe (previous) (diff)

comment:16 Changed 5 months ago by gh-mjungmath

Right, but

sage: I02 = OpenInterval(0, 2, ambient_interval=RealLine())
sage: I13 = OpenInterval(1, 3, ambient_interval=RealLine())
sage: U = I02.union(I13)
sage: RealSet(U)

won't work, no?

comment:17 Changed 5 months ago by mkoeppe

Good catch...

comment:18 Changed 5 months ago by mkoeppe

Folllow up in #31878.

comment:19 in reply to: ↑ 7 Changed 5 months ago by tscrim

Replying to mkoeppe:

... or rather from manifolds to topological spaces (see #31877).

Do we have forgetful functors anywhere already?

categories.functor.ForgetfulFunctor

comment:20 Changed 4 months ago by git

  • Commit changed from fb33c0f4f26e907ba58a75224efaf263f245d69d to 785aed0f2ef1efb2282e003c39be671ebcd74fd9
  • Status changed from positive_review to needs_review

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

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
785aed0Merge #31644

comment:21 Changed 4 months ago by mkoeppe

  • Status changed from needs_review to positive_review

Trivial merge with updated dependency

comment:22 Changed 4 months ago by mkoeppe

  • Dependencies changed from #31644 to #31644, #31877

comment:23 Changed 4 months ago by git

  • Commit changed from 785aed0f2ef1efb2282e003c39be671ebcd74fd9 to 81b43f4bb4d337f3cb1cf59c2d866fddab51b6b2
  • 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:

4b09685RealSet: Put it in a suitable subcategory of TopologicalSpaces()
46eed0eRealSet.ambient: Change to a normal method
5b8cecaInternalRealInterval, RealSet: Add _latex_ methods
69ca854RealSet._repr_: Use unicode cup sign instead of +
dbdfc06InternalRealInterval, RealSet: Remove extra whitespace in latex, add documentation
7f56338PiecewiseFunction: Adjust doctests for changed RealSet repr
8abdc8bsrc/sage/functions/piecewise.py: Add coding header
5b0f85dMerge #31880
81b43f4Merge branch 't/31877/refine_category_of_realset' into t/30832/connect_realset_to_sage_manifolds

comment:24 Changed 4 months ago by mkoeppe

Another trivial merge to remove a merge conflict

comment:25 Changed 4 months ago by mkoeppe

  • Status changed from needs_review to positive_review

comment:26 Changed 4 months ago by vbraun

  • Branch changed from u/mkoeppe/connect_realset_to_sage_manifolds to 81b43f4bb4d337f3cb1cf59c2d866fddab51b6b2
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.