Opened 2 years ago

Closed 2 years ago

#30519 closed defect (fixed)

Fix check of orientation in volume form

Reported by: Eric Gourgoulhon Owned by:
Priority: critical Milestone: sage-9.2
Component: manifolds Keywords: orientation, volume_form
Cc: Michael Jung, Travis Scrimshaw Merged in:
Authors: Eric Gourgoulhon Reviewers: Michael Jung
Report Upstream: N/A Work issues:
Branch: e7c9d04 (Commits, GitHub, GitLab) Commit: e7c9d04759a03c6088fab9e43343f6349d521182
Dependencies: Stopgaps:

Status badges

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.add_comp_by_continuation(eV, U.intersection(V), chart=c_uv)
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])

Change History (13)

comment:1 Changed 2 years ago by Eric Gourgoulhon

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

New commits:

6229d65Fix check of orientation in volume form (Trac #30519)

comment:2 Changed 2 years ago by Eric Gourgoulhon

Cc: Michael Jung Travis Scrimshaw added
Status: newneeds_review

comment:3 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
    index 417183a..f0855ad 100644
    a b class DiffForm(TensorField): 
    646646            sage: uv_to_xy = xy_to_uv.inverse()
    647647            sage: W = U.intersection(V) # The complement of the two poles
    648648            sage: eU = c_xy.frame() ; eV = c_uv.frame()
     649            sage: M.set_orientation([eU, eV])  # make M orientable
    649650            sage: g = M.metric('g')
    650651            sage: g[eU,1,1], g[eU,2,2] = 4/(1+x^2+y^2)^2, 4/(1+x^2+y^2)^2
    651652            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

Replying to egourgoulhon:

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: 6229d65246a7d058f85caed87d7912f536930ae5e7c9d04759a03c6088fab9e43343f6349d521182

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

Replying to gh-mjungmath:

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 ; Changed 2 years ago by Eric Gourgoulhon

Replying to gh-mjungmath:

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_reviewpositive_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: majorcritical

comment:11 Changed 2 years ago by Eric Gourgoulhon

Thank you for the review!

comment:12 in reply to:  7 Changed 2 years ago by Eric Gourgoulhon

Replying to egourgoulhon:

Same thing in the S^2 example notebook, which I am going to correct tomorrow.

Done; see cells [221] and below.

comment:13 Changed 2 years ago by Volker Braun

Branch: public/manifolds/fix_check_orientation-30519e7c9d04759a03c6088fab9e43343f6349d521182
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.