Add example of projective space to manifold catalog

[Trevor Karn]

Add projective space and charts in the manifold catalog.

See #31249.

[Trevor Karn]

Still has an issue with the domain of transition maps causing one test to fail.

---

New commits:

​af760c9

Add projective space

[Michael Jung]

For now this approach is all right. I am curious at which dimension this implementation becomes problematic in view of computational times.

[git]

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

​38f2be2

Fix inverse map and tests, and restrict to real projective space

[git]

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

​bdc0441

Fix PEP8 compliance

[Travis Scrimshaw]

Some trivial comments:

Why not use gens()?

Also, this should be split over 2 lines*

Ci, Cj = P.atlas()[i], P.atlas()[j]

I feel like the more standard latex would be to make it (blackboard?) bold. However, we should match what happens elsewhere, such as for Euclidean space and spheres. (This is more of a bikeshedding comment.) Should this also have a normal name being passed too?

[Trevor Karn]

Replying to tscrim:

Some trivial comments:

Why not use gens()?

It appears that no such method exists for charts - am I missing something?

sage: P = manifolds.RealProjectiveSpace(3)
sage: A = P.atlas(); A
[Chart (U0, (x_1, x_2, x_3)),
 Chart (U1, (x_0, x_2, x_3)),
 Chart (U2, (x_0, x_1, x_3)),
 Chart (U3, (x_0, x_1, x_2)),
 Chart (U0_inter_U1, (x_1, x_2, x_3)),
 Chart (U0_inter_U1, (x_0, x_2, x_3)),
 Chart (U0_inter_U2, (x_1, x_2, x_3)),
 Chart (U0_inter_U2, (x_0, x_1, x_3)),
 Chart (U0_inter_U3, (x_1, x_2, x_3)),
 Chart (U0_inter_U3, (x_0, x_1, x_2)),
 Chart (U1_inter_U2, (x_0, x_2, x_3)),
 Chart (U1_inter_U2, (x_0, x_1, x_3)),
 Chart (U1_inter_U3, (x_0, x_2, x_3)),
 Chart (U1_inter_U3, (x_0, x_1, x_2)),
 Chart (U2_inter_U3, (x_0, x_1, x_3)),
 Chart (U2_inter_U3, (x_0, x_1, x_2))]
sage: A[0].gens()
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-3-eff7e167b774> in <module>
----> 1 A[Integer(0)].gens()

AttributeError: 'RealChart' object has no attribute 'gens'

Should this also have a normal name being passed too?

I was thinking that it has the normal name "P", unless I misunderstand what you meant by "normal name"

[git]

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

​11cf10d

Add reviewer suggestions

[Trevor Karn]

Replying to tscrim:

I feel like the more standard latex would be to make it (blackboard?) bold. However, we should match what happens elsewhere, such as for Euclidean space and spheres.

Spheres do appear to use \mathbb (see ​https://sagemanifolds.obspm.fr/examples/pdf/SM_sphere_S2.pdf) so I adjusted accordingly, reflected in most recent commit.

[Eric Gourgoulhon]

Replying to tkarn:

Replying to tscrim:

I feel like the more standard latex would be to make it (blackboard?) bold. However, we should match what happens elsewhere, such as for Euclidean space and spheres.

Spheres do appear to use \mathbb (see ​https://sagemanifolds.obspm.fr/examples/pdf/SM_sphere_S2.pdf) so I adjusted accordingly, reflected in most recent commit.

Indeed, at the moment, we have

sage: latex(EuclideanSpace(2))
\mathbb{E}^{2}
sage: latex(manifolds.Sphere(3))
\mathbb{S}^{3}

So please use \mathbb{P} for projective spaces as well.

PS: there is some inconsistency with the above in

sage: latex(manifolds.RealLine())
\Bold{R}

This should probably be fixed/discussed in another ticket. I guess this was to be consistent with

sage: latex(RR)
\Bold{R}

but this may not be necessary, because after all RR and manifolds.RealLine() are two different beasts and having a distinction in the LaTeX output might not be a bad thing. Also in the long term, IMHO we should get rid of this \Bold extra command, which unnecessarily pollutes the Jupyter notebooks.

[Eric Gourgoulhon]

BTW, thanks for having opened this ticket! I've added it to the metaticket #30525. Feel free to perform such an addition yourself next time you are opening a ticket connected to manifolds. It helps to keep track of what's going on and to prepare the ​release tour.

[Trevor Karn]

Replying to egourgoulhon:

BTW, thanks for having opened this ticket! I've added it to the metaticket #30525. Feel free to perform such an addition yourself next time you are opening a ticket connected to manifolds. It helps to keep track of what's going on and to prepare the ​release tour.

Will do! And thanks for the advice on latex representation.

[Travis Scrimshaw]

Ah right, that was done as a workaround to get the X.<a,b> = M.chart() preparser syntax working. You should just use X[:] instead (which is what _first_ngens() does).

For the standard name, I meant pass a name="RP{}".format(dim). However, as you said, it is already implicitly there with the unnamed "P" input. I had missed that, sorry. For consistency, I think it should follow the latex name.

You also need {} for the latex in case the dimension is at least 10, so latex_name=r"\mathbb{{RP}}^{{{}}}".format(dim).

I would use a nested for loop instead of combinations:

A = P.atlas()
for i in range(dim+1):
    Ci = A[i]
    gi = Ci[:]
    for j in range(i,dim+1):
        Cj = A[j]
        gj = Cj[:]

and so on. Although being slightly clever, you can combine this with the previous for loop for creating the charts (this might not be the best option in terms of code presentation; only in terms of optimization). I also might not rely on atlas being in a particular order either and instead just store the charts you make in a list you make.

It will also be faster if you explicitly set the inverse rather than having Sage compute it (and you can skip the checks too in the final version).

It might be good to also give a latex name to the subsets of U_{i}.

[git]

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

​7b6dbf0	Make reviewer changes except for optimizing chart creation
​e5be551	Optimize creation of open subsets and charts into a single loop

[Trevor Karn]

Replying to tscrim: Thanks for all the suggestions! They are incorporated in the latest commit.

[Travis Scrimshaw]

Other than the fact that this is using the real numbers, making the documentation incorrect (there is no ``field``), I have no further comments. Anyone else?

[git]

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

​f18aa29

Fix documentation and field import

[Eric Gourgoulhon]

Replying to tscrim:

I have no further comments. Anyone else?

A minor typo in the documentation, which prevents the display of dim:

-    - ``P`` -- the projective space `RP^d` where `d =```dim``.
+    - ``P`` -- the projective space `RP^d` where `d =``dim``.

[Trevor Karn]

Replying to egourgoulhon:

Replying to tscrim:

I have no further comments. Anyone else?

A minor typo in the documentation, which prevents the display of dim:

-    - ``P`` -- the projective space `RP^d` where `d =```dim``.
+    - ``P`` -- the projective space `RP^d` where `d =``dim``.

Thanks for the catch. I am hoping that d= is in math mode and dim is in fixed-width font. Will adding a space between = and dim

`d=` ``dim``

work for that?

[Eric Gourgoulhon]

Replying to tkarn:

Thanks for the catch. I am hoping that d= is in math mode and dim is in fixed-width font.

Ah yes, I see.

Will adding a space between = and dim

`d=` ``dim``

work for that?

I would say yes, but Sphinx is sometimes touchy. You can try by yourself: implement the change and run

sage -b
sage -docbuild reference/manifolds html

This is faster than running make doc.

[git]

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

​0eb3b73

Fix documentation by adding space

[Trevor Karn]

Replying to egourgoulhon:

Replying to tkarn:

Thanks for the catch. I am hoping that d= is in math mode and dim is in fixed-width font.

Ah yes, I see.

Will adding a space between = and dim

`d=` ``dim``

work for that?

I would say yes, but Sphinx is sometimes touchy. You can try by yourself: implement the change and run

sage -b
sage -docbuild reference/manifolds html

This is faster than running make doc.

Adding the space does work, and is reflected in the most recent commit. Thanks for the tip on doc building.

[Travis Scrimshaw]

Instead of using \Bold{R}, you can just use the Sage-specific macro \RR (good for uniformity and easy if we decide to change our minds).

[git]

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

​316dd9e

\Bold -> \RR

[Trevor Karn]

Replying to tscrim:

Instead of using \Bold{R}, you can just use the Sage-specific macro \RR (good for uniformity and easy if we decide to change our minds).

Fixed! Thanks!

[git]

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

​51f9419

Make RP bold

[Travis Scrimshaw]

In regards to comment:2, this seems to be reasonably fast up to dimension 10 (I didn't check further):

sage: P4 = manifolds.RealProjectiveSpace(4)
sage: %time P = manifolds.RealProjectiveSpace(3)
CPU times: user 448 ms, sys: 16 ms, total: 464 ms
Wall time: 387 ms
sage: %time P = manifolds.RealProjectiveSpace(4)
CPU times: user 888 ms, sys: 24 ms, total: 912 ms
Wall time: 778 ms
sage: %time P = manifolds.RealProjectiveSpace(5)
CPU times: user 1.53 s, sys: 44 ms, total: 1.57 s
Wall time: 1.35 s
sage: %time P = manifolds.RealProjectiveSpace(6)
CPU times: user 2.47 s, sys: 56 ms, total: 2.53 s
Wall time: 2.17 s
sage: %time P = manifolds.RealProjectiveSpace(7)
CPU times: user 3.81 s, sys: 112 ms, total: 3.92 s
Wall time: 3.38 s
sage: %time P = manifolds.RealProjectiveSpace(8)
CPU times: user 5.44 s, sys: 128 ms, total: 5.56 s
Wall time: 4.78 s
sage: %time P = manifolds.RealProjectiveSpace(9)
CPU times: user 7.54 s, sys: 168 ms, total: 7.71 s
Wall time: 6.6 s
sage: %time P = manifolds.RealProjectiveSpace(10)
CPU times: user 10.2 s, sys: 224 ms, total: 10.4 s
Wall time: 8.95 s

This will be a good placeholder until projective space has its own class.

[Eric Gourgoulhon]

Now that the ticket branch has been merged in Sage 9.6.beta1, there remains a last thing to do: preparing some examples of use for the ​Sage 9.6 release tour, on the same footing as what has been done for Sage 9.5: ​https://wiki.sagemath.org/ReleaseTours/sage-9.5#Manifolds.

[Eric Gourgoulhon]

Replying to egourgoulhon:

Now that the ticket branch has been merged in Sage 9.6.beta1, there remains a last thing to do: preparing some examples of use for the ​Sage 9.6 release tour, on the same footing as what has been done for Sage 9.5: ​https://wiki.sagemath.org/ReleaseTours/sage-9.5#Manifolds.

Ping (since the release of Sage 9.6 seems to be close).

[Trevor Karn]

Thanks for the ping. Just took care of it! ​https://wiki.sagemath.org/ReleaseTours/sage-9.6#Projective_spaces

[Eric Gourgoulhon]

Replying to tkarn:

Thanks for the ping. Just took care of it! ​https://wiki.sagemath.org/ReleaseTours/sage-9.6#Projective_spaces

Thanks a lot!