Opened 2 years ago

Closed 2 years ago

# Fix check of orientation in volume form

Reported by: Owned by: Eric Gourgoulhon critical sage-9.2 manifolds orientation, volume_form Michael Jung, Travis Scrimshaw Eric Gourgoulhon Michael Jung N/A e7c9d04 e7c9d04759a03c6088fab9e43343f6349d521182

### Description

After the introduction of manifold orientation in #30178, attempting to get the volume form of a metric on a manifold without any predefined orientation yields a weird error message:

```sage: M = Manifold(2, 'M', structure='Riemannian')  # the 2-sphere
sage: U = M.open_subset('U'); V = M.open_subset('V')
sage: M.declare_union(U, V)
sage: c_xy.<x,y> = U.chart() # stereographic coord. from the North pole
sage: c_uv.<u,v> = V.chart() # stereographic coord. from the South pole
sage: xy_to_uv = c_xy.transition_map(c_uv, (x/(x^2+y^2), y/(x^2+y^2)),
....:                intersection_name='W', restrictions1= x^2+y^2!=0,
....:                restrictions2= u^2+v^2!=0)
sage: uv_to_xy = xy_to_uv.inverse()
sage: eU = c_xy.frame(); eV = c_uv.frame()
sage: g = M.metric()
sage: g[eU,0,0], g[eU,1,1] = 4/(1+x^2+y^2)^2, 4/(1+x^2+y^2)^2
sage: g.volume_form()
Traceback (most recent call last):
...
IndexError: list index out of range
```

This surprising error message (which list index?) is due to a bad check for an orientation in `PseudoRiemannianMetric.volume_form()`; the fix is

```-        if orient is None:
+        if not orient:
```

Indeed, the attribute `_orientation` of manifolds is initialized as `[]`, not as `None`. With the fix, the error message is more expressive:

```ValueError: 2-dimensional Riemannian manifold M must admit an orientation
```

In addition to the above fix, with a new doctest checking it, this ticket corrects some mistake in the documentation illustrating the choice of an orientation on the 2-sphere in the docstring of class `PseudoRiemannianMetric`:

```sage: M.set_orientation([c_xy, c_uv])
```

does not define a valid orientation because the stereographic frames from the North and South poles have opposite orientations, the Jacobian of the transition map `c_xy` --> `c_uv` being negative. It is replaced here by

```sage: f = U.vector_frame('f', (eU[2], eU[1]))
sage: M.set_orientation([eV, f])
```

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

Branch: → public/manifolds/fix_check_orientation-30519 → 6229d65246a7d058f85caed87d7912f536930ae5

New commits:

 ​6229d65 `Fix check of orientation in volume form (Trac #30519)`

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

Cc: Michael Jung Travis Scrimshaw added new → needs_review

### comment:3 follow-ups:  6  7 Changed 2 years ago by Michael Jung

Thank you so much for catching this bug!

Of course, the orientation changes when using stereographic projections! I just haven't thought about it in the first place, thank you for catching this, too.

This is indeed a nice improvement of the documentation as well.

---

Just one minor thing. I made the same mistake in `diff_form.py`:

• ## src/sage/manifolds/differentiable/diff_form.py

```diff --git a/src/sage/manifolds/differentiable/diff_form.py b/src/sage/manifolds/differentiable/diff_form.py
 a class DiffForm(TensorField): sage: uv_to_xy = xy_to_uv.inverse() sage: W = U.intersection(V) # The complement of the two poles sage: eU = c_xy.frame() ; eV = c_uv.frame() sage: M.set_orientation([eU, eV])  # make M orientable sage: g = M.metric('g') sage: g[eU,1,1], g[eU,2,2] = 4/(1+x^2+y^2)^2, 4/(1+x^2+y^2)^2 sage: g[eV,1,1], g[eV,2,2] = 4/(1+u^2+v^2)^2, 4/(1+u^2+v^2)^2

### comment:4 in reply to:  description Changed 2 years ago by Travis Scrimshaw

In addition to the above fix, with a new doctest checking it, this ticket corrects some mistake in the documentation illustrating the choice of an orientation on the 2-sphere in the docstring of class `PseudoRiemannianMetric`:

```sage: M.set_orientation([c_xy, c_uv])
```

does not define a valid orientation because the stereographic frames from the North and South poles have opposite orientations, the Jacobian of the transition map `c_xy` --> `c_uv` being negative. It is replaced here by

```sage: f = U.vector_frame('f', (eU[2], eU[1]))
sage: M.set_orientation([eV, f])
```

I recently noticed this while teaching my class as well and was going to tell you. Thank you for catching this and fixing it.

### comment:5 Changed 2 years ago by git

Commit: 6229d65246a7d058f85caed87d7912f536930ae5 → e7c9d04759a03c6088fab9e43343f6349d521182

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

 ​e7c9d04 `#30519: Correct orientation in a diff form example`

### comment:6 in reply to:  3 Changed 2 years ago by Eric Gourgoulhon

Just one minor thing. I made the same mistake in `diff_form.py`:

Thanks for pointing this out; it's corrected in the latest commit.

### comment:7 in reply to:  3 ; follow-up:  12 Changed 2 years ago by Eric Gourgoulhon

Thank you so much for catching this bug!

You're welcome. Thanks to you for having introduced orientations on manifolds. They were really missing!

Of course, the orientation changes when using stereographic projections! I just haven't thought about it in the first place,

No problem. The error was already there, when the volume form took the same shape, with the same sign, in the two stereographic frames. Same thing in the S^2 example notebook, which I am going to correct tomorrow.

### comment:8 Changed 2 years ago by Michael Jung

Status: needs_review → positive_review

Perfect. Thank you.

LGTM.

### comment:9 Changed 2 years ago by Matthias Köppe

Reviewers: → Michael Jung

### comment:10 Changed 2 years ago by Matthias Köppe

Priority: major → critical

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

Thank you for the review!