Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pullback silently fails in some cases with multiple charts #31904

Closed
egourgoulhon opened this issue Jun 4, 2021 · 16 comments
Closed

Pullback silently fails in some cases with multiple charts #31904

egourgoulhon opened this issue Jun 4, 2021 · 16 comments

Comments

@egourgoulhon
Copy link
Member

In Sage 9.3, we have

sage: E.<x,y> = EuclideanSpace()
sage: polar.<r,ph> = E.polar_coordinates()
sage: g = E.metric()
sage: M = Manifold(1, 'M')
sage: Ct.<t> = M.chart()
sage: F = M.diff_map(E, coord_functions={(Ct, polar): (1 + cos(t), t)})
sage: gM = F.pullback(g)
sage: gM
Field of symmetric bilinear forms on the 1-dimensional differentiable
manifold M

So far so good, but

sage: gM.display()
ValueError: no basis could be found for computing the components in 
 the Coordinate frame (M, (d/dt)

Actually, gM has been initialized as a tensor field object, but its components have not been evaluated in any frame:

sage: gM._components
{}

Forcing the coordinate expression of the map F in the Cartesian chart (for instance by a call to F.display()) fixes the issue:

sage: F.display()
M --> E^2
   t |--> (x, y) = (cos(t)^2 + cos(t), (cos(t) + 1)*sin(t))
   t |--> (r, ph) = (cos(t) + 1, t)
sage: gM = F.pullback(g)
sage: gM.display()
(2*cos(t) + 2) dt*dt

However, the expression of F in Cartesian coordinates should not be required to compute the pullback of g since the latter is known in polar coordinates, where F has been defined:

sage: g.display(polar)
g = dr*dr + r^2 dph*dph

This bug has been reported at https://ask.sagemath.org/question/57431/

CC: @tscrim @mjungmath @mkoeppe

Component: manifolds

Keywords: pullback

Author: Eric Gourgoulhon

Branch/Commit: aea4554

Reviewer: Ricardo Buring

Issue created by migration from https://trac.sagemath.org/ticket/31904

@egourgoulhon egourgoulhon added this to the sage-9.4 milestone Jun 4, 2021
@egourgoulhon

This comment has been minimized.

@egourgoulhon

This comment has been minimized.

@egourgoulhon
Copy link
Member Author

New commits:

8455aabFix bug #31904 in pullback

@egourgoulhon
Copy link
Member Author

Author: Eric Gourgoulhon

@egourgoulhon
Copy link
Member Author

Commit: 8455aab

@egourgoulhon
Copy link
Member Author

Branch: public/manifolds/pullback_bug-31904

@egourgoulhon
Copy link
Member Author

comment:4

The fix consisted in making the internal function _pullback_chart of the method pullback to operate for a single pair of charts (now added as arguments), which is determined in the main part of pullback, based on the knowledge of the map's coordinate expressions.

@rburing
Copy link
Contributor

rburing commented Jun 4, 2021

comment:5

The return partial statement in the parallel code has seemingly accidentally been indented too far.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 5, 2021

Branch pushed to git repo; I updated commit sha1. New commits:

aea4554#31904: Fix indentation in _pullback_chart

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 5, 2021

Changed commit from 8455aab to aea4554

@egourgoulhon
Copy link
Member Author

comment:7

Replying to @rburing:

The return partial statement in the parallel code has seemingly accidentally been indented too far.

Good catch, thanks! (it was not revealed by the parallel doctest because local_list_ind had a single element in that case).
This is corrected in the above commit (as well as a pyflakes error reported by the patchbot).

@egourgoulhon
Copy link
Member Author

comment:8

Ricardo, do you agree with the last version?

@rburing
Copy link
Contributor

rburing commented Jun 17, 2021

Reviewer: Ricardo Buring

@rburing
Copy link
Contributor

rburing commented Jun 17, 2021

comment:9

Yes, looks good now.

@egourgoulhon
Copy link
Member Author

comment:10

Thank you!

@vbraun
Copy link
Member

vbraun commented Jun 29, 2021

Changed branch from public/manifolds/pullback_bug-31904 to aea4554

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants