Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refine categories of Chart objects, add method codomain #31894

Open
mkoeppe opened this issue Jun 2, 2021 · 62 comments
Open

Refine categories of Chart objects, add method codomain #31894

mkoeppe opened this issue Jun 2, 2021 · 62 comments

Comments

@mkoeppe
Copy link
Member

mkoeppe commented Jun 2, 2021

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 mkoeppe added this to the sage-9.4 milestone Jun 2, 2021
@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Member Author

mkoeppe commented Jun 3, 2021

@mkoeppe
Copy link
Member Author

mkoeppe commented Jun 3, 2021

Commit: 759309b

@mkoeppe
Copy link
Member Author

mkoeppe commented Jun 3, 2021

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:

759309bChart: Make it a Map

@mkoeppe
Copy link
Member Author

mkoeppe commented Jun 20, 2021

Dependencies: #32009

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 20, 2021

Changed commit from 759309b to bd199dc

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 20, 2021

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

5d5e7baChart: Make it a Map
8ba174cEliminate direct use of Chart._domain
e6070fdMerge #32009
bd199dcWIP Chart as a Map

@mkoeppe
Copy link
Member Author

mkoeppe commented Jun 20, 2021

Changed dependencies from #32009 to #31901, #32009

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 24, 2021

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

27433ccChart: Use WithEqualityById

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 24, 2021

Changed commit from bd199dc to 27433cc

@mkoeppe
Copy link
Member Author

mkoeppe commented Jul 3, 2021

Work Issues: redo on top of #32116

@mkoeppe
Copy link
Member Author

mkoeppe commented Jul 3, 2021

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

@mkoeppe
Copy link
Member Author

mkoeppe commented Jul 12, 2021

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

@mkoeppe
Copy link
Member Author

mkoeppe commented Jul 12, 2021

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

@mkoeppe
Copy link
Member Author

mkoeppe commented Jul 12, 2021

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 12, 2021

Changed commit from 27433cc to 9626b54

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 12, 2021

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

0e63ff1Merge #32116
f66e4bcMerge #32116
7c003dbChart.__init__: Restore default of calc_method argument for consistency
f85e710Chart: in the description of the argument coord_restrictions, replace all instances of 'restrictions' by 'coord_restrictions'
80f6195Chart, RealChart: In class docstring, order arguments as they appear in __classcall__/__init__
a39e6fcDiffChart, RealDiffChart: In class docstring, order arguments as they appear in __classcall__/__init__; add description of argument coord_restrictions
cdf20b0TopologicalManifold.chart: Add description of argument coord_restrictions
741fd2eTopologicalManifold.chart: Add an example of using coord_restrictions
141ccb5Merge #32009
9626b54Merge #32102

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 12, 2021

Changed commit from 9626b54 to ce33546

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 12, 2021

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

4e7020aTopologicalManifold._Hom_: Handle other categories
ac60cc5Chart._restrict_set: Fix for empty inputs, set/frozenset inputs
ce33546Chart: Make it a Map

@mkoeppe
Copy link
Member Author

mkoeppe commented Jul 12, 2021

Author: Matthias Koeppe

@mkoeppe
Copy link
Member Author

mkoeppe commented Jul 12, 2021

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

This comment has been minimized.

@tscrim
Copy link
Collaborator

tscrim commented Jul 12, 2021

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. :/

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 13, 2021

Changed commit from ce33546 to 728c3df

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 13, 2021

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

fff2a79src/sage/sets/set.py: Fix docstring markup
1ceafd0Merge #32013
c89c697Merge #32102
451f5cfSets.ParentMethods: Update doctest
f135a05Merge #32015
728c3dfMerge #32089

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 13, 2021

Changed commit from 728c3df to 9d19b21

@mjungmath
Copy link

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
Copy link
Member

comment:38

Replying to @mjungmath:

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

+1

@egourgoulhon
Copy link
Member

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
Copy link
Member

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
Copy link
Member Author

mkoeppe commented Jul 18, 2021

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)

@mkoeppe
Copy link
Member Author

mkoeppe commented Jul 18, 2021

comment:42

Should ManifoldPoint.add_coordinates canonicalize the coordinates?
Or should ManifoldPoint.coordinates do this?

@mjungmath
Copy link

comment:45

Replying to @egourgoulhon:

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.

I'm sorry. What is the punchline here?

@mjungmath
Copy link

comment:46

Just forget what I just said. It didn't make sense. Sorry.

@mjungmath
Copy link

comment:47

Replying to @egourgoulhon:

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

What do you mean?

What we can do is that the user has to state the fundamental domain (which currently happens up to boundary only). Or we provide which boundary component belongs to the fundamental domain (I opt for the lower bound). Then it is clear which points the section map has to map to.

For the 1-sphere this could be for example the clopen interval [-pi,pi). Then it becomes clear that the point on the 1-sphere determined by pi is mapped to -pi via the (section) chart. Similarly then for any other point.

The only obstruction I see right now is that the symbolic ring doesn't provide any modulo operation in the spirit of x - y*floor(x/y). Or does it?

@mkoeppe
Copy link
Member Author

mkoeppe commented Jul 18, 2021

comment:48

Replying to @mjungmath:

the symbolic ring doesn't provide any modulo operation in the spirit of x - y*floor(x/y).

That's right - this is #25644

@mkoeppe
Copy link
Member Author

mkoeppe commented Jul 19, 2021

comment:49

Setting a new milestone for this ticket based on a cursory review.

@mkoeppe mkoeppe modified the milestones: sage-9.4, sage-9.5 Jul 19, 2021
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 19, 2021

Changed commit from c21f884 to 89039b2

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 19, 2021

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

ea08261Merge #32009
3d1c44dMerge tag '9.4.beta5' into t/32116/chart__parse_coordinates
cf3abdaMerge #32116
c218828Merge #32116
89039b2Merge #32102

@egourgoulhon
Copy link
Member

comment:51

Replying to @mjungmath:

Replying to @egourgoulhon:

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.

I'm sorry. What is the punchline here?

Periodic charts are useful in practice. Otherwise, we could have lived without them, sticking to the standard definition of a chart on a manifold.

@mjungmath
Copy link

comment:52

I guess then, this ticket depends on #25644...

@mjungmath
Copy link

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

@mjungmath
Copy link

comment:54

Replying to @egourgoulhon:

I'm sorry. What is the punchline here?

Periodic charts are useful in practice. Otherwise, we could have lived without them, sticking to the standard definition of a chart on a manifold.

Thank you. It's clear now. It was my fault yesterday...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants