Opened 7 weeks ago

Closed 6 weeks ago

#30519 closed defect (fixed)

Fix check of orientation in volume form

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

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 7 weeks ago by egourgoulhon

  • Branch set to public/manifolds/fix_check_orientation-30519
  • Commit set to 6229d65246a7d058f85caed87d7912f536930ae5

New commits:

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

comment:2 Changed 7 weeks ago by egourgoulhon

  • Cc gh-mjungmath tscrim added
  • Status changed from new to needs_review

comment:3 follow-ups: Changed 7 weeks ago by gh-mjungmath

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 7 weeks ago by tscrim

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 7 weeks ago by git

  • Commit changed from 6229d65246a7d058f85caed87d7912f536930ae5 to 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 7 weeks ago by egourgoulhon

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 ; follow-up: Changed 7 weeks ago by egourgoulhon

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 7 weeks ago by gh-mjungmath

  • Status changed from needs_review to positive_review

Perfect. Thank you.

LGTM.

comment:9 Changed 7 weeks ago by mkoeppe

  • Reviewers set to Michael Jung

comment:10 Changed 7 weeks ago by mkoeppe

  • Priority changed from major to critical

comment:11 Changed 7 weeks ago by egourgoulhon

Thank you for the review!

comment:12 in reply to: ↑ 7 Changed 7 weeks ago by egourgoulhon

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 6 weeks ago by vbraun

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