Merge pull request #1084 from Geode-solutions/fix/nnsearch-log

fix(NNSearch): add trace log for future debug on CI

Author: MelchiorSchuh

bindings/python/tests/geometry/test-py-nnsearch.py

@@ -55,10 +55,3 @@
     colocated_info = colocator.colocated_index_mapping(basic.GLOBAL_EPSILON)
     if colocated_info.nb_colocated_points() != 3:
         raise ValueError("[Test] Should be 3 colocated points")
-    mapping_answer = [0, 0, 1, 0, 2, 1, 3]
-    if colocated_info.colocated_mapping != mapping_answer:
-        raise ValueError("[Test] Error in colocated mapping")
-    points_answer = [p0, p1, p2, p3]
-    for p in range(4):
-        if colocated_info.unique_points[p] != points_answer[p]:
-            raise ValueError("[Test] Error in unique points")

src/geode/geometry/nn_search.cpp

@@ -35,25 +35,6 @@
 #include <geode/basic/logger.hpp>
 #include <geode/basic/pimpl_impl.hpp>
 
-namespace
-{
-    geode::index_t find_min_unmapped_element(
-        absl::Span< const geode::index_t > elements,
-        absl::Span< const geode::index_t > mapping )
-    {
-        geode::index_t min_index{ geode::NO_ID };
-        for( const auto e : elements )
-        {
-            if( mapping[e] != e )
-            {
-                continue;
-            }
-            min_index = std::min( min_index, e );
-        }
-        return min_index;
-    }
-} // namespace
-
 namespace geode
 {
     template < index_t dimension >
@@ -208,34 +189,33 @@ namespace geode
             "should be bigger than GLOBAL_EPSILON (i.e. ",
             GLOBAL_EPSILON, ")" );
         typename NNSearch< dimension >::ColocatedInfo result;
-        std::vector< index_t > mapping( nb_points() );
-        absl::c_iota( mapping, 0 );
+        std::vector< index_t > mapping( nb_points(), NO_ID );
         std::mutex mutex;
         async::parallel_for( async::irange( index_t{ 0 }, nb_points() ),
             [&epsilon, &mapping, &mutex, this]( index_t p ) {
-                if( mapping[p] != p )
+                if( mapping[p] != NO_ID )
                 {
                     return;
                 }
                 const auto vertices = radius_neighbors( point( p ), epsilon );
-                auto min_index = find_min_unmapped_element( vertices, mapping );
                 std::lock_guard< std::mutex > lock( mutex );
-                if( mapping[p] != p )
+                if( mapping[p] != NO_ID )
                 {
+                    Logger::trace(
+                        p, " : correction 1 / mapping[p] ", mapping[p] );
                     return;
                 }
-                if( min_index != mapping[min_index] )
-                {
-                    min_index = find_min_unmapped_element( vertices, mapping );
-                }
                 for( const auto id : vertices )
                 {
-                    if( id == mapping[id] )
+                    Logger::trace(
+                        p, " : id ", id, " / mapping[id] ", mapping[id] );
+                    if( mapping[id] == NO_ID )
                     {
-                        mapping[id] = min_index;
+                        mapping[id] = p;
                     }
                 }
             } );
+        result.colocated_input_points = mapping;
         index_t nb_unique_points{ 0 };
         for( const auto p : Range{ nb_points() } )
         {
@@ -244,23 +224,22 @@ namespace geode
                 nb_unique_points++;
             }
         }
-        result.colocated_input_points = mapping;
-        index_t nb_colocated{ 0 };
-        index_t count{ 0 };
         result.unique_points.resize( nb_unique_points );
+        std::vector< index_t > old2new( nb_points(), NO_ID );
+        index_t count{ 0 };
         for( const auto p : Range{ nb_points() } )
         {
             if( mapping[p] == p )
             {
-                mapping[p] -= nb_colocated;
-                result.unique_points[count++] = point( p );
-            }
-            else
-            {
-                nb_colocated++;
-                mapping[p] = mapping[mapping[p]];
+                result.unique_points[count] = point( p );
+                old2new[p] = count;
+                count++;
             }
         }
+        for( const auto p : Range{ nb_points() } )
+        {
+            mapping[p] = old2new[mapping[p]];
+        }
         result.colocated_mapping = mapping;
         return result;
     }

tests/geometry/test-nnsearch.cpp

@@ -57,23 +57,46 @@ void first_test()
     const geode::Point3D p1{ { 2.4, 8.1, 7.6 } };
     const geode::Point3D p2{ { 8.1, 4.2, 3.8 } };
     const geode::Point3D p3{ { 3.1, 9.4, 9.7 } };
-    const geode::NNSearch3D colocator( { p0, p0, p1, p0, p2, p1, p3 } );
+    std::vector< geode::Point3D > points{ p0, p0, p1, p0, p2, p1, p3 };
+    const geode::NNSearch3D colocator( points );
 
     const auto colocated_info =
         colocator.colocated_index_mapping( geode::GLOBAL_EPSILON );
     OPENGEODE_EXCEPTION( colocated_info.nb_colocated_points() == 3,
-        "[Test] Should be 3 colocated points" );
-    const std::vector< geode::index_t > mapping_answer{ 0, 0, 1, 0, 2, 1, 3 };
-    OPENGEODE_EXCEPTION( colocated_info.colocated_mapping == mapping_answer,
-        "[Test] Error in colocated mapping" );
-    const std::vector< geode::Point3D > points_answer{ p0, p1, p2, p3 };
-    OPENGEODE_EXCEPTION( colocated_info.unique_points == points_answer,
-        "[Test] Error in unique points" );
+        "[Test 1] Should be 3 colocated points" );
+    for( const auto p : geode::Indices{ points } )
+    {
+        OPENGEODE_EXCEPTION( colocated_info.colocated_mapping[p]
+                                 < colocated_info.unique_points.size(),
+            "[Test 1] Wrong value of colocated_mapping (bigger than unique "
+            "points size)" );
+        const auto& colocated_point =
+            colocated_info.unique_points[colocated_info.colocated_mapping[p]];
+        OPENGEODE_EXCEPTION(
+            geode::point_point_distance( points[p], colocated_point )
+                <= geode::GLOBAL_EPSILON,
+            "[Test 1] Colocated point is not close enough to original point" );
+    }
+    for( const auto up0 : geode::Indices{ colocated_info.unique_points } )
+    {
+        for( const auto up1 : geode::Indices{ colocated_info.unique_points } )
+        {
+            if( up1 <= up0 )
+            {
+                continue;
+            }
+            OPENGEODE_EXCEPTION(
+                geode::point_point_distance( colocated_info.unique_points[up0],
+                    colocated_info.unique_points[up1] )
+                    > geode::GLOBAL_EPSILON,
+                "[Test 1] Colocated points are too close" );
+        }
+    }
 }
 
 void second_test()
 {
-    geode::Logger::set_level( geode::Logger::LEVEL::debug );
+    geode::Logger::set_level( geode::Logger::LEVEL::trace );
     std::vector< geode::Point2D > points{
         geode::Point2D{ { 0, 0.2 } },
         geode::Point2D{ { 0.25, 0 } },
@@ -87,18 +110,24 @@ void second_test()
 
     const auto colocated_info = colocator.colocated_index_mapping( DISTANCE );
     DEBUG( colocated_info.nb_unique_points() );
+    for( const auto v : geode::Range{ 6 } )
+    {
+        geode::Logger::trace( v, " : ",
+            colocated_info.colocated_input_points[v], " / ",
+            colocated_info.colocated_mapping[v] );
+    }
     for( const auto p : geode::Indices{ points } )
     {
         OPENGEODE_EXCEPTION( colocated_info.colocated_mapping[p]
                                  < colocated_info.unique_points.size(),
-            "[Test] Wrong value of colocated_mapping (bigger than unique "
+            "[Test 2] Wrong value of colocated_mapping (bigger than unique "
             "points size)" );
         const auto& colocated_point =
             colocated_info.unique_points[colocated_info.colocated_mapping[p]];
         OPENGEODE_EXCEPTION(
             geode::point_point_distance( points[p], colocated_point )
                 <= DISTANCE,
-            "[Test] Colocated point is not close enough to original point" );
+            "[Test 2] Colocated point is not close enough to original point" );
     }
     for( const auto up0 : geode::Indices{ colocated_info.unique_points } )
     {
@@ -112,11 +141,11 @@ void second_test()
                 geode::point_point_distance( colocated_info.unique_points[up0],
                     colocated_info.unique_points[up1] )
                     > DISTANCE,
-                "[Test] Colocated points are too close" );
+                "[Test 2] Colocated points are too close" );
         }
     }
     OPENGEODE_EXCEPTION( colocated_info.nb_unique_points() == 2,
-        "[Test] Should be 2 unique points" );
+        "[Test 2] Should be 2 unique points" );
 }
 
 void test()

tests/mesh/test-merge-surface.cpp

@@ -136,8 +136,9 @@ void test_import()
     }
     geode::NNSearch3D nns( points );
     const auto mappings = nns.colocated_index_mapping( geode::GLOBAL_EPSILON );
+    DEBUG( mappings.nb_colocated_points() );
     OPENGEODE_EXCEPTION( mappings.nb_colocated_points() == 0,
-        "[Test] Should be nomore colocated points" );
+        "[Test] Should be no more colocated points" );
     for( const auto p : geode::Indices{ points } )
     {
         OPENGEODE_EXCEPTION(