Opened 3 years ago

Last modified 3 years ago

#25457 needs_work enhancement

Enter the components of a metric while declaring

Reported by: Dicolevrai Owned by:
Priority: major Milestone: sage-8.3
Component: geometry Keywords: Manifold, Pseudo-Riemannian Metric, Degenerate Metric
Cc: egourgoulhon Merged in:
Authors: Hans Fotsing Tetsing Reviewers: Eric Gourgoulhon
Report Upstream: N/A Work issues:
Branch: public/manifolds/MetricComponents (Commits) Commit: f494678c9733967e0054169761dbff78ee4e9adc
Dependencies: 0 Stopgaps:

Description (last modified by Dicolevrai)

The main purpose of this ticket is to give the possibility of entering the components of a metric while declaring this metric on a manifold.

In this ticket, there is a method which determine the signature of a metric. So it is no more necessary to give the signature of your metric while declaring, Sage is able to compute it! Therefore Sage would give the truth class of the declaring metric: Riemannian metric or Lorentzian metric or pseudo-Riemannian metric or degenerate metric.

Change History (20)

comment:1 Changed 3 years ago by git

  • Commit changed from 52ea82102a5949b7f63f946ffd6f4a0ca34b21b8 to 7dfaad99c70403bbe39f9a483aabf07caecafc86

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

7dfaad9Enter components of a metric while creating the metric

comment:2 Changed 3 years ago by Dicolevrai

  • Status changed from new to needs_review

comment:3 Changed 3 years ago by git

  • Commit changed from 7dfaad99c70403bbe39f9a483aabf07caecafc86 to 188a1c318a9e3d41b8b31a3c5a50c11e1df6089c

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

188a1c3Enter the component of a metric while declaring the metric on a Parallelizable Manifold

comment:4 Changed 3 years ago by Dicolevrai

  • Summary changed from Enter the component of a metric while declaring the metric on a Parallelizable Manifold to Enter the components of a metric while declaring the metric on a Parallelizable Manifold

comment:5 Changed 3 years ago by git

  • Commit changed from 188a1c318a9e3d41b8b31a3c5a50c11e1df6089c to 33edd0effe892b9810273f5246c6ea3be2f055dc

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

33edd0eEnter the components of a metric while declaring this metric on a Parallelizable Manifold

comment:6 Changed 3 years ago by git

  • Commit changed from 33edd0effe892b9810273f5246c6ea3be2f055dc to 76e40c54d973dc8e4de40bf3dbd636e9d1359f5f

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

76e40c5Enter the components of a metric while declaring this metric on a Parallelizable Manifold

comment:7 Changed 3 years ago by egourgoulhon

  • Status changed from needs_review to needs_work

Thanks for this contribution!

As you can see on the patchbot report (which you can get by clicking on the yellow question mark at the top right and then on the "shortlog" items), there are many doctest failures, mostly due to the change in handling the metric signature. Could you fix this please? Before submitting the change, you can make sure that all doctests are passed by running

sage -tp --long src/sage/manifolds/

In case you need more information about the patchbot, see here.

comment:8 Changed 3 years ago by Dicolevrai

New commits:

52ea821Enter the component of a meric while declaring the metric on a Parallelizable Manifold
7dfaad9Enter components of a metric while creating the metric
188a1c3Enter the component of a metric while declaring the metric on a Parallelizable Manifold
33edd0eEnter the components of a metric while declaring this metric on a Parallelizable Manifold
76e40c5Enter the components of a metric while declaring this metric on a Parallelizable Manifold

comment:9 Changed 3 years ago by Dicolevrai

  • Branch changed from public/manifolds/Metric to public/manifolds/MetricComponents
  • Commit 76e40c54d973dc8e4de40bf3dbd636e9d1359f5f deleted

comment:10 Changed 3 years ago by git

  • Commit set to a406fe689953d5431fda8cb915c6ba0ba2d4fc62

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

52ea821Enter the component of a meric while declaring the metric on a Parallelizable Manifold
7dfaad9Enter components of a metric while creating the metric
188a1c3Enter the component of a metric while declaring the metric on a Parallelizable Manifold
25cff3cEnter the component of a metric while declaring the metric on a Parallelizable Manifold
a406fe6Solving some doctests errors

comment:11 Changed 3 years ago by Dicolevrai

  • Status changed from needs_work to needs_review

comment:12 Changed 3 years ago by egourgoulhon

  • Status changed from needs_review to needs_work

A doctest fails:

sage -t --long --warn-long 49.9 src/sage/manifolds/differentiable/metric.py
**********************************************************************
File "src/sage/manifolds/differentiable/metric.py", line 1006, in sage.manifolds.differentiable.metric.PseudoRiemannianMetric.riemann
Failed example:
    g.riemann()[:]
Expected:
    [[[[0, 0], [0, 0]], [[0, sin(th)^2], [-sin(th)^2, 0]]],
    [[[0, (cos(th)^2 - 1)/sin(th)^2], [1, 0]], [[0, 0], [0, 0]]]]
Got:
    [[[[0, 0], [0, 0]], [[0, sin(th)^2], [-sin(th)^2, 0]]],
     [[[0, -1], [1, 0]], [[0, 0], [0, 0]]]]
**********************************************************************
1 item had failures:
   1 of  15 in sage.manifolds.differentiable.metric.PseudoRiemannianMetric.riemann
    [534 tests, 1 failure, 86.67 s]

Besides, the following doctest in line 168-170 of src/sage/manifolds/differentiable/pseudo_riemannian.py:

sage: g = M.metric()
sage: g
Riemannian metric g on the 4-dimensional Lorentzian manifold M

is not correct: the output should be Lorentzian metric (as it was prior to this ticket), since M has been declared in line 158 as being a Lorentzian manifold.

Similarly, in lines 566-567 of the same file, the output of the doctest

sage: h = M.metric('h', signature=1); h
Riemannian metric h on the 3-dimensional Riemannian manifold M

is not correct, since a signature of 1 on a 3-dimensional manifold implies a Lorentzian metric.

In the helper function _diag defined in line 1908 of src/sage/manifolds/differentiable/metric.py, it is quite unconventional to name the first argument self since _diag is not a class method.

You should not end docstring lines by a backslash character, as done in lines 1976-1982 of metric.py.

The documentation fails to build: the command

./sage -docbuild reference/manifolds html

returns errors like

docstring of sage.manifolds.differentiable.metric.PseudoRiemannianMetricParal.index:7: 
WARNING: Bullet list ends without a blank line; unexpected unindent.

comment:13 Changed 3 years ago by egourgoulhon

Another point: you should fill the Description field of this ticket, to tell what it is about.

comment:14 Changed 3 years ago by git

  • Commit changed from a406fe689953d5431fda8cb915c6ba0ba2d4fc62 to a6071cbff670f43afdd8b5bc62e6c44fa62f2467

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

a6071cbEntering components of a metric while declaring the metric on a parallelizable manifold

comment:15 Changed 3 years ago by Dicolevrai

  • Description modified (diff)
  • Status changed from needs_work to needs_review

comment:16 Changed 3 years ago by git

  • Commit changed from a6071cbff670f43afdd8b5bc62e6c44fa62f2467 to cae28a718d0cd7c9fe7b867855b84caaa6c83919

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

8ddc1b2Merge branch 'public/manifolds/MetricComponents' of git://trac.sagemath.org/sage into Sage 8.3.beta4
cae28a7Reviewer tweaks for #25457

comment:17 Changed 3 years ago by egourgoulhon

  • Status changed from needs_review to needs_work

I've pushed to the ticket branch a commit with small tweaks, which

  • correct errors in the documentation building
  • correct the doctest failure mentioned in comment:12 (trigonometric simplification)
  • apply some coding style along the recommended PEP 8 conventions, in particular: "The preferred way of wrapping long lines is by using Python's implied line continuation inside parentheses, brackets and braces. Long lines can be broken over multiple lines by wrapping expressions in parentheses. These should be used in preference to using a backslash for line continuation."

To see all my changes, click on cae28a7 in comment:16.

There are still some points that must be discussed or fixed:

  • At the moment, if no metric components are provided at some metric declaration (i.e. if comp is None), this defaults to initializing the metric components to diag(1,1,...,1). It would be preferable to left the components uninitialized instead, especially if the default chart is not of Cartesian type. I think we can make both behaviors (the previous one, with no components preinitialized, and the new one, with components set via the argument comp=...) coexist nicely
  • Since the argument comp is implemented only in PseudoRiemannianMetricParal.__init__, there is a difference of behavior between parallelizable manifolds and non-parallelizable ones; the argument comp has been added to VectorFieldModule.metric but is totally unused there.
  • Even on parallelizable manifolds, what if more than one chart is required to cover the manifold, like for instance S1 or S3 ?
  • The issue with the naming of the first argument of _diag mentioned in comment:12 is not addressed
  • The argument comp should be documented everywhere it has been added (e.g. in VectorFieldFreeModule.metric)

comment:18 Changed 3 years ago by git

  • Commit changed from cae28a718d0cd7c9fe7b867855b84caaa6c83919 to f494678c9733967e0054169761dbff78ee4e9adc

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

f494678Entering the components of a metric while declaring

comment:19 Changed 3 years ago by Dicolevrai

  • Authors changed from Hans FOTSING TETSING to Hans Fotsing Tetsing
  • Description modified (diff)
  • Keywords Parallelizable removed
  • Status changed from needs_work to needs_review
  • Summary changed from Enter the components of a metric while declaring the metric on a Parallelizable Manifold to Enter the components of a metric while declaring

comment:20 Changed 3 years ago by egourgoulhon

  • Status changed from needs_review to needs_work

There are various issues:

  • The doctest time of metric.py is too long: on my computer, running
    sage -t --long src/sage/manifolds/differentiable/metric.py
    

yields a total CPU time of 144.2 s, while with Sage 8.3.rc0 (i.e. outside the ticket branch) it is only of 62.3 s. Such an increase of doctest time is not acceptable, especially for a new functionality, which in principle, does not require much computing. One can see the culprits by running

sage -t --long --warn-long 5 src/sage/manifolds/differentiable/metric.py

which reports the doctests running for more than 5 s. They are the doctests involving a metric on the sphere, with the components given in 6 charts. Why do you choose such a lengthy example? It is also very long, and thus not pleasant to read, in the html documentation.

  • In line 726 of levi_civita_connection.py, you have changed the output of the doctest by replacing Lorentzian metric by Riemannian metric (so that the doctest is passed), but this not correct since the metric has been declared as Lorentzian a few lines above. Actually this reveals that the handling of metric signatures must be entirely rethinked after the introduction of your function for computing the signature (previously, there was only the concept of "user declared signature"). This is out of the scope of this ticket and I will open a new ticket for this.
  • The method sign() should return a tuple, and not a list, since the signature is intended to be immutable.
  • The documentation does not build, due to a spurious u99 inserted in line 3216 of src/sage/manifolds/differentiable/manifold.py. On general grounds, please check that the documentation builds correctly, via
    sage -docbuild reference/manifolds html
    

and that the html output is OK, before submitting a ticket for review.

Note: See TracTickets for help on using tickets.