#30320 closed defect (fixed)
set_default_frame() too restrictive on parallelizable manifolds
Reported by:  Michael Jung  Owned by:  

Priority:  major  Milestone:  sage9.2 
Component:  manifolds  Keywords:  
Cc:  Eric Gourgoulhon, Travis Scrimshaw  Merged in:  
Authors:  Michael Jung  Reviewers:  Eric Gourgoulhon 
Report Upstream:  N/A  Work issues:  
Branch:  7f50b3d (Commits, GitHub, GitLab)  Commit:  7f50b3d5ff7786fc6a7874a8b28314a1fdfeca02 
Dependencies:  Stopgaps: 
Description
sage: M = Manifold(3, 'M') sage: U = M.open_subset('U'); V = M.open_subset('V') sage: c_xyz.<x,y,z> = U.chart(); c_uvt.<u,v,t> = V.chart() sage: M.default_frame() Coordinate frame (U, (d/dx,d/dy,d/dz)) sage: e = M.vector_frame('e'); e Vector frame (M, (e_0,e_1,e_2)) sage: M.default_frame() Coordinate frame (U, (d/dx,d/dy,d/dz)) sage: M.set_default_frame(c_xyz.frame()) Traceback (most recent call last): ... ValueError: the frame domain must coincide with the 3dimensional differentiable manifold M
In case of M=SU(2)=S^3
this can happen: it cannot be covered by one chart but is parallelizable.
However, due to the error message, the method default_frame
is obviously intended to return the default frame which covers M
if possible. This is not given in the above example.
Change History (15)
comment:1 Changed 2 years ago by
comment:2 Changed 2 years ago by
I think, both options are reasonable. The first is, of course, more comprehensible to user.
I don't see a third choice. If you have any ideas, I'd welcome them.
comment:3 followup: 4 Changed 2 years ago by
Thanks for handling this. In all the sage/manifolds settings, the "default frame" is initially set as the first frame defined on the manifold, usually (but not necessarily) the coordinate frame corresponding to the first chart defined on the manifold. For consistency among all kinds of manifolds, I would be inclined to keep this behaviour even for parallelizable manifolds not covered by a a single chart, such as S^{3}. So I would vote for option 1 of comment:1. Moreover, although it is also quite natural, option 2 would perform a silent change of the default frame, which may surprise the end user.
In DifferentiableManifold.set_default_frame()
, there is
if frame._domain is not self: if self.is_manifestly_parallelizable(): raise ValueError("the frame domain must coincide with " + "the {}".format(self))
which triggers the error reported in the ticket description. Maybe we can simply remove this...
comment:4 Changed 2 years ago by
Replying to egourgoulhon:
Thanks for handling this. In all the sage/manifolds settings, the "default frame" is initially set as the first frame defined on the manifold, usually (but not necessarily) the coordinate frame corresponding to the first chart defined on the manifold. For consistency among all kinds of manifolds, I would be inclined to keep this behaviour even for parallelizable manifolds not covered by a a single chart, such as S^{3}. So I would vote for option 1 of comment:1. Moreover, although it is also quite natural, option 2 would perform a silent change of the default frame, which may surprise the end user.
In
DifferentiableManifold.set_default_frame()
, there isif frame._domain is not self: if self.is_manifestly_parallelizable(): raise ValueError("the frame domain must coincide with " + "the {}".format(self))which triggers the error reported in the ticket description. Maybe we can simply remove this...
This is exactly what I meant by "The first [option] is [...] more comprehensible to user.". Thanks for pointing this out in such detail.
Vector bundles currently use option 1 as well, btw.
Then, I will take care of removing the corresponding error messages and adapt the documentation if necessary. This is good, because #30178 can then be kept.
comment:5 Changed 2 years ago by
Branch:  → u/ghmjungmath/parallelizable_manifold_not_covered_by_one_chart_and_default_frame 

comment:6 Changed 2 years ago by
Authors:  → Michael Jung 

Commit:  → 7f50b3d5ff7786fc6a7874a8b28314a1fdfeca02 
Status:  new → needs_review 
New commits:
7f50b3d  Trac #30320: error messages removed

comment:7 Changed 2 years ago by
Similarly for charts, btw, though more unlikely. I've removed that error message, too. Agreed?
comment:8 Changed 2 years ago by
Milestone:  sage9.2 → sage9.3 

comment:10 followup: 11 Changed 2 years ago by
Reviewers:  → Eric Gourgoulhon 

Status:  needs_review → positive_review 
Summary:  Parallelizable Manifold not covered by one Chart and default_frame → set_default_frame() too restrictive on parallelizable manifolds 
Replying to ghmjungmath:
Is that good or some more work needed here?
Yes it's good. Sorry for the delay.
PS: I've added the ticket to #30235, where it was missing.
comment:11 followup: 12 Changed 2 years ago by
Replying to egourgoulhon:
PS: I've added the ticket to #30235, where it was missing.
Btw, don't hesitate to add relevant tickets there.
comment:12 followup: 13 Changed 2 years ago by
Replying to egourgoulhon:
Replying to egourgoulhon:
PS: I've added the ticket to #30235, where it was missing.
Btw, don't hesitate to add relevant tickets there.
Okay, sure. :)
Besides, you meant the metaticket #30525, right?
comment:13 Changed 2 years ago by
comment:14 Changed 2 years ago by
Branch:  u/ghmjungmath/parallelizable_manifold_not_covered_by_one_chart_and_default_frame → 7f50b3d5ff7786fc6a7874a8b28314a1fdfeca02 

Resolution:  → fixed 
Status:  positive_review → closed 
comment:15 Changed 2 years ago by
Milestone:  sage9.3 → sage9.2 

I see two options here:
set_default_frame
, even in the parallelizable case.M
in case it haven't coveredM
before.