Changed: Optimizes internal collections with key-based access

benches/microbenchmarks.rs

@@ -128,7 +128,7 @@ macro_rules! generate_dimensional_benchmarks {
                                     tds
                                 },
                                 |mut tds| {
-                                    let removed = tds.remove_duplicate_cells();
+                                    let removed = tds.remove_duplicate_cells().expect("remove_duplicate_cells failed");
                                     black_box((tds, removed));
                                 },
                             );

cspell.json

@@ -115,8 +115,10 @@
     "cospherical",
     "cppcheck",
     "cpus",
+    "derefs",
     "detekt",
     "docstrings",
+    "doctest",
     "dotenv",
     "downcasting",
     "dtolnay",

src/core/algorithms/bowyer_watson.rs

@@ -739,12 +739,19 @@ mod tests {
 
         // Step 2: Test bad cell detection (now using trait method)
         let bad_cells =
-            <IncrementalBoyerWatson<f64, Option<()>, Option<()>, 3> as InsertionAlgorithm<
+            match <IncrementalBoyerWatson<f64, Option<()>, Option<()>, 3> as InsertionAlgorithm<
                 f64,
                 Option<()>,
                 Option<()>,
                 3,
-            >>::find_bad_cells(&mut algorithm, &tds, &new_vertex);
+            >>::find_bad_cells(&mut algorithm, &tds, &new_vertex)
+            {
+                Ok(cells) => cells,
+                Err(e) => {
+                    println!("Bad cells detection error: {e}");
+                    vec![] // Continue test with empty bad cells
+                }
+            };
         println!("Bad cells found: {} cells", bad_cells.len());
         for &cell_key in &bad_cells {
             if let Some(cell) = tds.cells().get(cell_key) {

src/core/algorithms/robust_bowyer_watson.rs

@@ -5,7 +5,7 @@
 //! "No cavity boundary facets found" error.
 
 use crate::core::collections::MAX_PRACTICAL_DIMENSION_SIZE;
-use crate::core::collections::{FastHashMap, FastHashSet, SmallBuffer};
+use crate::core::collections::{CellKeySet, FastHashMap, FastHashSet, SmallBuffer};
 use crate::core::util::derive_facet_key_from_vertices;
 use std::marker::PhantomData;
 use std::ops::{AddAssign, Div, DivAssign, SubAssign};
@@ -269,7 +269,10 @@ where
             {
                 let cells_removed = bad_cells.len();
                 <Self as InsertionAlgorithm<T, U, V, D>>::remove_bad_cells(tds, &bad_cells);
-                <Self as InsertionAlgorithm<T, U, V, D>>::ensure_vertex_in_tds(tds, vertex);
+
+                // Ensure vertex is in TDS - if this fails, propagate the error
+                <Self as InsertionAlgorithm<T, U, V, D>>::ensure_vertex_in_tds(tds, vertex)?;
+
                 let cells_created =
                     <Self as InsertionAlgorithm<T, U, V, D>>::create_cells_from_boundary_facets(
                         tds,
@@ -321,7 +324,9 @@ where
             self.find_visible_boundary_facets_with_robust_fallback(tds, vertex)
             && !visible_facets.is_empty()
         {
-            <Self as InsertionAlgorithm<T, U, V, D>>::ensure_vertex_in_tds(tds, vertex);
+            // Ensure vertex is in TDS - if this fails, propagate the error
+            <Self as InsertionAlgorithm<T, U, V, D>>::ensure_vertex_in_tds(tds, vertex)?;
+
             let cells_created =
                 <Self as InsertionAlgorithm<T, U, V, D>>::create_cells_from_boundary_facets(
                     tds,
@@ -370,16 +375,39 @@ where
         [f64; D]: Default + DeserializeOwned + Serialize + Sized,
     {
         // First try to find bad cells using the trait's method
-        let mut bad_cells = InsertionAlgorithm::<T, U, V, D>::find_bad_cells(self, tds, vertex);
+        let mut bad_cells =
+            match InsertionAlgorithm::<T, U, V, D>::find_bad_cells(self, tds, vertex) {
+                Ok(cells) => cells,
+                Err(crate::core::traits::insertion_algorithm::BadCellsError::AllCellsBad {
+                    ..
+                }) => {
+                    // All cells marked as bad - try robust method to get a better result
+                    self.robust_find_bad_cells(tds, vertex)
+                }
+                Err(
+                    crate::core::traits::insertion_algorithm::BadCellsError::TooManyDegenerateCells(
+                        _,
+                    ),
+                ) => {
+                    // Too many degenerate cells - try robust method as fallback
+                    self.robust_find_bad_cells(tds, vertex)
+                }
+                Err(crate::core::traits::insertion_algorithm::BadCellsError::NoCells) => {
+                    // No cells - return empty
+                    return Vec::new();
+                }
+            };
 
         // If the standard method doesn't find any bad cells (likely a degenerate case)
-        // or we're using the robust configuration, use robust predicates as well
+        // or we're using the robust configuration, supplement with robust predicates
         if bad_cells.is_empty() || self.predicate_config.base_tolerance > T::default_tolerance() {
             let robust_bad_cells = self.robust_find_bad_cells(tds, vertex);
 
-            // Add any cells found by robust method that weren't found by the standard method
+            // Use a set for O(1) membership checking to avoid O(n²) complexity
+            let mut seen: CellKeySet = bad_cells.iter().copied().collect();
             for cell_key in robust_bad_cells {
-                if !bad_cells.contains(&cell_key) {
+                // Only add if not already present (insert returns true if new)
+                if seen.insert(cell_key) {
                     bad_cells.push(cell_key);
                 }
             }
@@ -520,7 +548,7 @@ where
             return Ok(boundary_facets);
         }
 
-        let bad_cell_set: FastHashSet<CellKey> = bad_cells.iter().copied().collect();
+        let bad_cell_set: CellKeySet = bad_cells.iter().copied().collect();
 
         // Build facet-to-cells mapping with enhanced validation
         let facet_to_cells = self.build_validated_facet_mapping(tds)?;
@@ -612,7 +640,7 @@ where
         }
 
         // Add the vertex to the TDS if it's not already there
-        <Self as InsertionAlgorithm<T, U, V, D>>::ensure_vertex_in_tds(tds, vertex);
+        <Self as InsertionAlgorithm<T, U, V, D>>::ensure_vertex_in_tds(tds, vertex)?;
 
         let cells_created =
             <Self as InsertionAlgorithm<T, U, V, D>>::create_cells_from_boundary_facets(
@@ -813,14 +841,23 @@ where
             if let Some(cell) = tds.cells().get(cell_key) {
                 if let Ok(facets) = cell.facets() {
                     let idx = usize::from(facet_index);
-                    debug_assert!(idx < facets.len(), "facet_index out of bounds");
                     if idx < facets.len() {
                         let facet = &facets[idx];
 
                         // Test visibility using robust orientation predicates with fallback
                         if self.is_facet_visible_from_vertex_robust(tds, facet, vertex, cell_key) {
                             visible_facets.push(facet.clone());
                         }
+                    } else {
+                        // Fail fast on invalid facet index - indicates TDS corruption
+                        return Err(TriangulationValidationError::InconsistentDataStructure {
+                            message: format!(
+                                "Facet index {} out of bounds (cell has {} facets) during visibility computation. \
+                                 This indicates triangulation data structure corruption.",
+                                idx,
+                                facets.len()
+                            ),
+                        });
                     }
                 } else {
                     return Err(TriangulationValidationError::InconsistentDataStructure {

src/core/boundary.rs

@@ -6,13 +6,14 @@
 use super::{
     facet::{Facet, FacetError},
     traits::{boundary_analysis::BoundaryAnalysis, data_type::DataType},
-    triangulation_data_structure::{CellKey, Tds},
+    triangulation_data_structure::Tds,
     util::derive_facet_key_from_vertices,
 };
-use crate::core::collections::fast_hash_map_with_capacity;
+use crate::core::collections::{KeyBasedCellMap, fast_hash_map_with_capacity};
 use crate::geometry::traits::coordinate::CoordinateScalar;
 use nalgebra::ComplexField;
 use serde::{Serialize, de::DeserializeOwned};
+use std::collections::hash_map::Entry;
 use std::iter::Sum;
 use std::ops::{AddAssign, Div, SubAssign};
 
@@ -59,7 +60,10 @@ where
     ///
     /// # Errors
     ///
-    /// Returns a [`FacetError`] if any boundary facet cannot be created from the cells.
+    /// Returns a [`FacetError`] if:
+    /// - Any boundary facet cannot be created from the cells
+    /// - A facet index is out of bounds (indicates data corruption)
+    /// - A referenced cell is not found in the triangulation (indicates data corruption)
     ///
     /// # Examples
     ///
@@ -89,44 +93,36 @@ where
 
         // Per-call cache to avoid repeated cell.facets() allocations
         // when multiple boundary facets reference the same cell
-        let mut cell_facets_cache: crate::core::collections::FastHashMap<
-            CellKey,
-            Vec<Facet<T, U, V, D>>,
-        > = fast_hash_map_with_capacity(self.number_of_cells());
+        let mut cell_facets_cache: KeyBasedCellMap<Vec<Facet<T, U, V, D>>> =
+            fast_hash_map_with_capacity(self.number_of_cells());
 
         // Collect all facets that belong to only one cell
         for (_facet_key, cells) in facet_to_cells {
-            if cells.len() == 1
-                && let Some((cell_id, facet_index)) = cells.first().copied()
-            {
+            if let [(cell_id, facet_index)] = cells.as_slice() {
+                // Bind dereferenced values once to avoid repetitive derefs
+                let (cell_id, facet_index) = (*cell_id, *facet_index);
                 if let Some(cell) = self.cells().get(cell_id) {
-                    // Cache facets per cell to avoid repeated allocations
-                    cell_facets_cache
-                        .entry(cell_id)
-                        .or_insert_with(|| cell.facets().unwrap_or_default());
-                    let facets = &cell_facets_cache[&cell_id];
+                    // Cache facets per cell to avoid repeated allocations, but propagate errors
+                    let facets = match cell_facets_cache.entry(cell_id) {
+                        Entry::Occupied(e) => e.into_mut(),
+                        Entry::Vacant(v) => {
+                            let computed = cell.facets()?; // propagate FacetError
+                            v.insert(computed)
+                        }
+                    };
 
                     if let Some(f) = facets.get(usize::from(facet_index)) {
                         boundary_facets.push(f.clone());
                     } else {
-                        debug_assert!(
-                            usize::from(facet_index) < facets.len(),
-                            "facet_index {} out of bounds for {} facets",
-                            facet_index,
-                            facets.len()
-                        );
-                        #[cfg(debug_assertions)]
-                        eprintln!(
-                            "boundary_facets: facet_index {facet_index:?} out of bounds for cell_id {cell_id:?}"
-                        );
+                        // Fail fast: invalid facet index indicates data corruption
+                        return Err(FacetError::InvalidFacetIndex {
+                            index: facet_index,
+                            facet_count: facets.len(),
+                        });
                     }
                 } else {
-                    debug_assert!(
-                        self.cells().contains_key(cell_id),
-                        "cell_id {cell_id:?} should exist in SlotMap"
-                    );
-                    #[cfg(debug_assertions)]
-                    eprintln!("boundary_facets: cell_id {cell_id:?} not found");
+                    // Fail fast: cell not found indicates data corruption
+                    return Err(FacetError::CellNotFoundInTriangulation);
                 }
             }
         }