#28344 closed defect (fixed)

Fix some issues with submanifolds and improve their documentation

Reported by: egourgoulhon Owned by:
Priority: major Milestone: sage-8.9
Component: geometry Keywords: manifold, submanifold
Cc: gh-FlorentinJ, tscrim Merged in:
Authors: Eric Gourgoulhon Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: 10d5853 (Commits) Commit: 10d5853c00a8d2da510e19ff795a430a06d2c0d5
Dependencies: Stopgaps:

Description

When constructing a submanifold with Sage 8.8, some options are not taken into account:

sage: M = Manifold(3, 'M')
sage: S = Manifold(2, 'S', ambient=M, latex_name=r'\Sigma', start_index=1)
sage: latex(S)
S
sage: S.start_index()
0

Moreover the start_index parameter is not handled properly in some computational methods of class PseudoRiemannianSubmanifold, namely shift() and second_fundamental_form().

Another issue regards the generation of adapted coordinates in method TopologicalSubmanifold.adapted_chart(): if the ambient manifold contains a caret in its name, like E^3, the generated variable names in SR.var() are not valid Python identifiers.

This ticket fixes the above issues and improves significantly the documentation of submanifolds.

Change History (8)

comment:1 Changed 16 months ago by egourgoulhon

  • Branch set to public/manifolds/submanifolds_some_fixes
  • Commit set to a7843a4207df3445db70d5be7e23fa8b469b0a12

New commits:

9fcee93Fix various issues with submanifolds and improve their documentation
46ad0f9Improve _repr_ of submanifolds + doc
a7843a4Improve documentation of submanifolds

comment:2 Changed 16 months ago by egourgoulhon

  • Cc gh-FlorentinJ tscrim added
  • Status changed from new to needs_review

comment:3 follow-up: Changed 16 months ago by tscrim

Some comments:

Missing linebreak:

            raise NotImplementedError("mixed_projection() is implemented only "                          "for hypersurfaces")

Occurs 3 times.

I would probably write this as \cdots since it is a multiplication operation

-            n = \vec{*}(\mathrm{d}x_0\wedge\mathrm{d}x_1\wedge...
+            n = \vec{*}(\mathrm{d}x_0\wedge\mathrm{d}x_1\wedge\ldots
             \wedge\mathrm{d}x_{n-1})

but I know some mathematicians have a convention that the only thing that is \cdots is strictly usual multiplication \cdot.

Dollar signs snuck in here:

-One can then define the embedding $\phi_t$::
+One can then define the embedding `\phi_t`::
-and the partial inverse expressing the foliation parameter $t$ as a scalar
+and the partial inverse expressing the foliation parameter `t` as a scalar

Missing commas after the ... (which you might consider replacing with \ldots:

-        `(x_1,...,x_n,t_1,...t_{m-n})` on `M` such that `(x_1,...x_n)` are
-        coordinates on `N` and `(t_1,...t_{m-n})` are the `m-n` free parameters
+        `(x_1,...,x_n,t_1,...,t_{m-n})` on `M` such that `(x_1,...x_n)` are
+        coordinates on `N` and `(t_1,...,t_{m-n})` are the `m-n` free parameters
-          the names of the coordinates `(x_1,...x_n)` and of the parameters
-          `(t_1,...t_{m-n})`. If ``None``, ``"_" + self.ambient()._name`` is
+          the names of the coordinates `(x_1,...,x_n)` and of the parameters
+          `(t_1,...,t_{m-n})`. If ``None``, ``"_" + self.ambient()._name`` is
           used
         - ``latex_postscript`` -- (default: ``None``) string defining the LaTeX
           name of the coordinates of the adapated chart. This string will be
-          appended to the LaTeX names of the coordinates `(x_1,...x_n)` and of
-          the parameters `(t_1,...t_{m-n})`, If ``None``,
+          appended to the LaTeX names of the coordinates `(x_1,...,x_n)` and of
+          the parameters `(t_1,...,t_{m-n})`, If ``None``,

comment:4 Changed 16 months ago by git

  • Commit changed from a7843a4207df3445db70d5be7e23fa8b469b0a12 to 10d5853c00a8d2da510e19ff795a430a06d2c0d5

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

76ee621Merge branch 'public/manifolds/submanifolds_some_fixes' of git://trac.sagemath.org/sage into Sage 8.9.beta6.
10d5853A few corrections for submanifolds

comment:5 in reply to: ↑ 3 Changed 16 months ago by egourgoulhon

Replying to tscrim:

Some comments:

Thanks for yours comments (and prompt reply!). The above commit takes them into account.

comment:6 Changed 16 months ago by tscrim

  • Reviewers set to Travis Scrimshaw
  • Status changed from needs_review to positive_review

Thank you.

comment:7 Changed 16 months ago by egourgoulhon

Many thanks for the review!

comment:8 Changed 16 months ago by vbraun

  • Branch changed from public/manifolds/submanifolds_some_fixes to 10d5853c00a8d2da510e19ff795a430a06d2c0d5
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.