intersection line are not complanar with line in plane about scikit-spatial HOT 13 CLOSED

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 scikit-spatial.

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

from scikit-spatial.

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.

from scikit-spatial.

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.

from scikit-spatial.

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)
False

from scikit-spatial.

However, 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.

from scikit-spatial.

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.

from scikit-spatial.

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.

from scikit-spatial.

Then I think we should have two types of kwargs in the function signature: is_parallel_kwargs and is_coplanar_kwargs.

from scikit-spatial.

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.

from scikit-spatial.

Would you be fine with me doing this, or would you rather I review your PR?

from scikit-spatial.

I would like you to review my PR

from scikit-spatial.

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

from scikit-spatial.