Opened 5 years ago

Last modified 8 months ago

#23427 closed defect

Fix intersection of hyperbolic geodesic arcs — at Version 10

Reported by: vdelecroix Owned by:
Priority: major Milestone: sage-9.5
Component: geometry Keywords: bug
Cc: jhonrubia6, slelievre Merged in:
Authors: Javier Honrubia González Reviewers:
Report Upstream: N/A Work issues:
Branch: u/jhonrubia6/intersection_of_hyperbolic_geodesics_is_wrong (Commits, GitHub, GitLab) Commit: 3a07c7e7ceb2cc66f46d96b1f90b7bd574e52640
Dependencies: Stopgaps:

Status badges

Description (last modified by slelievre)

The following geodesic arcs do not intersect but Sage still gives an intersection point.

sage: UHP = HyperbolicPlane().UHP()
sage: g1 = UHP.get_geodesic(2*QQbar.gen(), 5)
sage: g2 = UHP.get_geodesic(-1/2, Infinity)
sage: g1.intersection(g2)
[Point in UHP -0.500000000000000? + 1.284523257866513?*I]

Change History (10)

comment:1 Changed 5 years ago by jhonrubia6

It seems easy. Checking that the real part of the fixed point of both involutions lies between the real part of endpoints of both geodesics,would be a fast implementation. Are you going to tackle this? I have time these days to code it.

comment:2 Changed 5 years ago by jhonrubia6

  • Branch set to u/jhonrubia6/intersection_of_hyperbolic_geodesics_is_wrong

comment:3 Changed 5 years ago by jhonrubia6

  • Authors set to Javier Honrubia González
  • Commit set to 3f2878194efe7adb432c655c2afcda70c7e33e13
  • Status changed from new to needs_review

New commits:

3f28781Bug corrected. Some examples on perpendicular_bisector() modified as to pass tests

comment:4 follow-up: Changed 5 years ago by vdelecroix

What about vertical geodesics?

comment:5 in reply to: ↑ 4 Changed 5 years ago by vdelecroix

  • Status changed from needs_review to needs_work

Replying to vdelecroix:

What about vertical geodesics?

More precisely

sage: UHP = HyperbolicPlane().UHP()
sage: g1 = UHP.get_geodesic(I, 2*I)
sage: g2 = UHP.get_geodesic(I/2, 4*I)
sage: g3 = UHP.get_geodesic(3*I, 4*I)
sage: g1.intersection(g2)
[Boundary point in UHP +Infinity, Boundary point in UHP 0]
sage: g1.intersection(g3)
[Boundary point in UHP +Infinity, Boundary point in UHP 0]

comment:6 follow-up: Changed 5 years ago by jhonrubia6

we also need to correct

sage: UHP = HyperbolicPlane().UHP()
sage: g1=UHP.get_geodesic(-1,+1)
sage: g2=UHP.get_geodesic(-1,-1+2*I)
sage: g1.intersection(g2)
[]

should return

[Boundary point in UHP -1]

and

UHP = HyperbolicPlane().UHP()
g1=UHP.get_geodesic(-1,I)
g2=UHP.get_geodesic(+1,(+cos(pi/3)+I*sin(pi/3)))
[Boundary point in UHP -1, Boundary point in UHP 1]

should return []

What should be the correct answer of the function in the following case?

UHP = HyperbolicPlane().UHP()
g1=UHP.get_geodesic(I,3*I)
g2=UHP.get_geodesic(2*I,4*I)

Maybe

[2*I,3*I]

or

Geodesic in UHP from 2*I to 3*I
Last edited 5 years ago by jhonrubia6 (previous) (diff)

comment:7 in reply to: ↑ 6 Changed 5 years ago by vdelecroix

Replying to jhonrubia6:

<SNIP>

What should be the correct answer of the function in the following case?

UHP = HyperbolicPlane().UHP()
g1=UHP.get_geodesic(I,3*I)
g2=UHP.get_geodesic(2*I,4*I)

Maybe

[2*I,3*I]

definitely not

or

Geodesic in UHP from 2*I to 3*I

Much better

comment:8 Changed 9 months ago by git

  • Commit changed from 3f2878194efe7adb432c655c2afcda70c7e33e13 to 3a07c7e7ceb2cc66f46d96b1f90b7bd574e52640

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

150a482Merge remote-tracking branch 'origin/master' into t/23427/intersection_of_hyperbolic_geodesics_is_wrong
3a07c7einteresection() heavily modified to account for all different cases.

comment:9 Changed 9 months ago by slelievre

Ideally, work on top of the develop branch (currently Sage 9.5.beta6) rather than master.

comment:10 Changed 9 months ago by slelievre

  • Cc slelievre added
  • Description modified (diff)
  • Milestone changed from sage-8.1 to sage-9.5
  • Summary changed from intersection of hyperbolic geodesics is wrong to Fix intersection of hyperbolic geodesic arcs
Note: See TracTickets for help on using tickets.