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

Circle doesn't have an orientation #31324

Open
tobiasdiez opened this issue Feb 2, 2021 · 34 comments
Open

Circle doesn't have an orientation #31324

tobiasdiez opened this issue Feb 2, 2021 · 34 comments

Comments

@tobiasdiez
Copy link
Contributor

The circle S1 with polar coordinates doesn't specify an orientation at the moment. It is also not possible to call set_orientation with the polar chart as argument, since this throws an error that the chart doesn't cover the whole manifold.

So I guess to fix this one would need to introduce a set of polar coordinates to get a bona-fide atlas.

CC: @tscrim @nthiery @mjungmath @egourgoulhon @mkoeppe

Component: manifolds

Author: Michael Jung

Branch/Commit: public/31324 @ 4358faf

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

@tobiasdiez tobiasdiez added this to the sage-9.3 milestone Feb 2, 2021
@egourgoulhon
Copy link
Member

comment:1

Indeed, by default S1 is endowed with a single chart and without any orientation:

sage: S1 = manifolds.Sphere(1) 
sage: S1.atlas()                                                                                    
[Chart (A, (phi,))]
sage: S1.orientation()                                                                              
[]

I think this is bad; the minimal atlas shall contain enough charts to cover the entire manifold. There is the same issue with all the spheres.
One has to invoke stereographic coordinates to get some orientation:

sage: S1.stereographic_coordinates()                                                                
Chart (S^1-{NP}, (y1,))
sage: S1.atlas()                                                                                    
[Chart (A, (phi,)),
 Chart (S^1-{NP}, (y1,)),
 Chart (S^1-{SP}, (yp1,)),
 Chart (S^1-{NP,SP}, (y1,)),
 Chart (S^1-{NP,SP}, (yp1,)),
 Chart (A, (y1,)),
 Chart (A, (yp1,))]
sage: S1.orientation()                                                                              
[Coordinate frame (S^1-{SP}, (d/dyp1)), Vector frame (S^1-{NP}, (f_1))]

@mjungmath
Copy link

comment:2

This is on purpose! The orientation must cover the entire manifold, otherwise a non-orientable manifold could mistakenly seen as orientable. Take for example the Moebius strip.

Polar coordinates do not yield an atlas for S^1, there is a slit missing.

So, this behavior is perfectly fine.

If you want to use polar coordinates for orientation stuff, you must restrict everything to the open subset A, i.e. S^n without a slit.

@mjungmath
Copy link

comment:3

As pointed out, to "fix" this (imho this is not a bug), one has to introduce an additional chart which covers the missing slit. This, however, would slow down the initialization of spheres again, especially in higher dimensions, why I would vote against it.

@egourgoulhon
Copy link
Member

comment:4

Replying to @mjungmath:

This is on purpose! The orientation must cover the entire manifold,

Yes, this is my point. Shouldn't the spheres be initialized with a minimal atlas covering them entirely? At least, for low dimension, due to the CPU cost.

@mjungmath
Copy link

comment:5

Replying to @egourgoulhon:

Replying to @mjungmath:

This is on purpose! The orientation must cover the entire manifold,

Yes, this is my point. Shouldn't the spheres be initialized with a minimal atlas covering them entirely? At least, for low dimension, due to the CPU cost.

Right. But one could argue that the topological data is still there in terms of stereographic coordinates (in contrast to a self-defined manifold with polar coordinates).

If one desires a complete atlas at initialization, I'd suggest to introduce some kind of 'lazy atlas' which knows all charts but computes them only on demand. That would be similar to Travis' idea for Grassmannians, see #31249, and in fact a nice generalization for demanding examples. And similar for the orientation.

@tobiasdiez
Copy link
Contributor Author

comment:6

Replying to @mjungmath:

This is on purpose! The orientation must cover the entire manifold, otherwise a non-orientable manifold could mistakenly seen as orientable. Take for example the Moebius strip.

Strictly speaking, it suffices to specify the orientation (i.e. a norm-one vector field) only on a dense subspace. But I guess it's probably impossible to check if a subset is dense in sage.

Polar coordinates do not yield an atlas for S^1, there is a slit missing.

That's a bit the problem. With the current default atlas, S^1 is diffeomorphic to R, which of course it not correct (or in other words, what is currently returned by S1.atlas is not an atlas of S^1). I cannot say much about the implementation/performance issues, but isn't the transition map to the other polar chart not just a translation by pi?.

@mjungmath
Copy link

comment:7

Replying to @tobiasdiez:

Strictly speaking, it suffices to specify the orientation (i.e. a norm-one vector field) only on a dense subspace. But I guess it's probably impossible to check if a subset is dense in sage.

The (open) Moebius strip is a counter-example. When you cut the Moebius strip, the cut has measure zero, but the remainder is orientable.

That's a bit the problem. With the current default atlas, S^1 is diffeomorphic to R, which of course it not correct (or in other words, what is currently returned by S1.atlas is not an atlas of S^1). I cannot say much about the implementation/performance issues, but isn't the transition map to the other polar chart not just a translation by pi?.

That's not true. The polar coordinates are defined only on A, which is a real subset of S^1. A is diffeomorphic to R, S^1 is still not.

@egourgoulhon
Copy link
Member

comment:8

Replying to @mjungmath:

That's a bit the problem. With the current default atlas, S1 is diffeomorphic to R, which of course it not correct (or in other words, what is currently returned by S1.atlas is not an atlas of S1). I cannot say much about the implementation/performance issues, but isn't the transition map to the other polar chart not just a translation by pi?.

That's not true. The polar coordinates are defined only on A, which is a real subset of S^1. A is diffeomorphic to R, S^1 is still not.

Anyway, it would be good, at least for pedagogical purposes, that objects returned by manifolds.Sphere(n) come with a minimal atlas that covers the whole manifold, e.g. an atlas with the two stereographic charts. I understand that there is a performance issue for n large. To circumvent this, we could add an option like complete_atlas=True, so that if some user does not want to construct the stereographic charts and their transition maps when initializing the manifold, he/she could set this option to False.

@tobiasdiez
Copy link
Contributor Author

comment:9

Replying to @mjungmath:

Replying to @tobiasdiez:

Strictly speaking, it suffices to specify the orientation (i.e. a norm-one vector field) only on a dense subspace. But I guess it's probably impossible to check if a subset is dense in sage.

The (open) Moebius strip is a counter-example. When you cut the Moebius strip, the cut has measure zero, but the remainder is orientable.

That's only a counterexample to the (wrong) statement that a manifold is orientable iff if it is orientable on dense subset. However, to specify an orientation it is enough to give a unit-norm vector field on a dense subset (such that the vector field extends continuously to the whole manifold).

@mjungmath
Copy link

comment:10

By the way, what do you mean with "unit-norm vector field". One vector field alone is not enough to determine an orientation, you need an actual frame, i.e. a bunch of point-wise linearly independent vector fields (if you wish they can have unit norm).

Even then, the statement seems wrong to me. Assume, your dense set consists of different connected components (for example, make two cuts in the torus). Then you can choose any orientation on each component which might not constitute an orientation on the whole manifold.

@tobiasdiez
Copy link
Contributor Author

comment:11

For an orientation you only need to specify what is outward pointing. That's the job of the unit-norm vector field X; the opposite orientation is given by -X.

In abstract terms, the orientation bundle is a principal bundle with structure group GL / GL_+ = Z_2. And a continuous section of this bundle is an orientation. This can be a unit-norm vector field or a non-vanishing differential form. Moreover, a continuous map is determined by its values on a dense subset (but of course not every map on a dense subset extends to the whole).

@mjungmath
Copy link

comment:12

Replying to @tobiasdiez:

For an orientation you only need to specify what is outward pointing. That's the job of the unit-norm vector field X; the opposite orientation is given by -X.

If I understand you correctly here, you are talking about submanifolds with codimension 1. But that is a very special case. The implementation of orientations is entirely intrinsic.

@mjungmath
Copy link

comment:13

And yes, one could use a globally defined (non-vanishing) volume form instead. But that works not on purely topologial manifolds. Besides, at the current stage of implementation, such a form must always be stated in terms of frames, from which we already have to know what orientation they have.

@mjungmath
Copy link

comment:14

Replying to @egourgoulhon:

Replying to @mjungmath:

That's a bit the problem. With the current default atlas, S1 is diffeomorphic to R, which of course it not correct (or in other words, what is currently returned by S1.atlas is not an atlas of S1). I cannot say much about the implementation/performance issues, but isn't the transition map to the other polar chart not just a translation by pi?.

That's not true. The polar coordinates are defined only on A, which is a real subset of S^1. A is diffeomorphic to R, S^1 is still not.

Anyway, it would be good, at least for pedagogical purposes, that objects returned by manifolds.Sphere(n) come with a minimal atlas that covers the whole manifold, e.g. an atlas with the two stereographic charts. I understand that there is a performance issue for n large. To circumvent this, we could add an option like complete_atlas=True, so that if some user does not want to construct the stereographic charts and their transition maps when initializing the manifold, he/she could set this option to False.

For the records, 'large' already means n ≥ 4. I think, the initialization should run fast enough at least for n≤7.

The current implementation is, imho, a good compromise between performance and accuracy. Like I said, the topological data is not lost and can easily be retrieved. Besides,

sage: S1 = manifolds.Sphere(1, coordinates='stereographic')

initializes the sphere with stereographic coordinates and has therefore the same effect as your suggestion.

I don't see a need for a change here; apart from making stereographic coordinates the standard set of charts, but due to performance issues I'd not recommend it.

By the way, the intention is not to get the charts by using the atlas but rather calling stereographic_coordinates() etc, which then constructs the charts (and orientation!) on demand.

Travis, what do you think?

@mjungmath
Copy link

comment:15

However, I agree. The least we should do is to document that behavior, i.e. saying that polar coordinates don't constitute a complete atlas.

@tscrim
Copy link
Collaborator

tscrim commented Feb 7, 2021

comment:16

S1 is a bit special since you can use a periodic coordinate chart to uniquely specify the coordinates. So we can special case that (although this technically is not a chart I believe). Otherwise, I think we should document it stating the polar coordinates are not a complete atlas.

However, for stereographic coordinates, I would think with knowing all of the formulas explicitly, it shouldn't be too expensive to initialize it. Perhaps you need a mechanism to avoid trying to simplify expressions or something?

@mjungmath
Copy link

comment:17

Replying to @tscrim:

S1 is a bit special since you can use a periodic coordinate chart to uniquely specify the coordinates. So we can special case that (although this technically is not a chart I believe). Otherwise, I think we should document it stating the polar coordinates are not a complete atlas.

I agree, this should be documented. I'll push some changes here, soon.

However, for stereographic coordinates, I would think with knowing all of the formulas explicitly, it shouldn't be too expensive to initialize it. Perhaps you need a mechanism to avoid trying to simplify expressions or something?

You are welcome to optimize my code. It's not unlikely that I overlooked potential optimizations.

@mjungmath
Copy link

Branch: public/31324

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 14, 2021

Commit: e83733d

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 14, 2021

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

e83733dTrac #31324: documentation on atlas behavior of spherical coordinates

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 14, 2021

Changed commit from e83733d to 4358faf

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 14, 2021

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

4358fafTrac #31324: documentation on atlas behavior of spherical coordinates

@mjungmath
Copy link

Author: Michael Jung

@mjungmath
Copy link

comment:22

For now, I'd say we leave it with a warning. I pushed the changes.

@tscrim
Copy link
Collaborator

tscrim commented Feb 16, 2021

comment:23

I think it is worthwhile special-casing the S1 case since we do have the periodic charts. Is there an argument against this?

@mjungmath
Copy link

comment:24

Yes, we cannot make an exception for S^1 because this requires a complete refactoring of the current orientation code. However, I wouldn't recommend it since orientations must always cover the whole manifold.

Alternatively, one could define the domain of the periodic chart being the whole S^1, but that is just mathematically wrong. It would suggest that S^1 could be covered by one chart, which is extremely wrong. And since Sage is also used as teaching tool, I would strongly advice against it.

@tscrim
Copy link
Collaborator

tscrim commented Feb 16, 2021

comment:25

Replying to @mjungmath:

Yes, we cannot make an exception for S^1 because this requires a complete refactoring of the current orientation code. However, I wouldn't recommend it since orientations must always cover the whole manifold.

I don't think that is an argument because we can cover S1 by a single chart with the current implementation, the periodic chart [0, 1). The tricky bit is this is not a chart in the mathematical sense (as far as I am aware), but it is still a single chart here.

Alternatively, one could define the domain of the periodic chart being the whole S^1, but that is just mathematically wrong. It would suggest that S^1 could be covered by one chart, which is extremely wrong. And since Sage is also used as teaching tool, I would strongly advice against it.

As we both seem to agree, this is a problem with the implementation differing from the mathematics. So we need to rectify the two. Implicitly, I believe we are thinking the periodic chart is 2 charts pieced together in a natural way with an implicit orientation. So it would make sense IMO to use that single periodic chart to define the orientation. Otherwise you will need to strip out the abuse, which will make doing research with the code harder and not worth it.

S1 is also special as you don't have to miss 2 points with the polar coordinates, unless you want a disconnected manifold. Because of its special topology compared to higher dimensional spheres, there is plenty of justification for making it behave differently, even if that means returning the stereographic coordinates as the default.

@tobiasdiez
Copy link
Contributor Author

comment:26

I'm bit out of the loop, but where is the problem in specifying a correct atlas using two charts in spherical coordinates? So for S^1, on the first chart \phi \in (0, 2 \pi) and on the second chart \phi \in (-\pi, \pi). This is a proper atlas whose transition function is adding pi. That would be mathematically correct and also easy to implement.

@egourgoulhon
Copy link
Member

comment:27

Replying to @tobiasdiez:

I'm bit out of the loop, but where is the problem in specifying a correct atlas using two charts in spherical coordinates? So for S^1, on the first chart \phi \in (0, 2 \pi) and on the second chart \phi \in (-\pi, \pi). This is a proper atlas whose transition function is adding pi. That would be mathematically correct and also easy to implement.

Yes, this seems the thing to do, probably by introducing a subclass of Sphere. Subclassing Sphere seems required anyway for the parallelisable spheres, i.e. S1, S3 and S7.

@mjungmath
Copy link

comment:28

Replying to @tscrim:

Implicitly, I believe we are thinking the periodic chart is 2 charts pieced together...

I like that way of thinking. But that should definitely be made clear to the user. With that argument in mind, I would feel more comfortable to refactor the domain of that chart to the whole sphere for the special case S^1 (and then it can be stated as orientation). But clearly saying that this does not give a chart in the mathematical sense, but should rather be thought of two charts glued together. This would also be in line with the examples presented for periodic charts in the charts documentation where the chart covers the whole manifold. In the same instance I would suggest to add a similar comment to the charts documentation where period charts are introduced.

S1 is also special as you don't have to miss 2 points with the polar coordinates, unless you want a disconnected manifold. Because of its special topology compared to higher dimensional spheres, there is plenty of justification for making it behave differently, even if that means returning the stereographic coordinates as the default.

Even for higher dimension, you never tear that sphere apart. The missing point just becomes a missing slit. If you allow periodicity, you only miss (the) poles.

@mkoeppe
Copy link
Contributor

mkoeppe commented Mar 15, 2021

comment:29

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

@mkoeppe mkoeppe modified the milestones: sage-9.3, sage-9.4 Mar 15, 2021
@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 19, 2021

comment:30

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
@mjungmath
Copy link

comment:31

See also #31894.

@mkoeppe
Copy link
Contributor

mkoeppe commented Dec 18, 2021

comment:32

Stalled in needs_review or needs_info; likely won't make it into Sage 9.5.

@mkoeppe mkoeppe modified the milestones: sage-9.5, sage-9.6 Dec 18, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.6, sage-9.7 Apr 2, 2022
@mkoeppe mkoeppe modified the milestones: sage-9.7, sage-9.8 Sep 19, 2022
@mkoeppe mkoeppe removed this from the sage-9.8 milestone Jan 29, 2023
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

5 participants