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:  sage9.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: 
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 2sphere 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: 2dimensional 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 2sphere 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
Branch:  → public/manifolds/fix_check_orientation30519 

Commit:  → 6229d65246a7d058f85caed87d7912f536930ae5 
comment:2 Changed 2 years ago by
Cc:  Michael Jung Travis Scrimshaw added 

Status:  new → needs_review 
comment:3 followups: 6 7 Changed 2 years ago by
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): 646 646 sage: uv_to_xy = xy_to_uv.inverse() 647 647 sage: W = U.intersection(V) # The complement of the two poles 648 648 sage: eU = c_xy.frame() ; eV = c_uv.frame() 649 sage: M.set_orientation([eU, eV]) # make M orientable 649 650 sage: g = M.metric('g') 650 651 sage: g[eU,1,1], g[eU,2,2] = 4/(1+x^2+y^2)^2, 4/(1+x^2+y^2)^2 651 652 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 Changed 2 years ago by
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 2sphere 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 bysage: 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
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 Changed 2 years ago by
Replying to ghmjungmath:
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 followup: 12 Changed 2 years ago by
Replying to ghmjungmath:
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:9 Changed 2 years ago by
Reviewers:  → Michael Jung 

comment:10 Changed 2 years ago by
Priority:  major → critical 

comment:12 Changed 2 years ago by
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
Branch:  public/manifolds/fix_check_orientation30519 → e7c9d04759a03c6088fab9e43343f6349d521182 

Resolution:  → fixed 
Status:  positive_review → closed 
New commits:
Fix check of orientation in volume form (Trac #30519)