Opened 18 months ago
Closed 17 months ago
#28669 closed defect (fixed)
Unnecessary coordinate check while initializing an integrated curve
Reported by: | egourgoulhon | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-9.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 18 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 follow-up: ↓ 4 Changed 18 months ago by
please add a doctest
comment:3 Changed 18 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 ; follow-up: ↓ 5 Changed 18 months ago by
comment:5 in reply to: ↑ 4 Changed 18 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 18 months ago by
Thanks for the review!
comment:7 Changed 18 months ago by
- Status changed from positive_review to needs_work
Reviewer name is missing
comment:8 Changed 18 months ago by
- Reviewers set to Karim Van Aelst
- Status changed from needs_work to positive_review
comment:9 Changed 17 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__()