intersection line are not complanar with line in plane
volkoshkursk opened this issue · 13 comments
Hello! I generate a plane from points and make a plane from point and normal. Then I make an intersection line with Plane.intersect_plane. When I try to find an intersection point of the intersection line and a line from points, which was used to making a first plane I get an error
565 if not self.is_coplanar(other, tol=tol):
--> 566 raise ValueError("The lines must be coplanar.")
568 # Vector from line A to line B.
569 vector_ab = Vector.from_points(self.point, other.point)
ValueError: The lines must be coplanar.
Hello @volkoshkursk,
Could you please post a minimal working example of this? I'll need to see this in code to decide if this is an issue.
from skspatial.objects import Plane
import numpy as np
points = np.array([[ 80. , 103.89014 , 378. ],
[ 80. , 104. , 377.75623 ],
[ 79.957245, 104. , 378. ]], dtype=np.float32)
secant = Plane(point=(0,0,0), normal=(1,1,1))
poly = Plane.from_points(points[0],points[1], points[2])
intersection = secant.intersect_plane(poly, abs_tol=1e-6)
line = Line.from_points(points[0], points[1])
line.intersect_line(intersection, abs_tol=1e-6)
P.S. ubuntu 20, python 3.10
P.P.S. I open an Pull Request which fix this behaviour
It doesn't make sense to add a tolerance to the coplanar check in Line.intersect_line. If the lines are not coplanar, then they cannot have an intersection point.
These lines can not be non-coplanar due to the fact that line is build with the points, which was the plane build with and secant is intersection of the plane and another plane. If we check distance between one of the point from points and plane with Plane.distance_point it would not be 0, which would be impossible if we had precise data. The error is happens because we check the coplanarity with more precision that is in data. This situation is equal to is_parallel's limitation of tolerance. My hypothesis also can be proofed graphically.
This situation is equal to is_parallel's limitation of tolerance.
It's actually not the same. See this code:
if self.direction.is_parallel(other.direction, **kwargs):
raise ValueError("The lines must not be parallel.")
if not self.is_coplanar(other):
raise ValueError("The lines must be coplanar.")Notice that the first check is "if parallel", but the second check is "if NOT coplanar". So passing a tolerance to is_parallel makes that check more strict, but passing a tolerance to is_coplanar would make the check less strict.
If the lines aren't coplanar, then the output of this method will not be an intersection point of the two lines. It'll be the point on line A which is closest to line B, but it won't be an actual intersection point because the lines don't intersect.
I tried your code without the is_coplanar check, and the resulting point is indeed not on line B, which I verified by running this:
>>> point = line.intersect_line(intersection)
>>> intersection.contains_point(point)
FalseHowever, if we calculate the distance between the point and line we get the value, which is less than data precision.
>>> intersection.distance_point(point)
2.9251899693564517e-06
I completely agree, that it would be non-coplanar if the data was enought precise. However, my data's precision is limited by accuracy of sensor. So, difference less then 10^-6 in absolute value is most probably due to inaccuracy of collecting/storage/processing data.
Ok, how about I add a keyword argument check_coplanar with default value True? Then you can disable the coplanar check when needed by passing check_coplanar=False.
Sorry for late answer. Yes, it would be solution for my case, because I know that points are coplanar. However, I think, that the problem of non-precise data is popular. Usually, the threshold can be estimated from the description of the task. So, in direct case I think using a tolerance is more expedient.
Then I think we should have two types of kwargs in the function signature: is_parallel_kwargs and is_coplanar_kwargs.
Yes, you are right. I solve this by adding an optional parameter tol to Plane.intersect_line. Also, it was necessary to add this parameter as optional to functions, which depend on Line.intersect_line. More detailed description I add as comment to pull request.
In general, numpy.linalg.optional, witch is called from is_coplanar, has only 2 optional parameters: tol=None and hermitian=False. However, I did't face wis situation when I need to change hermitian=False in context of this task.
Also, we can add an optional parameter dict_of_kwargs={'is_parallel_kwargs': is_parallel_kwargs, 'is_coplanar_kwargs': is_coplanar_kwargs} instead of **kwargs but we lose a backward compatibility then.
Would you be fine with me doing this, or would you rather I review your PR?
I would like you to review my PR
Hello I think you have some trailing whitespaces causing the automatic checks to fail.
trim trailing whitespace.................................................Failed
- hook id: trailing-whitespace
- exit code: 1
- files were modified by this hook
Fixing src/skspatial/objects/line.py
Fixing src/skspatial/objects/line_segment.py