From e84da46e84d6b0b4ea893ecfc39126d5b708fa1c Mon Sep 17 00:00:00 2001 From: Thibault Lejemble Date: Fri, 4 Aug 2023 12:02:40 +0200 Subject: [PATCH 1/6] fix status seperate the case where the Base class is not STABLE (then the current status is simply returned) and where there is not enough neighbors (then the current status is set to UNDEFINED and returned) --- Ponca/src/Fitting/covarianceFit.hpp | 6 +++--- Ponca/src/Fitting/orientedSphereFit.hpp | 4 +++- Ponca/src/Fitting/sphereFit.hpp | 4 +++- Ponca/src/Fitting/unorientedSphereFit.hpp | 4 +++- 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/Ponca/src/Fitting/covarianceFit.hpp b/Ponca/src/Fitting/covarianceFit.hpp index aaddb9e73..461f6a18c 100644 --- a/Ponca/src/Fitting/covarianceFit.hpp +++ b/Ponca/src/Fitting/covarianceFit.hpp @@ -35,11 +35,11 @@ FIT_RESULT CovarianceFitBase::finalize () { // handle specific configurations + if(Base::finalize() != STABLE) + return Base::m_eCurrentState; // With less than 3 neighbors the fitting is undefined - if(Base::finalize() != STABLE || Base::getNumNeighbors() < 3) - { + if(Base::getNumNeighbors() < 3) return Base::m_eCurrentState = UNDEFINED; - } // Center the covariance on the centroid auto centroid = Base::barycenter(); diff --git a/Ponca/src/Fitting/orientedSphereFit.hpp b/Ponca/src/Fitting/orientedSphereFit.hpp index e65422509..5f30be6a1 100644 --- a/Ponca/src/Fitting/orientedSphereFit.hpp +++ b/Ponca/src/Fitting/orientedSphereFit.hpp @@ -41,8 +41,10 @@ OrientedSphereFitImpl::finalize () PONCA_MULTIARCH_STD_MATH(abs); // Compute status - if(Base::finalize() != STABLE || Base::getNumNeighbors() < 3) + if(Base::finalize() != STABLE) return Base::m_eCurrentState; + if(Base::getNumNeighbors() < 3) + return Base::m_eCurrentState = UNDEFINED; if (Base::algebraicSphere().isValid()) Base::m_eCurrentState = CONFLICT_ERROR_FOUND; else diff --git a/Ponca/src/Fitting/sphereFit.hpp b/Ponca/src/Fitting/sphereFit.hpp index 009bf93dd..8e34e11e2 100644 --- a/Ponca/src/Fitting/sphereFit.hpp +++ b/Ponca/src/Fitting/sphereFit.hpp @@ -41,7 +41,9 @@ FIT_RESULT SphereFitImpl::finalize () { // Compute status - if(Base::finalize() != STABLE || Base::getNumNeighbors() < 3) + if(Base::finalize() != STABLE) + return Base::m_eCurrentState; + if(Base::getNumNeighbors() < 3) return Base::m_eCurrentState = UNDEFINED; if (Base::algebraicSphere().isValid()) Base::m_eCurrentState = CONFLICT_ERROR_FOUND; diff --git a/Ponca/src/Fitting/unorientedSphereFit.hpp b/Ponca/src/Fitting/unorientedSphereFit.hpp index 5104a9811..ddf757fa4 100644 --- a/Ponca/src/Fitting/unorientedSphereFit.hpp +++ b/Ponca/src/Fitting/unorientedSphereFit.hpp @@ -90,8 +90,10 @@ UnorientedSphereFitImpl::finalize () constexpr int Dim = DataPoint::Dim; // Compute status - if(Base::finalize() != STABLE || Base::getNumNeighbors() < 3) + if(Base::finalize() != STABLE) return Base::m_eCurrentState; + if(Base::getNumNeighbors() < 3) + return Base::m_eCurrentState = UNDEFINED; if (Base::algebraicSphere().isValid()) Base::m_eCurrentState = CONFLICT_ERROR_FOUND; else From 38ca6e07798a47c1ae988bbe3f9be5e8faf147d8 Mon Sep 17 00:00:00 2001 From: Thibault Lejemble Date: Fri, 4 Aug 2023 12:05:11 +0200 Subject: [PATCH 2/6] clean primitive finalize - remove comments about the mean - remove call to init() since it does nothing in this case --- Ponca/src/Fitting/primitive.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/Ponca/src/Fitting/primitive.h b/Ponca/src/Fitting/primitive.h index 4a970e2de..51653886a 100644 --- a/Ponca/src/Fitting/primitive.h +++ b/Ponca/src/Fitting/primitive.h @@ -120,9 +120,7 @@ class PrimitiveBase PONCA_FITTING_APIDOC_FINALIZE PONCA_MULTIARCH inline FIT_RESULT finalize(){ // handle specific configurations - // We need to have at least one neighbor to compute the mean if (m_sumW == Scalar(0.) || m_nbNeighbors < 1) { - init( m_w.basisCenter() ); return m_eCurrentState = UNDEFINED; } return m_eCurrentState = STABLE; From 6b99c976bfeda17db3e570595d80b2da61a921e9 Mon Sep 17 00:00:00 2001 From: Thibault Lejemble Date: Fri, 4 Aug 2023 12:06:18 +0200 Subject: [PATCH 3/6] check for CONFLICT_ERROR_FOUND in CovarianceLineFitImpl as done in CovariancePlaneFitImpl --- Ponca/src/Fitting/covarianceLineFit.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Ponca/src/Fitting/covarianceLineFit.h b/Ponca/src/Fitting/covarianceLineFit.h index 9e4db6cd8..273e3b66d 100644 --- a/Ponca/src/Fitting/covarianceLineFit.h +++ b/Ponca/src/Fitting/covarianceLineFit.h @@ -50,8 +50,10 @@ class CovarianceLineFitImpl : public T PONCA_MULTIARCH inline FIT_RESULT finalize() { static const int smallestEigenValue = DataPoint::Dim - 1; - if (Base::finalize() == STABLE) + if (Base::finalize() == STABLE) { + if (Base::line().isValid()) Base::m_eCurrentState = CONFLICT_ERROR_FOUND; Base::setLine(Base::barycenter(), Base::m_solver.eigenvectors().col(smallestEigenValue).normalized()); + } return Base::m_eCurrentState; } }; From 204fd632c74ebbe784ffbd771eb24b6dada9a616 Mon Sep 17 00:00:00 2001 From: Thibault Lejemble Date: Fri, 4 Aug 2023 12:09:29 +0200 Subject: [PATCH 4/6] clarify FIT_RESULT comments remove neighbor count threshold that depends on fitting procedures --- Ponca/src/Fitting/enums.h | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/Ponca/src/Fitting/enums.h b/Ponca/src/Fitting/enums.h index a461f7be3..799ceb1e5 100644 --- a/Ponca/src/Fitting/enums.h +++ b/Ponca/src/Fitting/enums.h @@ -9,18 +9,16 @@ namespace Ponca { /*! - Enum corresponding to the state of a fitting method (and what the finalize function can return + Enum corresponding to the state of a fitting method (and what the finalize function returns) */ enum FIT_RESULT : unsigned char { - /*! \brief The fitting is stable an ready to use (and having more than 6 - neighbours)*/ + /*! \brief The fitting is stable and ready to use*/ STABLE = 0, - /*! \brief The fitting is ready to use but it can be unstable (and - having between 3 and 6 neighbors)*/ + /*! \brief The fitting is ready to use but it is considered + as unstable (if the number of neighbors is low for example)*/ UNSTABLE = 1, - /*! \brief The fitting is undefined, you can't use it for valid results - (and having less than 3 neighbors)*/ + /*! \brief The fitting is undefined, you can't use it for valid results*/ UNDEFINED = 2, /*! \brief The fitting procedure needs to analyse the neighborhood another time*/ From e244a0480d3c3b4f73e49b31d0a2efddc92a5ea2 Mon Sep 17 00:00:00 2001 From: Thibault Lejemble Date: Fri, 4 Aug 2023 16:35:02 +0200 Subject: [PATCH 5/6] use Dim for stability check --- Ponca/src/Fitting/covarianceFit.hpp | 4 ++-- Ponca/src/Fitting/orientedSphereFit.hpp | 4 ++-- Ponca/src/Fitting/sphereFit.hpp | 4 ++-- Ponca/src/Fitting/unorientedSphereFit.hpp | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Ponca/src/Fitting/covarianceFit.hpp b/Ponca/src/Fitting/covarianceFit.hpp index 461f6a18c..f228ed001 100644 --- a/Ponca/src/Fitting/covarianceFit.hpp +++ b/Ponca/src/Fitting/covarianceFit.hpp @@ -37,8 +37,8 @@ CovarianceFitBase::finalize () // handle specific configurations if(Base::finalize() != STABLE) return Base::m_eCurrentState; - // With less than 3 neighbors the fitting is undefined - if(Base::getNumNeighbors() < 3) + // With less than Dim neighbors the fitting is undefined + if(Base::getNumNeighbors() < DataPoint::Dim) return Base::m_eCurrentState = UNDEFINED; // Center the covariance on the centroid diff --git a/Ponca/src/Fitting/orientedSphereFit.hpp b/Ponca/src/Fitting/orientedSphereFit.hpp index 5f30be6a1..764996b18 100644 --- a/Ponca/src/Fitting/orientedSphereFit.hpp +++ b/Ponca/src/Fitting/orientedSphereFit.hpp @@ -43,12 +43,12 @@ OrientedSphereFitImpl::finalize () // Compute status if(Base::finalize() != STABLE) return Base::m_eCurrentState; - if(Base::getNumNeighbors() < 3) + if(Base::getNumNeighbors() < DataPoint::Dim) return Base::m_eCurrentState = UNDEFINED; if (Base::algebraicSphere().isValid()) Base::m_eCurrentState = CONFLICT_ERROR_FOUND; else - Base::m_eCurrentState = Base::getNumNeighbors() < 6 ? UNSTABLE : STABLE; + Base::m_eCurrentState = Base::getNumNeighbors() < 2*DataPoint::Dim ? UNSTABLE : STABLE; // 1. finalize sphere fitting Scalar epsilon = Eigen::NumTraits::dummy_precision(); diff --git a/Ponca/src/Fitting/sphereFit.hpp b/Ponca/src/Fitting/sphereFit.hpp index 8e34e11e2..7d0ec6a5f 100644 --- a/Ponca/src/Fitting/sphereFit.hpp +++ b/Ponca/src/Fitting/sphereFit.hpp @@ -43,12 +43,12 @@ SphereFitImpl::finalize () // Compute status if(Base::finalize() != STABLE) return Base::m_eCurrentState; - if(Base::getNumNeighbors() < 3) + if(Base::getNumNeighbors() < DataPoint::Dim) return Base::m_eCurrentState = UNDEFINED; if (Base::algebraicSphere().isValid()) Base::m_eCurrentState = CONFLICT_ERROR_FOUND; else - Base::m_eCurrentState = Base::getNumNeighbors() < 6 ? UNSTABLE : STABLE; + Base::m_eCurrentState = Base::getNumNeighbors() < 2*DataPoint::Dim ? UNSTABLE : STABLE; MatrixA matC; matC.setIdentity(); diff --git a/Ponca/src/Fitting/unorientedSphereFit.hpp b/Ponca/src/Fitting/unorientedSphereFit.hpp index ddf757fa4..6800dec96 100644 --- a/Ponca/src/Fitting/unorientedSphereFit.hpp +++ b/Ponca/src/Fitting/unorientedSphereFit.hpp @@ -92,12 +92,12 @@ UnorientedSphereFitImpl::finalize () // Compute status if(Base::finalize() != STABLE) return Base::m_eCurrentState; - if(Base::getNumNeighbors() < 3) + if(Base::getNumNeighbors() < DataPoint::Dim) return Base::m_eCurrentState = UNDEFINED; if (Base::algebraicSphere().isValid()) Base::m_eCurrentState = CONFLICT_ERROR_FOUND; else - Base::m_eCurrentState = Base::getNumNeighbors() < 6 ? UNSTABLE : STABLE; + Base::m_eCurrentState = Base::getNumNeighbors() < 2*DataPoint::Dim ? UNSTABLE : STABLE; // 1. finalize sphere fitting Scalar invSumW = Scalar(1.) / Base::getWeightSum(); From 23eed6634478ace533693c1f3eae44dbe9679a15 Mon Sep 17 00:00:00 2001 From: Thibault Lejemble Date: Fri, 4 Aug 2023 12:52:10 +0200 Subject: [PATCH 6/6] Update Changelog --- CHANGELOG | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG b/CHANGELOG index 90967bf80..9ccc6144f 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -10,6 +10,10 @@ Current head (v.1.2 RC) - Bug-fixes and code improvements - [fitting] Use variadic template for basket extensions (#85) + - [fitting] Fix current status issue (#108) + +-Docs + - [fitting] Clarify documentation on FIT_RESULT (#108) -------------------------------------------------------------------------------- v.1.1 RC