Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#30320 closed defect (fixed)

set_default_frame() too restrictive on parallelizable manifolds

Reported by: Michael Jung Owned by:
Priority: major Milestone: sage-9.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:

Status badges

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 3-dimensional 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 Michael Jung

I see two options here:

  1. One allows also frames defined on subsets for set_default_frame, even in the parallelizable case.
  2. One overwrites the default frame for the first frame which covers M in case it haven't covered M before.

comment:2 Changed 2 years ago by Michael Jung

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.

Last edited 2 years ago by Michael Jung (previous) (diff)

comment:3 Changed 2 years ago by Eric Gourgoulhon

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 S3. 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 in reply to:  3 Changed 2 years ago by Michael Jung

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 S3. 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...

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 Michael Jung

Branch: u/gh-mjungmath/parallelizable_manifold_not_covered_by_one_chart_and_default_frame

comment:6 Changed 2 years ago by Michael Jung

Authors: Michael Jung
Commit: 7f50b3d5ff7786fc6a7874a8b28314a1fdfeca02
Status: newneeds_review

New commits:

7f50b3dTrac #30320: error messages removed

comment:7 Changed 2 years ago by Michael Jung

Similarly for charts, btw, though more unlikely. I've removed that error message, too. Agreed?

Last edited 2 years ago by Michael Jung (previous) (diff)

comment:8 Changed 2 years ago by Matthias Köppe

Milestone: sage-9.2sage-9.3

comment:9 Changed 2 years ago by Michael Jung

Is that good or some more work needed here?

comment:10 in reply to:  9 ; Changed 2 years ago by Eric Gourgoulhon

Reviewers: Eric Gourgoulhon
Status: needs_reviewpositive_review
Summary: Parallelizable Manifold not covered by one Chart and default_frameset_default_frame() too restrictive on parallelizable manifolds

Replying to gh-mjungmath:

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 in reply to:  10 ; Changed 2 years ago by Eric Gourgoulhon

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 in reply to:  11 ; Changed 2 years ago by Michael Jung

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 in reply to:  12 Changed 2 years ago by Eric Gourgoulhon

Replying to gh-mjungmath:

Besides, you meant the metaticket #30525, right?

Yes.

comment:14 Changed 2 years ago by Volker Braun

Branch: u/gh-mjungmath/parallelizable_manifold_not_covered_by_one_chart_and_default_frame7f50b3d5ff7786fc6a7874a8b28314a1fdfeca02
Resolution: fixed
Status: positive_reviewclosed

comment:15 Changed 2 years ago by Matthias Köppe

Milestone: sage-9.3sage-9.2
Note: See TracTickets for help on using tickets.