Opened 2 years ago

Closed 2 years ago

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

Reported by: Owned by: egourgoulhon major sage-8.9 geometry submanifolds, pseudo-Riemannian, second_fundamental_form tscrim, 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 2 years ago by egourgoulhon

• Description modified (diff)

### comment:2 Changed 2 years ago by egourgoulhon

• Branch set to public/manifolds/bug_second_fund_form-28462
• Commit set to 7f96e90c78b57358f7e72e7e887df5c6c42e40bd

New commits:

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

### comment:3 Changed 2 years ago by egourgoulhon

• Status changed from new to needs_review

### comment:4 follow-up: ↓ 5 Changed 2 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 2 years ago by egourgoulhon

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 in not specified in `matrix()`). This is the case of the doctests.

Version 0, edited 2 years ago by egourgoulhon (next)

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

• Status changed from needs_review to positive_review

### comment:7 follow-up: ↓ 8 Changed 2 years ago by egourgoulhon

• Status changed from positive_review to 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 2 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 2 years ago by git

• Commit changed from 7f96e90c78b57358f7e72e7e887df5c6c42e40bd to f8153780899df2a1c936b1c368e8adb85e0b562d

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 2 years ago by egourgoulhon

`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 2 years ago by egourgoulhon

• Status changed from needs_work to needs_review

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

• Status changed from needs_review to positive_review

### comment:13 Changed 2 years ago by egourgoulhon

• Reviewers set to Florentin Jaffredo

Thanks for the review!

### comment:14 Changed 2 years ago by vbraun

• Branch changed from public/manifolds/bug_second_fund_form-28462 to f8153780899df2a1c936b1c368e8adb85e0b562d
• Resolution set to fixed
• Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.