Fix intersection of hyperbolic geodesic arcs #23427

videlec · 2017-07-13T21:12:00Z

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]

CC: @sagetrac-jhonrubia6 @slel

Component: geometry

Keywords: bug

Author: Javier Honrubia González

Branch/Commit: f3dc878

Reviewer: Vincent Delecroix, Dave Morris, Dima Pasechnik

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

The text was updated successfully, but these errors were encountered:

sagetrac-jhonrubia6 · 2017-07-14T12:38:21Z

comment:1

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.

sagetrac-jhonrubia6 · 2017-08-01T16:49:55Z

Branch: u/jhonrubia6/intersection_of_hyperbolic_geodesics_is_wrong

sagetrac-jhonrubia6 · 2017-08-01T17:51:28Z

New commits:

`3f28781`	`Bug corrected. Some examples on perpendicular_bisector() modified as to pass tests`

sagetrac-jhonrubia6 · 2017-08-01T17:51:28Z

Commit: 3f28781

sagetrac-jhonrubia6 · 2017-08-01T17:51:28Z

Author: Javier Honrubia González

videlec · 2017-08-01T20:50:32Z

comment:4

What about vertical geodesics?

videlec · 2017-08-03T08:30:16Z

comment:5

Replying to @videlec:

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]

sagetrac-jhonrubia6 · 2017-08-10T09:54:26Z

comment:6

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

videlec · 2017-08-10T10:07:39Z

comment:7

Replying to @sagetrac-jhonrubia6:

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

sagetrac-git · 2021-11-13T17:57:39Z

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

`150a482`	`Merge remote-tracking branch 'origin/master' into t/23427/intersection_of_hyperbolic_geodesics_is_wrong`
`3a07c7e`	`interesection() heavily modified to account for all different cases.`

sagetrac-git · 2021-11-13T17:57:39Z

Changed commit from 3f28781 to 3a07c7e

slel · 2021-11-13T18:03:00Z

comment:9

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

sagetrac-jhonrubia6 · 2021-11-13T18:05:46Z

comment:11

Replying to @slel:

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

Right, I've just noticed, I merge with develop and re-submit

sagetrac-git · 2021-11-16T18:34:21Z

Changed commit from 3a07c7e to 715cbac

sagetrac-git · 2021-11-16T18:34:21Z

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

`df1ed36`	`Merge branch 'develop' into t/23427/intersection_of_hyperbolic_geodesics_is_wrong`
`715cbac`	`merged with the develop branch. Most issues fixed.`

sagetrac-jhonrubia6 · 2021-11-16T18:42:14Z

comment:13

However an important new bug appeared when dealing with the intersection in the PD model:

sage: g=HyperbolicPlane().PD().random_geodesic()
sage: h = g.perpendicular_bisector().complete()
sage: g.intersection(h)
[]

which is an obvious error. Somehow the problem lies in the abstract method intersection(self, other)

sage: g._cached_geodesic.intersection(h)
[]

whereas

sage: g._cached_geodesic.intersection(h._cached_geodesic)
[Point in UHP -1.27483700342883 + 2.78119562788909*I]

However

sage: g=HyperbolicPlane().PD().random_geodesic()
sage: h = g.perpendicular_bisector().complete()
sage: g.intersection(h)
[Point in UHP 0.815206723839169 + 2.95290895719856*I]

Work in progress

sagetrac-git · 2021-11-17T17:53:26Z

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

`57917b5`	`Bug involving intersection models other tha UHP solved.`

sagetrac-git · 2021-11-17T17:53:26Z

Changed commit from 715cbac to 57917b5

sagetrac-git · 2021-11-17T22:29:12Z

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

`b0648b2`	`ll test passed. Bug corrected with asymptotically parallel geodesics`

sagetrac-git · 2021-11-17T22:29:12Z

Changed commit from 57917b5 to b0648b2

dimpase · 2021-11-25T23:11:48Z

comment:17

Just merge (or rebase) locally, and push the merged branch.

DaveWitteMorris · 2021-11-26T00:47:59Z

comment:18

Here are the merge conflicts that I see with 9.5b7. They are in the EXAMPLES sections of the two perpendicular_bisector methods.

<<<<<<< HEAD
            sage: h = g.perpendicular_bisector()
            sage: P = g.plot(color='blue')+h.plot(color='orange');P  # optional - sage.plot
=======
            sage: h = g.perpendicular_bisector().complete()
            sage: P = g.plot(color='blue')+h.plot(color='orange');P
>>>>>>> b0648b27e31c67536205a4fe9c4e85d66adea382

<<<<<<< HEAD
            sage: h = g.perpendicular_bisector()
            sage: show(g.plot(color='blue')+h.plot(color='orange'))  # optional - sage.plot
=======
            sage: h = g.perpendicular_bisector().complete()
            sage: show(g.plot(color='blue')+h.plot(color='orange'))
>>>>>>> b0648b27e31c67536205a4fe9c4e85d66adea382

dimpase · 2021-11-26T15:09:00Z

comment:19

so just use the lines in HEAD when you resolve the conflict.

dimpase · 2021-11-26T15:12:09Z

comment:20

sorry - if these .complete() things are in the ticket's branch the you need to do the obvious editing.

sagetrac-git · 2021-11-29T17:49:37Z

Changed commit from b0648b2 to 398c65d

sagetrac-git · 2021-11-29T17:49:37Z

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

`398c65d`	`merged with develop. Conflicts in EXAMPLES fixed`

dimpase · 2021-12-03T13:24:30Z

comment:24

clean up is needed:

% tox -e pycodestyle --  src/sage/geometry/hyperbolic_space/hyperbolic_geodesic.py
pycodestyle recreate: /Users/dima/sage/.tox/src
pycodestyle run-test-pre: PYTHONHASHSEED='3653170419'
pycodestyle run-test: commands[0] | tox -c /Users/dima/sage/src/tox.ini -e pycodestyle -- src/sage/geometry/hyperbolic_space/hyperbolic_geodesic.py
pycodestyle create: /Users/dima/sage/src/.tox/pycodestyle
pycodestyle installdeps: pycodestyle
pycodestyle installed: pycodestyle==2.8.0
pycodestyle run-test-pre: PYTHONHASHSEED='400015120'
pycodestyle run-test: commands[0] | pycodestyle sage/geometry/hyperbolic_space/hyperbolic_geodesic.py
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1043:1: E265 block comment should start with '# '
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1045:1: E265 block comment should start with '# '
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1155:1: W293 blank line contains whitespace
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1358:1: W293 blank line contains whitespace
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1388:1: W293 blank line contains whitespace
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1395:1: W293 blank line contains whitespace
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1401:1: W293 blank line contains whitespace
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1407:1: W293 blank line contains whitespace
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1413:1: W293 blank line contains whitespace
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1419:1: W293 blank line contains whitespace
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1425:1: W293 blank line contains whitespace
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1431:1: W293 blank line contains whitespace
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1443:1: W293 blank line contains whitespace
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1449:1: W293 blank line contains whitespace
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1455:1: W293 blank line contains whitespace
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1460:25: E225 missing whitespace around operator
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1462:9: E265 block comment should start with '# '
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1467:9: E265 block comment should start with '# '
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1468:9: E265 block comment should start with '# '
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1469:42: E261 at least two spaces before inline comment
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1469:43: E262 inline comment should start with '# '
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1470:20: E231 missing whitespace after ','
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1470:43: W291 trailing whitespace
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1473:24: E231 missing whitespace after ','
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1474:9: E265 block comment should start with '# '
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1475:9: E265 block comment should start with '# '
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1477:20: E231 missing whitespace after ','
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1480:24: E231 missing whitespace after ','
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1483:23: E225 missing whitespace around operator
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1483:42: E225 missing whitespace around operator
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1486:17: E265 block comment should start with '# '
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1488:21: E265 block comment should start with '# '
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1489:28: E231 missing whitespace after ','
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1489:46: E231 missing whitespace after ','
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1491:25: E225 missing whitespace around operator
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1494:52: E231 missing whitespace after ','
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1494:59: E261 at least two spaces before inline comment
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1494:60: E262 inline comment should start with '# '
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1495:34: E225 missing whitespace around operator
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1500:17: E265 block comment should start with '# '
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1501:17: E265 block comment should start with '# '
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1502:34: E225 missing whitespace around operator
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1503:28: E231 missing whitespace after ','
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1505:25: E225 missing whitespace around operator
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1507:34: E225 missing whitespace around operator
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1512:13: E265 block comment should start with '# '
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1513:13: E265 block comment should start with '# '
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1514:30: E225 missing whitespace around operator
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1515:24: E231 missing whitespace after ','
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1518:17: E265 block comment should start with '# '
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1532:21: E265 block comment should start with '# '
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1533:29: E225 missing whitespace around operator
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1537:42: E225 missing whitespace around operator
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1537:52: E225 missing whitespace around operator
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1537:85: E225 missing whitespace around operator
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1537:95: E225 missing whitespace around operator
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:1874:1: W293 blank line contains whitespace
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:2031:13: E741 ambiguous variable name 'I'
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:2036:13: E741 ambiguous variable name 'I'
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:2143:1: E265 block comment should start with '# '
sage/geometry/hyperbolic_space/hyperbolic_geodesic.py:2145:1: E265 block comment should start with '# '
14      E225 missing whitespace around operator
9       E231 missing whitespace after ','
2       E261 at least two spaces before inline comment
2       E262 inline comment should start with '# '
17      E265 block comment should start with '# '
2       E741 ambiguous variable name 'I'
1       W291 trailing whitespace
14      W293 blank line contains whitespace
ERROR: InvocationError for command /Users/dima/sage/src/.tox/pycodestyle/bin/pycodestyle sage/geometry/hyperbolic_space/hyperbolic_geodesic.py (exited with code 1)
______________________________ summary _______________________________
ERROR:   pycodestyle: commands failed
ERROR: InvocationError for command /usr/local/bin/tox -c src/tox.ini -e pycodestyle -- src/sage/geometry/hyperbolic_space/hyperbolic_geodesic.py (exited with code 1)

all of these except E741 can be trivially fixed. (Leave E741 as is).

sagetrac-git · 2021-12-06T20:16:57Z

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

`f3dc878`	`Clean up done leaving E741`

sagetrac-git · 2021-12-06T20:16:57Z

Changed commit from 398c65d to f3dc878

dimpase · 2021-12-06T22:31:15Z

comment:27

OK, good.

dimpase · 2021-12-06T22:31:15Z

Reviewer: Dima Pasechnik

slel · 2021-12-07T16:17:39Z

Changed reviewer from Dima Pasechnik to Vincent Delecroix, Dave Morris, Dima Pasechnik

vbraun · 2021-12-19T11:47:55Z

Changed branch from u/jhonrubia6/intersection_of_hyperbolic_geodesics_is_wrong to f3dc878

videlec added this to the sage-8.1 milestone Jul 13, 2017

videlec added c: geometry labels Jul 13, 2017

sagetrac-jhonrubia6 mannequin added the s: needs review label Aug 1, 2017

videlec added s: needs work and removed s: needs review labels Aug 3, 2017

This comment has been minimized.

Sign in to view

slel modified the milestones: sage-8.1, sage-9.5 Nov 13, 2021

slel changed the title ~~intersection of hyperbolic geodesics is wrong~~ Fix intersection of hyperbolic geodesic arcs Nov 13, 2021

sagetrac-jhonrubia6 mannequin added s: needs review and removed s: needs work labels Nov 20, 2021

DaveWitteMorris added s: needs work and removed s: needs review labels Nov 29, 2021

sagetrac-jhonrubia6 mannequin added s: needs review and removed s: needs work labels Nov 30, 2021

dimpase added s: needs work and removed s: needs review labels Dec 3, 2021

sagetrac-jhonrubia6 mannequin added s: needs review and removed s: needs work labels Dec 6, 2021

dimpase added s: positive review and removed s: needs review labels Dec 6, 2021

vbraun removed the s: positive review label Dec 19, 2021

vbraun closed this as completed in 00dbed0 Dec 19, 2021

slel mentioned this issue Jun 5, 2022

Meta-ticket: Hyperbolic geometry #32400

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix intersection of hyperbolic geodesic arcs #23427

Fix intersection of hyperbolic geodesic arcs #23427

videlec commented Jul 13, 2017

sagetrac-jhonrubia6 mannequin commented Jul 14, 2017

sagetrac-jhonrubia6 mannequin commented Aug 1, 2017

sagetrac-jhonrubia6 mannequin commented Aug 1, 2017

sagetrac-jhonrubia6 mannequin commented Aug 1, 2017

sagetrac-jhonrubia6 mannequin commented Aug 1, 2017

videlec commented Aug 1, 2017

videlec commented Aug 3, 2017

sagetrac-jhonrubia6 mannequin commented Aug 10, 2017

videlec commented Aug 10, 2017

sagetrac-git mannequin commented Nov 13, 2021

sagetrac-git mannequin commented Nov 13, 2021

slel commented Nov 13, 2021

This comment has been minimized.

sagetrac-jhonrubia6 mannequin commented Nov 13, 2021

sagetrac-git mannequin commented Nov 16, 2021

sagetrac-git mannequin commented Nov 16, 2021

sagetrac-jhonrubia6 mannequin commented Nov 16, 2021

sagetrac-git mannequin commented Nov 17, 2021

sagetrac-git mannequin commented Nov 17, 2021

sagetrac-git mannequin commented Nov 17, 2021

sagetrac-git mannequin commented Nov 17, 2021

dimpase commented Nov 25, 2021

DaveWitteMorris commented Nov 26, 2021

dimpase commented Nov 26, 2021

dimpase commented Nov 26, 2021

sagetrac-git mannequin commented Nov 29, 2021

sagetrac-git mannequin commented Nov 29, 2021

dimpase commented Dec 3, 2021

sagetrac-git mannequin commented Dec 6, 2021

sagetrac-git mannequin commented Dec 6, 2021

dimpase commented Dec 6, 2021

dimpase commented Dec 6, 2021

slel commented Dec 7, 2021

vbraun commented Dec 19, 2021

Fix intersection of hyperbolic geodesic arcs #23427

Fix intersection of hyperbolic geodesic arcs #23427

Comments

videlec commented Jul 13, 2017

sagetrac-jhonrubia6 mannequin commented Jul 14, 2017

sagetrac-jhonrubia6 mannequin commented Aug 1, 2017

sagetrac-jhonrubia6 mannequin commented Aug 1, 2017

sagetrac-jhonrubia6 mannequin commented Aug 1, 2017

sagetrac-jhonrubia6 mannequin commented Aug 1, 2017

videlec commented Aug 1, 2017

videlec commented Aug 3, 2017

sagetrac-jhonrubia6 mannequin commented Aug 10, 2017

videlec commented Aug 10, 2017

sagetrac-git mannequin commented Nov 13, 2021

sagetrac-git mannequin commented Nov 13, 2021

slel commented Nov 13, 2021

This comment has been minimized.

sagetrac-jhonrubia6 mannequin commented Nov 13, 2021

sagetrac-git mannequin commented Nov 16, 2021

sagetrac-git mannequin commented Nov 16, 2021

sagetrac-jhonrubia6 mannequin commented Nov 16, 2021

sagetrac-git mannequin commented Nov 17, 2021

sagetrac-git mannequin commented Nov 17, 2021

sagetrac-git mannequin commented Nov 17, 2021

sagetrac-git mannequin commented Nov 17, 2021

dimpase commented Nov 25, 2021

DaveWitteMorris commented Nov 26, 2021

dimpase commented Nov 26, 2021

dimpase commented Nov 26, 2021

sagetrac-git mannequin commented Nov 29, 2021

sagetrac-git mannequin commented Nov 29, 2021

dimpase commented Dec 3, 2021

sagetrac-git mannequin commented Dec 6, 2021

sagetrac-git mannequin commented Dec 6, 2021

dimpase commented Dec 6, 2021

dimpase commented Dec 6, 2021

slel commented Dec 7, 2021

vbraun commented Dec 19, 2021