Opened 4 years ago
Last modified 4 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, GitHub, GitLab) | Commit: | f494678c9733967e0054169761dbff78ee4e9adc |
Dependencies: | 0 | Stopgaps: |
Description (last modified by )
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 4 years ago by
- Commit changed from 52ea82102a5949b7f63f946ffd6f4a0ca34b21b8 to 7dfaad99c70403bbe39f9a483aabf07caecafc86
comment:2 Changed 4 years ago by
- Status changed from new to needs_review
comment:3 Changed 4 years ago by
- Commit changed from 7dfaad99c70403bbe39f9a483aabf07caecafc86 to 188a1c318a9e3d41b8b31a3c5a50c11e1df6089c
Branch pushed to git repo; I updated commit sha1. New commits:
188a1c3 | Enter the component of a metric while declaring the metric on a Parallelizable Manifold
|
comment:4 Changed 4 years ago by
- 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 4 years ago by
- Commit changed from 188a1c318a9e3d41b8b31a3c5a50c11e1df6089c to 33edd0effe892b9810273f5246c6ea3be2f055dc
Branch pushed to git repo; I updated commit sha1. New commits:
33edd0e | Enter the components of a metric while declaring this metric on a Parallelizable Manifold
|
comment:6 Changed 4 years ago by
- Commit changed from 33edd0effe892b9810273f5246c6ea3be2f055dc to 76e40c54d973dc8e4de40bf3dbd636e9d1359f5f
Branch pushed to git repo; I updated commit sha1. New commits:
76e40c5 | Enter the components of a metric while declaring this metric on a Parallelizable Manifold
|
comment:7 Changed 4 years ago by
- 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 4 years ago by
New commits:
52ea821 | Enter the component of a meric while declaring the metric on a Parallelizable Manifold
|
7dfaad9 | Enter components of a metric while creating the metric
|
188a1c3 | Enter the component of a metric while declaring the metric on a Parallelizable Manifold
|
33edd0e | Enter the components of a metric while declaring this metric on a Parallelizable Manifold
|
76e40c5 | Enter the components of a metric while declaring this metric on a Parallelizable Manifold
|
comment:9 Changed 4 years ago by
- Branch changed from public/manifolds/Metric to public/manifolds/MetricComponents
- Commit 76e40c54d973dc8e4de40bf3dbd636e9d1359f5f deleted
comment:10 Changed 4 years ago by
- Commit set to a406fe689953d5431fda8cb915c6ba0ba2d4fc62
Branch pushed to git repo; I updated commit sha1. New commits:
52ea821 | Enter the component of a meric while declaring the metric on a Parallelizable Manifold
|
7dfaad9 | Enter components of a metric while creating the metric
|
188a1c3 | Enter the component of a metric while declaring the metric on a Parallelizable Manifold
|
25cff3c | Enter the component of a metric while declaring the metric on a Parallelizable Manifold
|
a406fe6 | Solving some doctests errors
|
comment:11 Changed 4 years ago by
- Status changed from needs_work to needs_review
comment:12 Changed 4 years ago by
- 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 4 years ago by
Another point: you should fill the Description field of this ticket, to tell what it is about.
comment:14 Changed 4 years ago by
- Commit changed from a406fe689953d5431fda8cb915c6ba0ba2d4fc62 to a6071cbff670f43afdd8b5bc62e6c44fa62f2467
Branch pushed to git repo; I updated commit sha1. New commits:
a6071cb | Entering components of a metric while declaring the metric on a parallelizable manifold
|
comment:15 Changed 4 years ago by
- Description modified (diff)
- Status changed from needs_work to needs_review
comment:16 Changed 4 years ago by
- Commit changed from a6071cbff670f43afdd8b5bc62e6c44fa62f2467 to cae28a718d0cd7c9fe7b867855b84caaa6c83919
comment:17 Changed 4 years ago by
- 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
isNone
), this defaults to initializing the metric components todiag(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 argumentcomp=...
) coexist nicely - Since the argument
comp
is implemented only inPseudoRiemannianMetricParal.__init__
, there is a difference of behavior between parallelizable manifolds and non-parallelizable ones; the argumentcomp
has been added toVectorFieldModule.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. inVectorFieldFreeModule.metric
)
comment:18 Changed 4 years ago by
- Commit changed from cae28a718d0cd7c9fe7b867855b84caaa6c83919 to f494678c9733967e0054169761dbff78ee4e9adc
Branch pushed to git repo; I updated commit sha1. New commits:
f494678 | Entering the components of a metric while declaring
|
comment:19 Changed 4 years ago by
- 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 4 years ago by
- Status changed from needs_review to needs_work
There are various issues:
- The doctest time of
metric.py
is too long: on my computer, runningsage -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 replacingLorentzian metric
byRiemannian 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 ofsrc/sage/manifolds/differentiable/manifold.py
. On general grounds, please check that the documentation builds correctly, viasage -docbuild reference/manifolds html
and that the html output is OK, before submitting a ticket for review.
Branch pushed to git repo; I updated commit sha1. New commits:
Enter components of a metric while creating the metric