Refine categories of Chart objects, add method codomain

mkoeppe commented 3 years ago

Currently:

sage: M = Manifold(2, 'R^2', structure='topological')
sage: c_cart.<x,y> = M.chart() # Cartesian coordinates on R^2
sage: c_cart.category()
Category of objects

A Chart instance (with non-periodic coordinates) is a continuous map from its domain to R^n. This should be reflected in the category.

With this ticket:

sage: M = Manifold(2, 'R^2', structure='topological') 
sage: c_cart.<x,y> = M.chart() # Cartesian coordinates on R^2 
sage: c_cart.category() 
Category of elements of 
 Set of Morphisms 
  from 2-dimensional topological manifold R^2 
  to Vector space of dimension 2 over Real Field with 53 bits of precision 
  in Category of sets

Also:

sage: M = Manifold(2, 'M', field='complex', structure='topological') 
sage: X.<x,y> = M.chart(coord_restrictions=lambda x,y: [abs(x)<1, y!=0]) 
sage: X.codomain()                                                                                                                                                                                   
{ (x, y) ∈ Vector space of dimension 2 over Complex Field with 53 bits of precision : abs(x) < 1, y != 0 }

To put the map in a better category than Sets will need some follow-up tickets.

Depends on #32009 Depends on #32116 Depends on #32089 Depends on #32102 Depends on #25644

CC: @egourgoulhon @tscrim @mjungmath

Component: manifolds

Author: Matthias Koeppe

Branch/Commit: u/mkoeppe/refine_categories_of_chart_objects__add_method_codomain @ 89039b2

Issue created by migration from https://trac.sagemath.org/ticket/31894

mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -5,4 +5,4 @@
 sage: c_cart.category()
 Category of objects

-A Chart instance is a continuous map from its domain to R^n. This should be reflected in the category +A Chart instance (with non-periodic coordinates) is a continuous map from its domain to R^n. This should be reflected in the category

mkoeppe commented 3 years ago

Branch: u/mkoeppe/refine_categories_of_chart_objects__add_method_codomain

mkoeppe commented 3 years ago

Commit: 759309b

mkoeppe commented 3 years ago

comment:4

Here is an attempt; but there is a metaclass conflict between Map and UniqueRepresentation that I don't know how to resolve

New commits:

759309b Chart: Make it a Map

mkoeppe commented 3 years ago

Dependencies: #32009

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 759309b to bd199dc

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

`5d5e7ba`	`Chart: Make it a Map`
`8ba174c`	`Eliminate direct use of Chart._domain`
`e6070fd`	`Merge #32009`
`bd199dc`	`WIP Chart as a Map`

mkoeppe commented 3 years ago

Changed dependencies from #32009 to #31901, #32009

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

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

27433cc Chart: Use WithEqualityById

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from bd199dc to 27433cc

mkoeppe commented 3 years ago

Work Issues: redo on top of #32116

mkoeppe commented 3 years ago

Changed work issues from redo on top of #32116 to redo on top of #32116, #32089

mkoeppe commented 3 years ago

Changed dependencies from #31901, #32009 to #32009, #32116, #32089

mkoeppe commented 3 years ago

Changed work issues from redo on top of #32116, #32089 to none

mkoeppe commented 3 years ago

Changed dependencies from #32009, #32116, #32089 to #32009, #32116, #32089, #32102

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 27433cc to 9626b54

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:

`0e63ff1`	`Merge #32116`
`f66e4bc`	`Merge #32116`
`7c003db`	`Chart.__init__: Restore default of calc_method argument for consistency`
`f85e710`	`Chart: in the description of the argument coord_restrictions, replace all instances of 'restrictions' by 'coord_restrictions'`
`80f6195`	`Chart, RealChart: In class docstring, order arguments as they appear in __classcall__/__init__`
`a39e6fc`	`DiffChart, RealDiffChart: In class docstring, order arguments as they appear in __classcall__/__init__; add description of argument coord_restrictions`
`cdf20b0`	`TopologicalManifold.chart: Add description of argument coord_restrictions`
`741fd2e`	`TopologicalManifold.chart: Add an example of using coord_restrictions`
`141ccb5`	`Merge #32009`
`9626b54`	`Merge #32102`

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 9626b54 to ce33546

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

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

`4e7020a`	`TopologicalManifold._Hom_: Handle other categories`
`ac60cc5`	`Chart._restrict_set: Fix for empty inputs, set/frozenset inputs`
`ce33546`	`Chart: Make it a Map`

mkoeppe commented 3 years ago

Author: Matthias Koeppe

mkoeppe commented 3 years ago

comment:15

This is almost ready, except that a Chart does not have the correct element class of the Homset, so some _test_category tests are failing. Help with this would be very welcome.

mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -1,3 +1,4 @@
+Currently:

sage: M = Manifold(2, 'R^2', structure='topological') @@ -5,4 +6,28 @@ sage: c_cart.category() Category of objects

-A `Chart` instance (with non-periodic coordinates) is a continuous map from its domain to `R^n`. This should be reflected in the category
+A `Chart` instance (with non-periodic coordinates) is a continuous map from its domain to `R^n`. This should be reflected in the category.
+
+With this ticket:
+
+```
+sage: M = Manifold(2, 'R^2', structure='topological') 
+sage: c_cart.<x,y> = M.chart() # Cartesian coordinates on R^2 
+sage: c_cart.category() 
+Category of elements of 
+ Set of Morphisms 
+  from 2-dimensional topological manifold R^2 
+  to Vector space of dimension 2 over Real Field with 53 bits of precision 
+  in Category of sets
+```
+
+Also:
+
+```
+sage: M = Manifold(2, 'M', field='complex', structure='topological') 
+sage: X.<x,y> = M.chart(coord_restrictions=lambda x,y: [abs(x)<1, y!=0]) 
+sage: X.codomain()                                                                                                                                                                                   
+{ (x, y) ∈ Vector space of dimension 2 over Complex Field with 53 bits of precision : abs(x) < 1, y != 0 }
+```
+
+To put the map in a better category than `Sets` will need some follow-up tickets.

tscrim commented 3 years ago

comment:17

We generally skip _test_category for Homset subclasses IIRC. They can often have multiple classes that are all elements because we need different implementations based on input types. We currently don't have a good system for dealing with parents with multiple element classes. :/

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from ce33546 to 728c3df

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

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

`fff2a79`	`src/sage/sets/set.py: Fix docstring markup`
`1ceafd0`	`Merge #32013`
`c89c697`	`Merge #32102`
`451f5cf`	`Sets.ParentMethods: Update doctest`
`f135a05`	`Merge #32015`
`728c3df`	`Merge #32089`

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 728c3df to 9d19b21

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

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

9d19b21 Chart, DiffChart: Skip _test_category

mkoeppe commented 3 years ago

comment:20

Replying to @tscrim:

We generally skip _test_category for Homset subclasses IIRC. They can often have multiple classes that are all elements because we need different implementations based on input types.

Thanks, that's a simple solution. Ready for review then

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

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

d554f09 Chart.is_injective, is_surjective, inverse, ...

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 9d19b21 to d554f09

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from d554f09 to 243fcfd

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

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

`acdb6a7`	`Chart, ChartInverse._call_`
`243fcfd`	`Periodic charts as sections`

mjungmath commented 3 years ago

comment:24

Why is a periodic chart considered non-surjective but injective? Isn't it exactly the opposite?

Besides, Travis suggested that periodic charts could be considered being proper charts but glued together #31324@comment:25. Maybe we can somehow incorporate this here?

mkoeppe commented 3 years ago

comment:25

I have two possible variants:

as of acdb6a7, the codomain of the Chart is a fundamental cell of the periodic coordinate space. Presumably the user declares the periodic coordinates in a way that the computed coordinates always lie in the fundamental domain. Corestricted like this, the chart is bijective, with a bijective ChartInverse as its inverse. But it does not capture the fact that one is also allowed to use coordinates outside of the fundamental domain to construct a point.
with 243fcfd + work in progress (not on the branch yet), the ChartInverse is a surjection from the full coordinate space to the chart's domain, and Chart is a section map of the ChartInverse.

I don't know which of the two you would find preferable and also cannot speak to Travis' suggestion regarding possible changes to the design.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

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

`b76ddd7`	`Chart: Refactor through methods _init_coordinates, _codomain_set; remove _domain attribute`
`c21f884`	`Chart._codomain_set: Add option periodic_extension, use it to construct the domain of the ChartInverse`

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 243fcfd to c21f884

mkoeppe commented 3 years ago

comment:27

Here's a more complete version of the second variant.

mjungmath commented 3 years ago

comment:29

I like the second variant. It better reflects the situation and what periodic charts actually are.

Then it also makes sense that periodic charts (as maps from a manifold subset into a subset of R^n) are injective but non-surjective.

mjungmath commented 3 years ago

comment:30

But then we have to make a choice, right? The current situation is as follows (for example):

sage: S1 = manifolds.Sphere(1)
sage: spher = S1.spherical_coordinates()
sage: p = S1.point((pi,))
sage: q = S1.point((-pi,))
sage: spher(p)
(pi,)
sage: spher(q)
(-pi,)

At current stage, this is not a well-defined map.

mkoeppe commented 3 years ago

comment:31

That's right, there's currently no code to normalize the coordinates

mkoeppe commented 3 years ago

comment:32

It would all be easier if we had an implementation of RR/ZZ in Sage...

mjungmath commented 3 years ago

comment:33

Replying to @mkoeppe:

It would all be easier if we had an implementation of RR/ZZ in Sage...

...or even better topological quotient spaces.

mkoeppe commented 3 years ago

comment:34

OK, QQ/ZZ exists (sage.groups.additive_abelian.qmodnz), and we also have at least two incompatible implementations of lattices and their quotients in sage.geometry.toric_lattice and sage.modules.free_module_integer. Perhaps also something in sage.schemes somewhere

mjungmath commented 3 years ago

comment:35

What about abstract quotients in the spirit of #31644 and/or #31691?

mkoeppe commented 3 years ago

comment:36

Well, I have #32012 (Quotient of polyhedron or convex set by a lattice) but one has to be careful that things remain concrete enough to be useful, as the main point of the ticket is to make things more "composable".

mjungmath commented 3 years ago

comment:37

Periodic charts are always defined via (connected) intervals, aren't they? W.l.o.g. we can always choose the minimum/maximum the chart maps to? Alternatively, we can make a chart a bijection onto (cross products of) half-open intervals? Please correct me if I'm wrong.

egourgoulhon commented 3 years ago

comment:38

Replying to @mjungmath:

I like the second variant. It better reflects the situation and what periodic charts actually are.

+1

egourgoulhon commented 3 years ago

comment:39

Replying to @mjungmath:

Periodic charts are always defined via (connected) intervals, aren't they?

Yes I think so.

W.l.o.g. we can always choose the minimum/maximum the chart maps to?

What do you mean?

egourgoulhon commented 3 years ago

comment:40

Just a word of context about periodic charts: they have been introduced because they are useful when computing a geodesic with some numerical integrator. Typically, when the geodesic is an orbit around some center, the azimuthal coordinate returned by the integrator increases without any bound, instead of being confined to [0, 2\pi). Here are some examples in Kerr spacetime.

mkoeppe commented 3 years ago

comment:41

Replying to @egourgoulhon:

Replying to @mjungmath:

I like the second variant. It better reflects the situation and what periodic charts actually are.

+1

OK, then it just remains to make it well-defined (comment:30, comment:31)

sagemath / sage

Refine categories of Chart objects, add method codomain #31894