Move examples from sage.geometry.riemannian_manifolds to sage.manifolds

mkoeppe commented 3 years ago

... with deprecation

Depends on #32116 Depends on #32102

CC: @egourgoulhon @mjungmath @videlec

Component: manifolds

Author: Matthias Koeppe

Branch/Commit: u/mkoeppe/move_examples_from_sage_geometry_riemannian_manifolds_to_sage_manifolds @ 1b4fe44

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

mkoeppe commented 3 years ago

Dependencies: #32116

mkoeppe commented 3 years ago

Author: Matthias Koeppe

mkoeppe commented 3 years ago

Changed dependencies from #32116 to #32116, #32102

mkoeppe commented 3 years ago

Branch: u/mkoeppe/move_examples_from_sage_geometry_riemannian_manifolds_to_sage_manifolds

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

Commit: fcf62a6

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

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

fcf62a6 ParametrizedSurface3D: Initialize immersion, replace attribute 'equation' by lazy attribute

mkoeppe commented 3 years ago

comment:5

The next step would be to deprecate most of the methods

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

Changed commit from fcf62a6 to 1b4fe44

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

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

1b4fe44 Merge tag '9.5.beta1' into t/32228/move_examples_from_sage_geometry_riemannian_manifolds_to_sage_manifolds

mjungmath commented 3 years ago

comment:9

Do we want to fix the syntax-block errors and unused import in this ticket?

Coverage is likely more for another ticket.

egourgoulhon commented 3 years ago

comment:10

Thanks for this first move. However it is quite incomplete at this stage. Indeed, it makes ParametrizedSurface3D a derived class of PseudoRiemannianSubmanifold, which is very natural. But many of the methods, like first_fundamental_form, second_fundamental_form, point, plot, tangent_vector, etc. have their code left in place, so that they appear redefined with respect to the corresponding PseudoRiemannianSubmanifold methods with very different inputs and outputs. For instance, ParametrizedSurface3D.tangent_vector takes a tuple of coordinates and returns a tuple of vector components, while PseudoRiemannianSubmanifold.tangent_vector takes a manifold point and returns an element of the tangent space at that point. It would be quite disturbing for the end user that manifolds.Ellipsoid() behaves so differently from manifolds.Sphere(2). At first glance, all the methods implemented in ParametrizedSurface3D have their equivalent in PseudoRiemannianSubmanifold, but it requires some work to implement things properly. I guess the question is then: shall this be done in this ticket?

Also what about the deprecation? If we change the behavior of most methods to make them coincide with the manifold framework, we should leave the old code in src/sage/geometry/riemannian_manifolds (with a deprecation warning), shouldn't we? In doing so, during the deprecation period, we would have:

sage: C1 = manifolds.Catenoid()  # returns a manifold-type object
sage: C2 = surfaces.Catenoid()    # returns a (deprecated) old type of object

I am adding Vincent to Cc, since he took part in the initial implementation of ParametrizedSurface3D in #10132.

videlec commented 3 years ago

comment:11

If all methods from ParametrizedSurface3D have equivalent functionalities in PseudoRiemannianSubmanifold I would say that deprecation of the class makes sense. However, this could potentially breaks a lot of code in the future. For that reason, I would suggest to carefully document the "new way of doing". That is to say

in each method of ParametrizedSurface3D there should be an explanation of the equivalent PseudoRiemannianSubmanifold way of doing
for each relevant doctest in ParametrizedSurface3D, the same computation should be reproduced with a PseudoRiemannianSubmanifold object.

This is just my way of thinking. I will not work on the ticket and you are definitely free to take any decision you would find more appropriate.

egourgoulhon commented 3 years ago

comment:13

Replying to @videlec:

This is just my way of thinking. I will not work on the ticket and you are definitely free to take any decision you would find more appropriate.

Thanks for your feedback! On my side, this project is on my todo list, but with a zillion of other things. I hope to do it relatively soon, unless someone do it before.

sagemath / sage

Move examples from sage.geometry.riemannian_manifolds to sage.manifolds #32228