Opened 21 months ago
Closed 21 months ago
#28669 closed defect (fixed)
Unnecessary coordinate check while initializing an integrated curve
Reported by:  egourgoulhon  Owned by:  

Priority:  major  Milestone:  sage9.0 
Component:  geometry  Keywords:  integrated_curve, geodesic 
Cc:  karimvanaelst  Merged in:  
Authors:  Eric Gourgoulhon  Reviewers:  Karim Van Aelst 
Report Upstream:  N/A  Work issues:  
Branch:  c0efb2e (Commits, GitHub, GitLab)  Commit:  c0efb2e0e181bbc29818736c69d68648604a7d7a 
Dependencies:  Stopgaps: 
Description
In Sage 8.9, we have:
sage: E.<r,phi> = EuclideanSpace(coordinates='polar') sage: p = E((1, 0)) # the initial point sage: v = E.tangent_space(p)((2, 1)) # the initial vector sage: t = var('t') sage: c = E.integrated_geodesic(E.metric(), (t, 0, 10), v) ... ValueError: initial point should be in the domain of the chart
Now, the initial point p
does lie in the domain of the chart:
sage: E.default_chart()(p) (1, 0)
Indeed, 0
is a perfectly valid value for the polar coordinate phi
:
sage: E.default_chart().coord_range() r: (0, +oo); phi: [0, 2*pi] (periodic)
The above error turns out to result from a too strict and unnecessary coordinate check performed in IntegratedCurve.__init__()
. This check is removed here.
Change History (9)
comment:1 Changed 21 months ago by
 Branch set to public/manifolds/integrated_curve_init
 Cc karimvanaelst added
 Commit set to 3ab55d7e91044c0cb177f9bfd125f30c281bf3da
 Status changed from new to needs_review
comment:2 followup: ↓ 4 Changed 21 months ago by
please add a doctest
comment:3 Changed 21 months ago by
 Commit changed from 3ab55d7e91044c0cb177f9bfd125f30c281bf3da to c0efb2e0e181bbc29818736c69d68648604a7d7a
Branch pushed to git repo; I updated commit sha1. New commits:
c0efb2e  Add doctest in IntegratedCurve.__init__()

comment:4 in reply to: ↑ 2 ; followup: ↓ 5 Changed 21 months ago by
comment:5 in reply to: ↑ 4 Changed 21 months ago by
 Status changed from needs_review to positive_review
Replying to egourgoulhon:
Replying to chapoton:
please add a doctest
Done in the above commit.
Ticket seems ready to be closed.
comment:6 Changed 21 months ago by
Thanks for the review!
comment:7 Changed 21 months ago by
 Status changed from positive_review to needs_work
Reviewer name is missing
comment:8 Changed 21 months ago by
 Reviewers set to Karim Van Aelst
 Status changed from needs_work to positive_review
comment:9 Changed 21 months ago by
 Branch changed from public/manifolds/integrated_curve_init to c0efb2e0e181bbc29818736c69d68648604a7d7a
 Resolution set to fixed
 Status changed from positive_review to closed
Note: See
TracTickets for help on using
tickets.
New commits:
Remove unnecessary coordinate check in IntegratedCurve.__init__()