Repository: sympy/sympy
Pull request: change float value of radius  to Rational in circle
<!-- 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 of radius of the circle, 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
  - radius of Circle is made Rational unless evaluate=False
<!-- END RELEASE NOTES -->


Problem statement:
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?

Hints from issue discussion:
I think this is due to many points after the decimal as you can see Lax.distance(Ci) and Ri has a very small difference (should be zero but not), I think due to this it is showing "false" for the tangent. 
although `c.is_tangent(c.tangent_lines(A)[1])` and `c.is_tangent(c.tangent_lines(A)[0)` are also giving false .
I think his should be a decimal precision problem. There is any workaround in this case?
@lourenko the problem is that they are not *exactly* tangent. There is a small difference.

You can eliminate rounding error within Sympy by using `Rational` as in 
```python
Ri = Rational('0.024127189424130748')
```
(note the quotes)

However this will not solve your problem since the numbers you are putting in are not precisely correct: Sympy is returning the correct answer for the numbers you give.
Thank you for the reply. The values are for a geometric problem where `d0` and `d2g` are given parameters. The rounding value occurs in sympy. I rewrite a MWE.

```
A = Point(0, d0/2)
G = Point(zmax-b2, d2g/2)
Lax = Line(A, slope=0)
Lgi = Line(G, slope=tan((270-3)*pi/180))

Pi, = Lax.intersect(Lgi)

Ri = 3*b2
Lci = Line(Point(0, d0/2+Ri), slope=0)
Lpi = Line(Pi, slope=tan(134.5*pi/180))
Ci, = Lci.intersect(Lpi)
c = Circle(Ci, Ri)
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?
```

I just made another line tangent the circle, got the point and renamed this line to `Lax`.
@lourenko can you provide a complete example with all variables defined and the imports for any functions shown?

I can't run this because I don't know what `d0` etc are. Also I can't tell whether `pi`, `tan` etc are imported from Sympy or from numpy, or the math module etc.
I solved the problem adding and subtracting `1e-14` to all numeric values. As @RituRajSingh878 and @oscarbenjamin stated, this should be a rounding error problem. Now the code is running fine. Thank for your attention patience.
Btw @oscarbenjamin, I am using:
`d0 = 0.1412898146509956 #[meters]`
e
`d2g = 0.30038307607263465 #[meters]`
and importing like:
```
import numpy as np
from numpy import pi, sqrt, sin, cos, tan, arctan2
from sympy.geometry import Point, Line, Circle, Segment
from sympy import symbols as symbol
```
The thing about something like `is_tangent` is that it requires *all* of your numbers to be exactly mathematically precise. The better solution here is either:
1. Use only exact calculations. Don't mix floats in and don't use numpy's pi, sqrt etc. Start with exact numbers from Sympy and use them to calculate everything, using only Sympy functions.
2. If you must use floats then you shouldn't ask a question like `is_tangent`. The question should be something like: is the distance smaller than some tolerance?

Commit message: added method for changing fractional value to Rational value for radius

Patch to review:
diff --git a/sympy/geometry/ellipse.py b/sympy/geometry/ellipse.py
index b817740506d2..905e16f21161 100644
--- a/sympy/geometry/ellipse.py
+++ b/sympy/geometry/ellipse.py
@@ -8,13 +8,14 @@
 
 from __future__ import division, print_function
 
-from sympy import Expr, Eq
+from sympy import Expr, Eq, Float
 from sympy.core import S, pi, sympify
+from sympy.core.containers import Tuple
 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
 from sympy.functions.elementary.trigonometric import cos, sin
 from sympy.functions.special.elliptic_integrals import elliptic_e
@@ -1436,7 +1437,11 @@ def __new__(cls, *args, **kwargs):
             elif len(args) == 2:
                 # Assume (center, radius) pair
                 c = Point(args[0], dim=2)
-                r = sympify(args[1])
+                r = Tuple(args[1], )
+                r = r.xreplace(dict(
+                [(f, simplify(nsimplify(f, rational=True)))
+                 for f in r.atoms(Float)]))
+                r = r[0]
 
             if not (c is None or r is None):
                 if r == 0:
diff --git a/sympy/geometry/tests/test_ellipse.py b/sympy/geometry/tests/test_ellipse.py
index 8a97c52dd1a9..221d74f87375 100644
--- a/sympy/geometry/tests/test_ellipse.py
+++ b/sympy/geometry/tests/test_ellipse.py
@@ -463,3 +463,10 @@ def test_circumference():
     assert abs(Ellipse(None, hradius=5, vradius=3).circumference.evalf(16) - 25.52699886339813) < 1e-10
 def test_issue_15259():
     assert Circle((1, 2), 0) == Point(1, 2)
+
+def test_issue_15797():
+    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


Code review comments:
diff_hunk: @@ -1436,7 +1437,11 @@ def __new__(cls, *args, **kwargs):
             elif len(args) == 2:
                 # Assume (center, radius) pair
                 c = Point(args[0], dim=2)
-                r = sympify(args[1])
+                r = Tuple(args[1], )
+                r = r.xreplace(dict(
+                [(f, simplify(nsimplify(f, rational=True)))
+                 for f in r.atoms(Float)]))
+                r = r[0]
path: sympy/geometry/ellipse.py
text: @user1:
Can the lines above be simplified to `r = nsimplify(args[1], rational=True)`?

@user1:
I don't understand why you've created a `Tuple`...

@author:
Thanks for the suggestion. I am changing it.

diff_hunk: @@ -1436,7 +1437,11 @@ def __new__(cls, *args, **kwargs):
             elif len(args) == 2:
                 # Assume (center, radius) pair
                 c = Point(args[0], dim=2)
-                r = sympify(args[1])
+                r = Tuple(args[1], )
+                r = r.xreplace(dict(
+                [(f, simplify(nsimplify(f, rational=True)))
+                 for f in r.atoms(Float)]))
path: sympy/geometry/ellipse.py
text: @user1:
I think this may support `evaluate` as an option, as in `Point`
https://github.com/sympy/sympy/blob/68aa33a78433c4c5e8308e9d2dc096e1524f9e6b/sympy/geometry/point.py#L165-L170

@author:
I think it will become more complicated and we have to add an extra argument 'evaluate'.

@user2:
I think it needs some way to stop `float`s being converted to `Rational`. I think it would be better if `evaluate` defaulted to `False` for `Point` as well.

@author:
But I think it will be better to change to Rational otherwise due to floating value it may rounded and result will always not going to be good and  in above case we can't round the floating value.So I don't think there should be an option for not conveting floating value to Rational.

@user2:
If we follow that argument to its conclusion then all floats *everywhere* should be converted to Rational. That doesn't solve the potential problems that users have though: if they use floats they can get rounding errors before calling any of Sympy's functions. Users who want exact results should provide exact inputs. Since `Point` already automatically converts float to Rational (by default) then maybe it makes sense to do so here as well. I wonder if actually the proper fix is that neither should be converted to Rational and `is_tangent` should be changed to use floating point comparison with tolerance.

@user2:
See [https://github.com/sympy/sympy/issues/11484](https://github.com/sympy/sympy/issues/11484). There may be other issues but I found that from a quick search.

@author:
>  `is_tangent` should be changed to use floating point comparison with tolerance.

I think this can be a possibility. But you know floating error even can occur in exact no. provided by the user like if the user provides 2.6666666 then there is a possibility that it may rounded in the calculation. And for floating point comparison with tolerance, we can do but the question is up to which degree we should do?

@user2:
See also #6716