Opened 3 years ago
Closed 3 years ago
#25438 closed enhancement (fixed)
Geometry of pseudo-Riemannian submanifolds
Reported by: | gh-FlorentinJ | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-8.3 |
Component: | geometry | Keywords: | submanifold, embedding, immersion, manifold, foliation |
Cc: | egourgoulhon | Merged in: | |
Authors: | Florentin Jaffredo | Reviewers: | Eric Gourgoulhon |
Report Upstream: | N/A | Work issues: | |
Branch: | 9477287 (Commits) | Commit: | 9477287c713213a28c526071e8b2485dd654daed |
Dependencies: | #25164,#25417 | Stopgaps: |
Description
This ticket is the logical continuation of #25164
It implements various calculus methods on pseudo-riemannian submanifolds (for example: normal vector, first/second fundamental form, gauss/principal/mean curvature...).
Some methods are also used exclusively in the case a foliation is provided (shift, lapse, ...).
Using these functions, one can for example easily perform n+1 decomposition of any tensorfield.
This is part of the SageManifolds project; see the metaticket #18528 for an overview.
Change History (20)
comment:1 Changed 3 years ago by
- Commit changed from 4a0881619f7a7c2926e37707e2549c9bec263351 to 4834f1cd49e7fa344d8e7a6226218691c7665630
comment:2 Changed 3 years ago by
- Commit changed from 4834f1cd49e7fa344d8e7a6226218691c7665630 to 65ae3ff1028d11b03ab5847cc787810be48723c1
Branch pushed to git repo; I updated commit sha1. New commits:
65ae3ff | Update to 8.3.beta3
|
comment:3 Changed 3 years ago by
- Summary changed from Calculus in embedded submanifolds to Geometry of pseudo-Riemannian submanifolds
comment:4 Changed 3 years ago by
- Status changed from new to needs_review
comment:5 Changed 3 years ago by
- Status changed from needs_review to needs_work
Have you noticed the errors reported by the patchbot (2 doctests + various plugins)?
To understand what the plugins check presicely, see the docstrings in the source file plugins.py. For instance, you will see that \overrightarrow
, which is a valid LaTeX instruction, triggers the plugin foreign_latex
...
comment:6 Changed 3 years ago by
- Commit changed from 65ae3ff1028d11b03ab5847cc787810be48723c1 to 2d6fd8e6048f7d6858508fc94f513b842f50cb20
comment:7 Changed 3 years ago by
- Status changed from needs_work to needs_review
comment:8 Changed 3 years ago by
- Status changed from needs_review to needs_work
This is a nice enhancement regarding differential geometry!
The code is carefully written and documented. The main issue is the long time of some doctests, for instance:
File "src/sage/manifolds/differentiable/pseudo_riemannian_submanifold.py", line 156, in sage.manifolds.differentiable.pseudo_riemannian_submanifold Warning, slow doctest: N.extrinsic_curvature()[:] # long time Test ran for 462.97 s
This is far too long for a single doctest. On my computer, there are 7 doctests with a time above 50 s. This increases too much the total time for doctesting all Sage library. Probably the solution would be to decrease the dimensionality of your examples, i.e. consider hypersurfaces embedded in a 3-dimensional manifold instead of a 4-dimensional one.
Another remark: the documentation generated for the method ambient_extrinsic_curvature
appears to be that of the decorator @cached_method
, because of the alias
ambient_extrinsic_curvature = cached_method(ambient_second_fundamental_form)
in line 974 of pseudo_riemannian_submanifold.py
. Same issue for extrinsic_curvature
, induced_metric
, ambient_induced_metric
.
comment:9 Changed 3 years ago by
- Reviewers set to Eric Gourgoulhon
comment:10 Changed 3 years ago by
- Commit changed from 2d6fd8e6048f7d6858508fc94f513b842f50cb20 to e5a290d0f1f934820de3ac39bd70096de2dacbd7
Branch pushed to git repo; I updated commit sha1. New commits:
e5a290d | Shorten some doctests, fix documentation of alias of cached_methods
|
comment:11 Changed 3 years ago by
I went from 4D Minkowsky to 3D Minkowsky in the first example, which resulted in a total test time of about 4 minutes on my laptop:
---------------------------------------------------------------------- All tests passed! ---------------------------------------------------------------------- Total time for all tests: 247.5 seconds cpu time: 269.6 seconds cumulative wall time: 247.4 seconds
Further reducing this time would mean further reducing the dimension, but 1D foliation of 2D manifold are not very interesting.
comment:12 Changed 3 years ago by
- Status changed from needs_work to needs_review
comment:13 Changed 3 years ago by
- Commit changed from e5a290d0f1f934820de3ac39bd70096de2dacbd7 to 069014498ed40a83a388da83431ca10f30505a5f
Branch pushed to git repo; I updated commit sha1. New commits:
0690144 | Reviewer tweaks regarding geometry of pseudo-Riemannian submanifolds
|
comment:14 Changed 3 years ago by
In the above "reviewer tweaks", I've mostly amended the documentation. The main change is the removal of the @cached_method
decorator on methods like first_fundamental_form
and the removal of the duplicated code in methods like induced_metric
. I understand that this was introduced to address the documentation issue related to @cached_method
mentioned in comment:8. In the new setting, the caching is performed "manually", by checking whether the attribute self._first_fundamental_form
is None
or not, and the alias is properly defined as
extrinsic_curvature = second_fundamental_form
This avoids the duplication of doctests. As a result, the doctest time for the file pseudo_riemannian_submanifold.py
decreases from 315 s to 248 s (on my computer). However this is still too much! A solution would be to replace most (all?) the sphere examples by simpler ones, like those for a cylinder, or even by those for a plane, in cases where they are not trivial (e.g. for the normal).
Another thing to do is to distinguish which methods are valid for a generic submanifold, like first_fundamental_form
or projector
, and those that are valid only for hypersurfaces, like second_fundamental_form
or normal
.
I am leaving the ticket status to "needs review" to catch the attention of the patchbot...
comment:15 Changed 3 years ago by
- Status changed from needs_review to needs_work
Now that one patchbot has done its job... (it's reporting only a failure of the commit_messages
plugin, which looks a false report to me, since all commits involved in this ticket do have a commit message...)
comment:16 Changed 3 years ago by
- Commit changed from 069014498ed40a83a388da83431ca10f30505a5f to b2fabf7c892ac4f28492d0979b9b251d88f1ca0c
comment:17 Changed 3 years ago by
- Commit changed from b2fabf7c892ac4f28492d0979b9b251d88f1ca0c to 9477287c713213a28c526071e8b2485dd654daed
Branch pushed to git repo; I updated commit sha1. New commits:
9477287 | Raise exception when an hypersurface is needed, normal can now return a multivector
|
comment:18 Changed 3 years ago by
- Status changed from needs_work to needs_review
comment:19 Changed 3 years ago by
- Status changed from needs_review to positive_review
Thanks for the changes. LGTM. Thanks for this nice piece of work!
comment:20 Changed 3 years ago by
- Branch changed from public/manifolds/submanifolds_calc to 9477287c713213a28c526071e8b2485dd654daed
- Resolution set to fixed
- Status changed from positive_review to closed
Branch pushed to git repo; I updated commit sha1. New commits:
Added mixed_projection
Added doctests
Merge branch 'develop' into calculus
Merge branch 'public/manifolds/submanifolds' of git://trac.sagemath.org/sage into Sage 8.3.beta2
Reviewer tweaks for submanifolds
Merge ticket 25164 into calculus
Fix error in mixed projection