Bug in principal direction extent

[eleftherioszisis]

There is a bug in the calculation of the principal direction extents:

NeuroM/neurom/morphmath.py

Lines 462 to 496 in fdf9f78

	def principal_direction_extent(points):
	"""Calculate the extent of a set of 3D points.
	The extent is defined as the maximum distance between
	the projections on the principal directions of the covariance matrix
	of the points.
	Parameter:
	points : a 2D numpy array of points
	Returns:
	extents : the extents for each of the eigenvectors of the cov matrix
	eigs : eigenvalues of the covariance matrix
	eigv : respective eigenvectors of the covariance matrix
	"""
	# pca can be biased by duplicate points
	points = np.unique(points, axis=0)
	# center the points around 0.0
	points -= np.mean(points, axis=0)
	# principal components
	_, eigv = pca(points)
	extent = np.zeros(3)
	for i in range(eigv.shape[1]):
	# orthogonal projection onto the direction of the v component
	scalar_projs = np.sort(np.array([np.dot(p, eigv[:, i]) for p in points]))
	extent[i] = scalar_projs[-1]
	if scalar_projs[0] < 0.:
	extent -= scalar_projs[0]
	return extent

At the point where the most negative value is subtracted from the entire extent:

         if scalar_projs[0] < 0.: 
             extent -= scalar_projs[0] 

It shouldn't subtract from the entire extent, rather from the i-th component extent[i].

Failing test example with points on a circle with radius 0.5, and center at 0.0:

from neurom.morphmath import principal_direction_extent
from numpy import testing as npt

def test_principal_direction_extent():

    circle_points = np.array([
        [ 5.0e-01,  0.0e+00,  0.0e+00],
        [ 4.7e-01,  1.6e-01,  0.0e+00],
        [ 3.9e-01,  3.1e-01,  0.0e+00],
        [ 2.7e-01,  4.2e-01,  0.0e+00],
        [ 1.2e-01,  4.8e-01,  0.0e+00],
        [-4.1e-02,  5.0e-01,  0.0e+00],
        [-2.0e-01,  4.6e-01,  0.0e+00],
        [-3.4e-01,  3.7e-01,  0.0e+00],
        [-4.4e-01,  2.4e-01,  0.0e+00],
        [-4.9e-01,  8.2e-02,  0.0e+00],
        [-4.9e-01, -8.2e-02,  0.0e+00],
        [-4.4e-01, -2.4e-01,  0.0e+00],
        [-3.4e-01, -3.7e-01,  0.0e+00],
        [-2.0e-01, -4.6e-01,  0.0e+00],
        [-4.1e-02, -5.0e-01,  0.0e+00],
        [ 1.2e-01, -4.8e-01,  0.0e+00],
        [ 2.7e-01, -4.2e-01,  0.0e+00],
        [ 3.9e-01, -3.1e-01,  0.0e+00],
        [ 4.7e-01, -1.6e-01,  0.0e+00],
        [ 5.0e-01, -1.2e-16,  0.0e+00]
    ])

    npt.assert_allclose(
        principal_direction_extent(circle_points),
        [1., 1., 0.],
    )

Which returns instead [1.49, 1.0, 0.0].

Thanks, @lidakanari for finding that there is something wrong with the principal direction extent function.

@arnaudon @adrien-berchet

[adrien-berchet]

Are you going to find a bug in every NeuroM feature? 😆
You're right again on this one.

[arnaudon]

Indeed! Also, I'm not sure I get this feature, extent shouldn't be the abs(max - min) of the projection, not just the max? Projection is on vector centered at mean of point cloud, so there will be positive and negative projections on these eigenvectors, no?

[eleftherioszisis]

@adrien-berchet , gotta catch them all! :D

@arnaudon , I will write it in a more clear way, but it is not wrong the way it is written. If the projections are not negative then the extent is equal to the greatest positive, whereas if there are negative projections, it also subtracts the smallest negative basically doing the abs(max - min),

[arnaudon]

Ah yes, you're right! Weird way to code that indeed haha