Repository: sympy/sympy

Issue:
Tangent line to Circle does not work
Running the code:
```
from sympy.geometry import Point, Line, Circle
Ri = 0.024127189424130748
Ci = Point(0.0864931002830291, 0.0819863295239654)
A = Point(0, 0.0578591400998346)
c = Circle(Ci, Ri)
Lax = Line(A, slope=0)
print(c.is_tangent(Lax)) # Gives False
I = c.tangent_lines(A)[1].p1
print(I[1]-A[1]) # But the distance of tangent is zero?
```
gives the horizontal line Lax is not tangent to the bottom of circle c. But measuring (and visually), the distance of circle to line gives zero. Can anyone check if I am doing something wrong please?

Discussion:
sympy-bot: :white_check_mark:

Hi, I am the [SymPy bot](https://github.com/sympy/sympy-bot) (v135). I'm here to help you write a release notes entry. Please read the [guide on how to write release notes](https://github.com/sympy/sympy/wiki/Writing-Release-Notes).



Your release notes are in good order.

Here is what the release notes will look like:
* geometry
  - change fractional values to rational . ([#15816](https://github.com/sympy/sympy/pull/15816) by [@RituRajSingh878](https://github.com/RituRajSingh878))

This will be added to https://github.com/sympy/sympy/wiki/Release-Notes-for-1.4.

Note: This comment will be updated with the latest check if you edit the pull request. You need to reload the page to see it. <details><summary>Click here to see the pull request description that was parsed.</summary>

    <!-- Your title above should be a short description of what
    was changed. Do not include the issue number in the title. -->

    #### References to other Issues or PRs
    <!-- If this pull request fixes an issue, write "Fixes #NNNN" in that exact
    format, e.g. "Fixes #1234". See
    https://github.com/blog/1506-closing-issues-via-pull-requests . Please also
    write a comment on that issue linking back to this pull request once it is
    open. -->
     Fixes #15797

    #### Brief description of what is fixed or changed

    Due to small fractional values circle.is_tangent is not working properly and many properties of the circle are also not working.

    > from sympy.geometry import Point, Line, Circle
    > Ri = 0.024127189424130748
    > Ci = Point(0.0864931002830291, 0.0819863295239654)
    > A = Point(0, 0.0578591400998346)
    > c = Circle(Ci, Ri)
    > print(c.is_tangent(c.tangent_lines(A)[0]))


     giving false.

    #### Other comments
    I just add a simple way to solve the problem .


    #### Release Notes

    <!-- Write the release notes for this release below. See
    https://github.com/sympy/sympy/wiki/Writing-Release-Notes for more information
    on how to write release notes. The bot will check your release notes
    automatically to see if they are formatted correctly. -->

    <!-- BEGIN RELEASE NOTES -->
    - geometry
      - change fractional values to rational .
    <!-- END RELEASE NOTES -->


</details><p>

oscarbenjamin: Can you explain how this solves the problem? Are you sure that the numbers you show are correct and that `is_tangent` should return `True`?

You need to add a test for any bug that is fixed or any new feature that you add.
oscarbenjamin: It's not obvious to me that this test should pass. If the user wants an exact tangent line then they should be using exact inputs i.e. `Rational` rather than `float`.
RituRajSingh878: In case if someone using float then for this case I think it will be helpful, otherwise it is same as previous.
sylee957: I think that the original issue did not get fixed yet
while running the code from #15797
```
from sympy import *

Ri = 0.024127189424130748
Ci = Point(0.0864931002830291, 0.0819863295239654)
A = Point(0, 0.0578591400998346)
c = Circle(Ci, Ri)
Lax = Line(A, slope=0)
print(c.is_tangent(Lax)) # Gives False
I = c.tangent_lines(A)[1].p1
print(I[1]-A[1]) # But the distance of tangent is zero?
```
it still gives
```
False
0
```
as well as the master.

While c had changed from
![image](https://user-images.githubusercontent.com/34944973/51489477-c5872800-1deb-11e9-8a69-3d3bde1bcff8.png)
to
![image](https://user-images.githubusercontent.com/34944973/51489499-dc2d7f00-1deb-11e9-9522-264526df974a.png)




oscarbenjamin: The issue won't get fixed because the numbers are not exactly correct. If they were then `Ci[1]-Ri-A[1]` would be zero but instead:
```julia
In [39]: from sympy import *
    ...: 
    ...: Ri = 0.024127189424130748
    ...: Ci = Point(0.0864931002830291, 0.0819863295239654)
    ...: A = Point(0, 0.0578591400998346)
    ...: c = Circle(Ci, Ri)
    ...: Lax = Line(A, slope=0)
    ...: print(c.is_tangent(Lax)) # Gives False
    ...: I = c.tangent_lines(A)[1].p1
    ...: print(I[1]-A[1]) # But the distance of tangent is zero?
    ...: 
    ...: 
False
0

In [40]: Ci[1] - Ri - A[1]
Out[40]: 4.16333634234434e-17
```

We can avoid all rounding errors by using `Rational` instead of `float`:
```julia
In [41]: from sympy import *
    ...: 
    ...: Ri = Rational('0.024127189424130748')
    ...: Ci = Point(Rational('0.0864931002830291'), Rational('0.0819863295239654'))
    ...: A = Point(0, Rational('0.0578591400998346'))
    ...: c = Circle(Ci, Ri)
    ...: Lax = Line(A, slope=0)
    ...: print(c.is_tangent(Lax)) # Gives False
    ...: I = c.tangent_lines(A)[1].p1
    ...: print(I[1]-A[1]) # But the distance of tangent is zero?
    ...: 
    ...: 
False
0

In [42]: Ci[1] - Ri - A[1]
Out[42]: 
        13        
──────────────────
250000000000000000
```

Note that the difference is more or less the same as in the `float` case the reason it's not exactly the same is because of rounding error.
```julia
In [43]: (Ci[1] - Ri - A[1]).evalf()
Out[43]: 5.20000000000000e-17
```
RituRajSingh878: 



> I think that the original issue did not get fixed yet
> while running the code from #15797
> 
> ```
> from sympy import *
> 
> Ri = 0.024127189424130748
> Ci = Point(0.0864931002830291, 0.0819863295239654)
> A = Point(0, 0.0578591400998346)
> c = Circle(Ci, Ri)
> Lax = Line(A, slope=0)
> print(c.is_tangent(Lax)) # Gives False
> I = c.tangent_lines(A)[1].p1
> print(I[1]-A[1]) # But the distance of tangent is zero?
> ```
> 
> it still gives
> 
> ```
> False
> 0
> ```
> 
> as well as the master.
> 
> While c had changed from
> ![image](https://user-images.githubusercontent.com/34944973/51489477-c5872800-1deb-11e9-8a69-3d3bde1bcff8.png)
> to
> ![image](https://user-images.githubusercontent.com/34944973/51489499-dc2d7f00-1deb-11e9-9522-264526df974a.png)

Because it is not an issue ,in actual Lax is not a tangent and you can find tangent from point A by c.tangent_lines which gives two tangent (which means A is outside the circle which can be seen from c.contains(A) gives False ) which is different from Lax(you can find slope of lines from Line.slope ).
and I[1]-A[1] gives zero because they are same point. And actual issue is c.is_tangent(c.tangen_lines(A)[0]) giving false .
oscarbenjamin: > And actual issue is c.is_tangent(c.tangen_lines(A)[0]) giving false .

Okay I see your point. On current master we have:
```julia
In [1]:     Ri = 0.024127189424130748 
   ...:     Ci = Point(0.0864931002830291, 0.0819863295239654) 
   ...:     A = Point(0, 0.0578591400998346) 
   ...:     c = Circle(Ci, Ri) 
   ...:     assert c.is_tangent(c.tangent_lines(A)[0]) == True                                                                    
---------------------------------------------------------------------------
AssertionError                            Traceback (most recent call last)
```

This is because although Sympy converts all the other quantities to Rational, the circle radius stays as a float.
```julia
In [2]: c.tangent_lines(A)                                                                                                        
Out[2]: 
[Line2D(Point2D(0, 289295700499173/5000000000000000), Point2D(370021790727421/5000000000000000, 20525958624431/200000000000000))
, Line2D(Point2D(0, 289295700499173/5000000000000000), Point2D(864931002830291/10000000000000000, 289295700499173/50000000000000
00))]

In [3]: c                                                                                                                         
Out[3]: Circle(Point2D(864931002830291/10000000000000000, 409931647619827/5000000000000000), 0.0241271894241307)
```

Your patch fixes this by ensuring that the radius is also converted to Rational so that all internal calculations are exact.

I hadn't realised that Sympy would convert all these internally to Rational. Generally I would disagree with that approach since users should provide exact values if they want exact calculations. That's the way it's done though so your patch just makes the radius consistent with the other bits which seems fair enough.

Maybe you should make your method for doing this conversion the same as for e.g. `Point`: [https://github.com/sympy/sympy/blob/03c392638e4532c800129460e8ceb01013f84402/sympy/geometry/point.py#L165](https://github.com/sympy/sympy/blob/03c392638e4532c800129460e8ceb01013f84402/sympy/geometry/point.py#L165).
oscargus: @RituRajSingh878 when possible, please merge the latest master for all your PRs as the Sage build issue on Travis has been solved/bypassed there so that it is at least possible to pass the tests.
RituRajSingh878: > > And actual issue is c.is_tangent(c.tangen_lines(A)[0]) giving false .
> 
> Okay I see your point. On current master we have:
> 
> ```julia
> In [1]:     Ri = 0.024127189424130748 
>    ...:     Ci = Point(0.0864931002830291, 0.0819863295239654) 
>    ...:     A = Point(0, 0.0578591400998346) 
>    ...:     c = Circle(Ci, Ri) 
>    ...:     assert c.is_tangent(c.tangent_lines(A)[0]) == True                                                                    
> ---------------------------------------------------------------------------
> AssertionError                            Traceback (most recent call last)
> ```
> 
> This is because although Sympy converts all the other quantities to Rational, the circle radius stays as a float.
> 
> ```julia
> In [2]: c.tangent_lines(A)                                                                                                        
> Out[2]: 
> [Line2D(Point2D(0, 289295700499173/5000000000000000), Point2D(370021790727421/5000000000000000, 20525958624431/200000000000000))
> , Line2D(Point2D(0, 289295700499173/5000000000000000), Point2D(864931002830291/10000000000000000, 289295700499173/50000000000000
> 00))]
> 
> In [3]: c                                                                                                                         
> Out[3]: Circle(Point2D(864931002830291/10000000000000000, 409931647619827/5000000000000000), 0.0241271894241307)
> ```
> 
> Your patch fixes this by ensuring that the radius is also converted to Rational so that all internal calculations are exact.
> 
> I hadn't realised that Sympy would convert all these internally to Rational. Generally I would disagree with that approach since users should provide exact values if they want exact calculations. That's the way it's done though so your patch just makes the radius consistent with the other bits which seems fair enough.
> 
> Maybe you should make your method for doing this conversion the same as for e.g. `Point`:
> 
> [sympy/sympy/geometry/point.py](https://github.com/sympy/sympy/blob/03c392638e4532c800129460e8ceb01013f84402/sympy/geometry/point.py#L165)
> 
> Line 165 in [03c3926](/sympy/sympy/commit/03c392638e4532c800129460e8ceb01013f84402)
>  # Turn any Floats into rationals and simplify 
> .

Yes I am trying to do .

Patch:
diff --git a/sympy/geometry/ellipse.py b/sympy/geometry/ellipse.py
index 88c0cccb47..d0bd20a059 100644
--- a/sympy/geometry/ellipse.py
+++ b/sympy/geometry/ellipse.py
@@ -10,11 +10,12 @@
 
 from sympy import Expr, Eq
 from sympy.core import S, pi, sympify
+from sympy.core.evaluate import global_evaluate
 from sympy.core.logic import fuzzy_bool
 from sympy.core.numbers import Rational, oo
 from sympy.core.compatibility import ordered
 from sympy.core.symbol import Dummy, _uniquely_named_symbol, _symbol
-from sympy.simplify import simplify, trigsimp
+from sympy.simplify import simplify, trigsimp, nsimplify
 from sympy.functions.elementary.miscellaneous import sqrt, Max
 from sympy.functions.elementary.trigonometric import cos, sin
 from sympy.functions.special.elliptic_integrals import elliptic_e
@@ -1416,7 +1417,7 @@ class Circle(Ellipse):
     def __new__(cls, *args, **kwargs):
         from sympy.geometry.util import find
         from .polygon import Triangle
-
+        evaluate = kwargs.get('evaluate', global_evaluate[0])
         if len(args) == 1 and isinstance(args[0], Expr):
             x = kwargs.get('x', 'x')
             y = kwargs.get('y', 'y')
@@ -1438,12 +1439,12 @@ def __new__(cls, *args, **kwargs):
             center_y = -d/b/2
             r2 = (center_x**2) + (center_y**2) - e
 
-            return Circle((center_x, center_y), sqrt(r2))
+            return Circle((center_x, center_y), sqrt(r2), evaluate=evaluate)
 
         else:
             c, r = None, None
             if len(args) == 3:
-                args = [Point(a, dim=2) for a in args]
+                args = [Point(a, dim=2, evaluate=evaluate) for a in args]
                 t = Triangle(*args)
                 if not isinstance(t, Triangle):
                     return t
@@ -1451,8 +1452,13 @@ def __new__(cls, *args, **kwargs):
                 r = t.circumradius
             elif len(args) == 2:
                 # Assume (center, radius) pair
-                c = Point(args[0], dim=2)
-                r = sympify(args[1])
+                c = Point(args[0], dim=2, evaluate=evaluate)
+                r = args[1]
+                # TODO: use this instead of the 'if evaluate' block below, but
+                # this will prohibit imaginary radius
+                # r = Point(r, 0, evaluate=evaluate).x  # convert via Point as necessary
+                if evaluate:
+                    r = simplify(nsimplify(r, rational=True))
 
             if not (c is None or r is None):
                 if r == 0:


Test patch:
diff --git a/sympy/geometry/tests/test_ellipse.py b/sympy/geometry/tests/test_ellipse.py
index 12e3af49d0..9058060ce0 100644
--- a/sympy/geometry/tests/test_ellipse.py
+++ b/sympy/geometry/tests/test_ellipse.py
@@ -470,6 +470,21 @@ def test_issue_15259():
     assert Circle((1, 2), 0) == Point(1, 2)
 
 
+def test_issue_15797():
+    Ri = 0.024127189424130748
+    Ci = (0.0864931002830291, 0.0819863295239654)
+    A = Point(0, 0.0578591400998346)
+    c = Circle(Ci, Ri)  # evaluated
+    assert c.is_tangent(c.tangent_lines(A)[0]) == True
+    assert c.center.x.is_Rational
+    assert c.center.y.is_Rational
+    assert c.radius.is_Rational
+    u = Circle(Ci, Ri, evaluate=False)  # unevaluated
+    assert u.center.x.is_Float
+    assert u.center.y.is_Float
+    assert u.radius.is_Float
+
+
 def test_auxiliary_circle():
     x, y, a, b = symbols('x y a b')
     e = Ellipse((x, y), a, b)