Opened 3 years ago

Closed 3 years ago

# Bug in computing the second fundamental form of a Riemannian submanifold

Reported by: Owned by: Eric Gourgoulhon major sage-8.9 geometry submanifolds, pseudo-Riemannian, second_fundamental_form Travis Scrimshaw, gh-FlorentinJ Eric Gourgoulhon Florentin Jaffredo N/A f815378 f8153780899df2a1c936b1c368e8adb85e0b562d

As reported in this ask.sagemath question, we have currently (Sage 8.8 and 8.9.beta9):

```sage: P = Manifold(3, 'P', structure='Riemannian')
sage: Q = Manifold(2, 'Q', ambient=P, structure='Riemannian')
sage: CP.<x,y,z> = P.chart()
sage: CQ.<u,v> = Q.chart()
sage: g = P.metric()
sage: c = 2/(1 + y^2 + z^2)
sage: g[0,0], g[1,1], g[2,2] = 1, c^2, c^2
sage: phi = Q.diff_map(P, (u+v, u, v))
sage: phi_inv = P.diff_map(Q, (y, z))
sage: Q.set_embedding(phi, inverse=phi_inv)
sage: Q.second_fundamental_form()
TypeError: unable to convert 1/2*sqrt(2)*(u^4 + v^4 + 2*(u^2 + 1)*v^2 + 2*u^2 + 1)*y/
(sqrt(u^4 + v^4 + 2*(u^2 + 1)*v^2 + 2*u^2 + 3)*(y^2 + z^2 + 1))
+ 1/2*sqrt(2)*(u^4 + v^4 + 2*(u^2 + 1)*v^2 + 2*u^2 + 1)*z/
(sqrt(u^4 + v^4 + 2*(u^2 + 1)*v^2 + 2*u^2 + 3)*(y^2 + z^2 + 1))
to an integer
```

This results from the mention of the ring `SR` missing in the declaration of a matrix involved in the computation.

The issue is fixed by this ticket.

### comment:1 Changed 3 years ago by Eric Gourgoulhon

Description: modified (diff)

### comment:2 Changed 3 years ago by Eric Gourgoulhon

Branch: → public/manifolds/bug_second_fund_form-28462 → 7f96e90c78b57358f7e72e7e887df5c6c42e40bd

New commits:

 ​7f96e90 `Fix bug in computation of second fundamental form`

### comment:3 Changed 3 years ago by Eric Gourgoulhon

Cc: Travis Scrimshaw gh-FlorentinJ added new → needs_review

### comment:4 follow-up:  5 Changed 3 years ago by gh-FlorentinJ

I completely agree with the changes, but why was there no errors in the doctests ? The example in `second_fundamental_form` features a symbolic computation, and the result was not an integer.

### comment:5 in reply to:  4 Changed 3 years ago by Eric Gourgoulhon

I completely agree with the changes, but why was there no errors in the doctests ? The example in `second_fundamental_form` features a symbolic computation, and the result was not an integer.

Actually, the error does not show off when the coefficients of the ambient connection are zero, since then `gamma_n` in line 1079 is filled with zeros, which is compatible with a matrix declared on the integers (the default, when the ring is not specified in `matrix()`). This is the case of the doctests.

Last edited 3 years ago by Eric Gourgoulhon (previous) (diff)

### comment:6 Changed 3 years ago by gh-FlorentinJ

Status: needs_review → positive_review

### comment:7 follow-up:  8 Changed 3 years ago by Eric Gourgoulhon

Status: positive_review → needs_work

Argh, there is another issue: I wanted to add the following doctest to check that the original issue is solved:

```            sage: M = Manifold(2, 'M', structure='Riemannian')
sage: N = Manifold(1, 'N', ambient=M, structure='Riemannian',
....:              start_index=1)
sage: CM.<x,y> = M.chart()
sage: CN.<u> = N.chart()
sage: g = M.metric()
sage: g[0, 0], g[1, 1] = 1, 1/(1 + y^2)^2
sage: phi = N.diff_map(M, (u, u))
sage: N.set_embedding(phi)
sage: N.second_fundamental_form()
Field of symmetric bilinear forms K on the 1-dimensional Riemannian
submanifold N embedded in the 2-dimensional Riemannian manifold M
sage: N.second_fundamental_form().display()
K = 2*sqrt(u^4 + 2*u^2 + 2)*(2*u*y^2 - (u^2 + 1)*y + 2*u)
/(u^6 + 3*u^4 + (u^6 + 3*u^4 + 4*u^2 + 2)*y^2 + 4*u^2 + 2) du*du
```

As you can see, the expression of the component K_{uu} involves `y`, while it should be a function of `u` only. I guess this is because the expression of the ambient connection coefficients returned by `expr()` in line 1098 involves `(x,y)`; `x` and `y` should be expressed in terms of `u` via the embedding formulas before continuing the computation.

### comment:8 in reply to:  7 ; follow-up:  10 Changed 3 years ago by gh-FlorentinJ

`x` and `y` should be expressed in terms of `u` via the embedding formulas before continuing the >computation.

So I guess `phi_inv` was really needed...

### comment:9 Changed 3 years ago by git

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

 ​f815378 `Fix another issue with second fundatemental form of pseudo-Riemannian submanifolds`

### comment:10 in reply to:  8 Changed 3 years ago by Eric Gourgoulhon

`x` and `y` should be expressed in terms of `u` via the embedding formulas before continuing the >computation.

So I guess `phi_inv` was really needed...

No it was not. It was only a matter of using `phi` to substitute `(x,y)` by their expressions in terms of `u`. This is done in the above commit. I've also added a simplification of the result by a call to `chart.simplify()`. I think that everything is fixed now.

### comment:11 Changed 3 years ago by Eric Gourgoulhon

Status: needs_work → needs_review

### comment:12 Changed 3 years ago by gh-FlorentinJ

Status: needs_review → positive_review

### comment:13 Changed 3 years ago by Eric Gourgoulhon

Reviewers: → Florentin Jaffredo

Thanks for the review!

### comment:14 Changed 3 years ago by Volker Braun

Branch: public/manifolds/bug_second_fund_form-28462 → f8153780899df2a1c936b1c368e8adb85e0b562d → fixed positive_review → closed
Note: See TracTickets for help on using tickets.