Skip to content

HyperbolicGeodesicPD.plot can be wrong for CDF coordinates #32362

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

Open
Vectornaut mannequin opened this issue Aug 10, 2021 · 12 comments · May be fixed by #39975
Open

HyperbolicGeodesicPD.plot can be wrong for CDF coordinates #32362

Vectornaut mannequin opened this issue Aug 10, 2021 · 12 comments · May be fixed by #39975

Comments

@Vectornaut
Copy link
Mannequin

Vectornaut mannequin commented Aug 10, 2021

This illustrates the wrong plotting:

sage: D = HyperbolicPlane().PD()
sage: a = 0
sage: b = 1e-15 + 0.7*I
sage: geodesics = [
....:     D.get_geodesic(a, b),
....:     D.get_geodesic(CC(a), CC(b)),
....:     D.get_geodesic(CDF(a), CDF(b))
....: ]
sage: show(graphics_array([geod.plot() for geod in geodesics]))

sage: print(geodesics[2].plot()[0])
Arc
  with ​center (-0.4210526315789469,1.0000000000000007)
 ​radii (0.42105263157894823,0.42105263157894823)
 ​angle 0.0
 ​inside the sector (-1.172273881128477,-0.6190660545826288)

The first two pictures show the expected output: a straight line from 0 to 0.7*I. The last picture shows the buggy output: a short arc between two points near 0.7*I. The printed output describes the arc in the buggy picture.

CC: @slel @sagetrac-jhonrubia6 @orlitzky

Component: geometry

Keywords: hyperbolic, geodesic, plot, precision

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

@Vectornaut Vectornaut mannequin added this to the sage-9.5 milestone Aug 10, 2021
@Vectornaut Vectornaut mannequin added c: geometry labels Aug 10, 2021
@Vectornaut
Copy link
Mannequin Author

Vectornaut mannequin commented Aug 10, 2021

Output image from example code

@slel
Copy link
Member

slel commented Aug 14, 2021

comment:1

Attachment: geodesic_bug.png

The current implementation of hyperbolic geodesics
relies on a conversion to the half-plane model.

Any geodesic g has its corresponding
half-plane model geodesic stored as
g._cached_geodesic.

The example geodesic has endpoints
I and 2.222222222222222e-14 + 5.666666666666666*I.

The corresponding complete geodesic is a
euclidean semicircle. Due to rounding errors
on its centre and radius, its endpoints are
computed in CDF as (-2.375, 1400000000000002.0),
whereas computations in CC give them much more
correctly as (0.0, 1.4e15).

Converting back to the disc model, the effect
is a disaster.

@Vectornaut
Copy link
Mannequin Author

Vectornaut mannequin commented Aug 17, 2021

comment:2

Aha! I suspect it's possible to find the ideal endpoints of a disk model geodesic in a more numerically stable way. I can work on a test implementation.

If the test implementation works well, do you know what we'd have to do to incorporate it into hyperbolic_geodesic.py? Is it just a matter of overriding ideal_endpoints in HyperbolicGeodesicPD?

@slel

This comment has been minimized.

@slel
Copy link
Member

slel commented Aug 18, 2021

comment:3

Replying to @Vectornaut:

Aha! I suspect it's possible to find the ideal endpoints
of a disk model geodesic in a more numerically stable way.
I can work on a test implementation.

Good!

If the test implementation works well, do you know
what we'd have to do to incorporate it into
hyperbolic_geodesic.py? Is it just a matter of
overriding ideal_endpoints in HyperbolicGeodesicPD?

Yes.

@slel
Copy link
Member

slel commented Aug 18, 2021

Changed keywords from none to hyperbolic, geodesic, plot, precision

@Vectornaut
Copy link
Mannequin Author

Vectornaut mannequin commented Aug 20, 2021

Finding ideal endpoints of geodesics on the Poincaré disk: example implementation

@Vectornaut
Copy link
Mannequin Author

Vectornaut mannequin commented Aug 20, 2021

Attachment: hyperbolic_geodesic_PD.sage.gz

Attachment: drawing-geodesics.pdf.gz

Finding ideal endpoints of geodesics on the Poincaré disk: method explanation

@Vectornaut
Copy link
Mannequin Author

Vectornaut mannequin commented Aug 20, 2021

comment:4

I just attached an example implementation. I've done some basic tests, but no systematic challenges or comparison with the current implementation (via the upper half-plane).

@sagetrac-jhonrubia6
Copy link
Mannequin

sagetrac-jhonrubia6 mannequin commented Nov 20, 2021

comment:6

Maybe unrelated or not (If you think it's unrelated I can open a new ticket on this) I've just discovered working on ticket [ticket:23427] that ideal endpoints on PD geodesics can be mapped to imag()<0 points which is an error, and when plotted often draw a geodesic int the lower half plane.
For example

Sage:    P = PD.get_point(-0.920704875684763 - 0.390259569889459*I)
Sage:    P
Boundary point in PD -0.920704875684763 - 0.390259569889459*I
Sage:    UHP(P)
Boundary point in UHP -0.662253938491478 - 1.66533453693773e-16*I
sage: g=HyperbolicPlane().PD().random_geodesic()
Sage:  g
Geodesic in PD from -0.920704875684763 - 0.390259569889459*I to 0.0632992206159446 + 0.997994593507106*I
sage: g.to_model('UHP')  
Geodesic in UHP from -0.662253938491478 - 5.55111512312578e-17*I to 31.5642842686728 - 8.34887714518118e-14*I

@mkoeppe mkoeppe modified the milestones: sage-9.5, sage-9.6 Dec 18, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.6, sage-9.7 May 3, 2022
@mkoeppe mkoeppe modified the milestones: sage-9.7, sage-9.8 Sep 19, 2022
@mkoeppe mkoeppe removed this from the sage-9.8 milestone Jan 29, 2023
@hgranath
Copy link
Contributor

Not being aware of this discussion I reported a similar issue here.

The one line change I mentioned there seems to me to fix the problem both in my use cases and in the case reported in the top post above. Maybe avoiding testing of exact equality of floting point numbers in the UHP code would be a good thing to do to improve the situation?

@hgranath
Copy link
Contributor

hgranath commented Sep 6, 2024

I found a case with similar symptoms, but which I think reflects a separate bug of the same type:

r = exp((pi*I/2).n())
G = hyperbolic_triangle(0, r, r*(1 + I)/2, model='PD', fill=True, alpha=.3)
G._show_axes = False
show(G)

(The central angle should be pi/4.) I think this is the root cause

PD = HyperbolicPlane().PD()
UHP = HyperbolicPlane().UHP()
r = exp((pi*I/2).n())
p = PD.get_point(r)
UHP(p)

which yields Boundary point in UHP 3.26624787063907e16 - 1.00000000000000*I A fix might be to change this line to

    if abs(x - I) < EPSILON:

(and add from sage.geometry.hyperbolic_space.hyperbolic_constants import EPSILON to that file's preamble).

@hgranath hgranath linked a pull request Apr 19, 2025 that will close this issue
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants