From 655096cdc4af2f61fe4cd1e8a5b6f183ad730144 Mon Sep 17 00:00:00 2001 From: Adam Getchell Date: Tue, 19 May 2026 13:10:31 -0700 Subject: [PATCH 1/5] perf(core): localize remove_vertex repair - Avoid global simplex scans and incident-simplex rebuilds on successful vertex removal by using the affected vertex star and scoped repair. - Promote and validate orientation over the touched removal scope, with the full global orientation path retained as a fallback. - Add a Criterion remove_vertex benchmark covering successful removals and invalid-remnant rollback across 2D through 5D. --- Cargo.toml | 5 + benches/README.md | 2 + benches/remove_vertex.rs | 317 ++++++++++++++++++++++++++++ src/core/repair.rs | 441 ++++++++++++++++++++++++++++++--------- 4 files changed, 669 insertions(+), 96 deletions(-) create mode 100644 benches/remove_vertex.rs diff --git a/Cargo.toml b/Cargo.toml index e2e4f2fa..096073db 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -98,6 +98,11 @@ name = "tds_clone" path = "benches/tds_clone.rs" harness = false +[[bench]] +name = "remove_vertex" +path = "benches/remove_vertex.rs" +harness = false + [[bench]] name = "ci_performance_suite" path = "benches/ci_performance_suite.rs" diff --git a/benches/README.md b/benches/README.md index fe99816f..b1a2e190 100644 --- a/benches/README.md +++ b/benches/README.md @@ -12,6 +12,7 @@ predicates fast across 2D-5D. | `circumsphere_containment.rs` | Compare circumsphere predicate methods | 2D-5D fixed, 3D random, edge cases | ~5 min | Predicate tuning, summaries | | `cold_path_predicates.rs` | Track hot/cold predicate paths | 2D-5D hot queries, near-boundary cases | ~2-5 min | Predicate optimization work | | `profiling_suite.rs` | Large-scale construction, memory, query, validation profiling | 2D/3D 10k, 4D 3k, 5D 1k | ~2-3 hr | Manual/monthly | +| `remove_vertex.rs` | Track topology-preserving vertex removal and invalid-remnant rollback | Deterministic 2D-5D triangulations | ~1-5 min | Vertex-removal/local-validation work | | `tds_clone.rs` | `Tds::clone()` snapshot cost | Deterministic 2D-5D triangulations | ~1-3 min | Rollback design baselines | | `topology_guarantee_construction.rs` | Cost of topology guarantee modes | 2D-5D construction cases | ~5-15 min | Manual topology policy work | @@ -29,6 +30,7 @@ predicates fast across 2D-5D. | Compare against an existing baseline | `just perf-compare ` | | Release performance summary | `just bench-perf-summary` | | Smoke-test benchmark harnesses | `just bench-smoke` | +| Vertex removal mutation baseline | `cargo bench --profile perf --bench remove_vertex -- --noplot` | | Predicate comparison | `cargo bench --profile perf --bench circumsphere_containment -- --noplot` | | Predicate cold-path work | `cargo bench --profile perf --bench cold_path_predicates -- --noplot` | | Large-scale scaling suite | `cargo bench --profile perf --bench profiling_suite -- --noplot` | diff --git a/benches/remove_vertex.rs b/benches/remove_vertex.rs new file mode 100644 index 00000000..67fed5ee --- /dev/null +++ b/benches/remove_vertex.rs @@ -0,0 +1,317 @@ +#![forbid(unsafe_code)] + +//! Benchmark: `DelaunayTriangulation::remove_vertex` mutation and rollback cost. +//! +//! This benchmark separates vertex-removal cost from construction cost by building +//! deterministic source triangulations once, then cloning them in Criterion setup +//! before timing the removal call itself. The timed path still includes the +//! operation's own transactional snapshot and invariant validation, which is the +//! behavior this benchmark is meant to track. +//! +//! Intended for **manual** runs (not part of the CI performance suite). +//! +//! Run with: +//! ```bash +//! cargo bench --profile perf --bench remove_vertex +//! ``` + +use criterion::{BatchSize, BenchmarkId, Criterion, Throughput, criterion_group, criterion_main}; +use delaunay::prelude::construction::{DelaunayTriangulation, Vertex}; +use delaunay::prelude::generators::generate_random_points_seeded; +use delaunay::prelude::geometry::{AdaptiveKernel, Coordinate, Point}; +use delaunay::prelude::tds::VertexKey; +use std::hint::black_box; +use std::time::Duration; + +/// Shared benchmark setup error helpers. +#[path = "common/bench_utils.rs"] +pub mod bench_utils; +use bench_utils::{bench_option, bench_result}; + +const INTERIOR_BOUNDS: (f64, f64) = (0.0, 1.0); +const INTERIOR_RADIUS_MIN: f64 = 0.15; +const INTERIOR_RADIUS_SPAN: f64 = 0.70; +const SEED_SALT: u64 = 0x9E37_79B9_7F4A_7C15; +const SEED_SEARCH_ATTEMPTS: usize = 64; +const SAMPLE_SIZE: usize = 10; +const WARM_UP_TIME: Duration = Duration::from_millis(500); +const MEASUREMENT_TIME: Duration = Duration::from_secs(2); + +type BenchTriangulation = DelaunayTriangulation, (), (), D>; + +struct RemovalSource { + vertex_count: usize, + simplex_count: usize, + triangulation: BenchTriangulation, + vertex_key: VertexKey, +} + +/// Derive a deterministic, dimension-specific seed for one benchmark case. +fn seed_for_case(requested_vertices: usize, seed_base: u64) -> u64 { + let vertices = bench_result( + u64::try_from(requested_vertices), + "vertex count does not fit in u64", + ); + let dimension = bench_result(u64::try_from(D), "dimension does not fit in u64"); + seed_base ^ vertices.wrapping_mul(SEED_SALT) ^ dimension.rotate_left(32) +} + +/// Generate a reproducible canonical simplex with interior points. +fn generate_vertices( + requested_vertices: usize, + seed: u64, +) -> Vec> { + let interior_count = requested_vertices.saturating_sub(D + 1); + let mut points = simplex_points::(); + let raw_points = bench_result( + generate_random_points_seeded::(interior_count, INTERIOR_BOUNDS, seed), + format!("failed to generate {D}D interior benchmark points"), + ); + + for (index, raw_point) in raw_points.iter().enumerate() { + let radius = interior_radius(index); + let direction = normalized_positive_direction(raw_point); + let mut coords = [0.0; D]; + for (coord, direction_coord) in coords.iter_mut().zip(direction) { + *coord = radius * direction_coord; + } + points.push(Point::new(coords)); + } + + Vertex::from_points(&points) +} + +/// Generate the minimal full-dimensional simplex points. +fn simplex_points() -> Vec> { + let mut points = Vec::with_capacity(D + 1); + points.push(Point::new([0.0; D])); + + for axis in 0..D { + let mut coords = [0.0; D]; + coords[axis] = 1.0; + points.push(Point::new(coords)); + } + + points +} + +/// Generate the minimal full-dimensional simplex for the rollback benchmark. +fn simplex_vertices() -> Vec> { + Vertex::from_points(&simplex_points::()) +} + +/// Deterministic radial coordinate for a point inside the canonical simplex. +fn interior_radius(index: usize) -> f64 { + let numerator = bench_result( + u32::try_from(index.wrapping_mul(37) % 997), + "interior radius numerator does not fit in u32", + ); + INTERIOR_RADIUS_MIN + INTERIOR_RADIUS_SPAN * f64::from(numerator) / 997.0 +} + +/// Convert a random point in `[0, 1]^D` into a positive simplex direction. +fn normalized_positive_direction(point: &Point) -> [f64; D] { + let mut weights = [0.0; D]; + let mut weight_sum = 0.0; + + for (weight, coordinate) in weights.iter_mut().zip(point.coords()) { + *weight = coordinate + f64::EPSILON; + weight_sum += *weight; + } + + for weight in &mut weights { + *weight /= weight_sum; + } + + weights +} + +/// Find a vertex whose removal succeeds for the prepared triangulation. +fn successful_removal_vertex( + triangulation: &BenchTriangulation, +) -> Option { + for (vertex_key, _) in triangulation.vertices() { + let mut candidate = triangulation.clone(); + if candidate.remove_vertex(vertex_key).is_ok() { + return Some(vertex_key); + } + } + + None +} + +/// Build the source triangulation for successful interior-removal measurements. +fn build_success_source( + requested_vertices: usize, + seed_base: u64, +) -> RemovalSource { + for attempt in 0..SEED_SEARCH_ATTEMPTS { + let attempt_seed = bench_result(u64::try_from(attempt), "seed attempt does not fit in u64"); + let seed = seed_for_case::(requested_vertices, seed_base) + ^ attempt_seed.wrapping_mul(SEED_SALT.rotate_left(17)); + let vertices = generate_vertices::(requested_vertices, seed); + let Ok(triangulation) = DelaunayTriangulation::new(&vertices) else { + continue; + }; + let Some(vertex_key) = successful_removal_vertex(&triangulation) else { + continue; + }; + + return RemovalSource { + vertex_count: triangulation.number_of_vertices(), + simplex_count: triangulation.number_of_simplices(), + triangulation, + vertex_key, + }; + } + + bench_option( + None, + format!( + "no successful {D}D remove_vertex fixture found for {requested_vertices} vertices \ + after {SEED_SEARCH_ATTEMPTS} seeds" + ), + ) +} + +/// Build the source triangulation for invalid-removal rollback measurements. +fn build_rollback_source() -> RemovalSource { + let vertices = simplex_vertices::(); + let triangulation: BenchTriangulation = bench_result( + DelaunayTriangulation::new(&vertices), + format!("failed to build {D}D rollback benchmark simplex"), + ); + let vertex_key = bench_option( + triangulation.vertices().next().map(|(key, _)| key), + format!("rollback benchmark simplex has no {D}D vertices"), + ); + + RemovalSource { + vertex_count: triangulation.number_of_vertices(), + simplex_count: triangulation.number_of_simplices(), + triangulation, + vertex_key, + } +} + +/// Report benchmark throughput in total stored vertices plus simplices. +fn triangulation_element_count(source: &RemovalSource) -> u64 { + let total_elements = source.vertex_count + source.simplex_count; + bench_result( + u64::try_from(total_elements), + "triangulation element count does not fit in u64", + ) +} + +/// Register the successful-removal cases for one dimension and input-size schedule. +fn bench_success_dimension( + c: &mut Criterion, + dim_label: &str, + counts: &[usize], + seed_base: u64, +) { + let mut group = c.benchmark_group(format!("remove_vertex/success/{dim_label}")); + group.sample_size(SAMPLE_SIZE); + group.warm_up_time(WARM_UP_TIME); + group.measurement_time(MEASUREMENT_TIME); + + for &requested_vertices in counts { + let source = build_success_source::(requested_vertices, seed_base); + group.throughput(Throughput::Elements(triangulation_element_count(&source))); + + group.bench_with_input( + BenchmarkId::new( + "remove_vertex", + format!( + "vertices_{}_simplices_{}", + source.vertex_count, source.simplex_count + ), + ), + &source, + |b, source| { + b.iter_batched( + || source.triangulation.clone(), + |mut triangulation| { + black_box(bench_result( + triangulation.remove_vertex(source.vertex_key), + "successful remove_vertex benchmark unexpectedly failed", + )); + }, + BatchSize::SmallInput, + ); + }, + ); + } + + group.finish(); +} + +/// Register the minimal-simplex rollback case for one dimension. +fn bench_rollback_dimension(c: &mut Criterion, dim_label: &str) { + let source = build_rollback_source::(); + let mut group = c.benchmark_group(format!("remove_vertex/rollback/{dim_label}")); + group.sample_size(SAMPLE_SIZE); + group.warm_up_time(WARM_UP_TIME); + group.measurement_time(MEASUREMENT_TIME); + group.throughput(Throughput::Elements(triangulation_element_count(&source))); + + group.bench_with_input( + BenchmarkId::new( + "remove_vertex_invalid_remnant", + format!( + "vertices_{}_simplices_{}", + source.vertex_count, source.simplex_count + ), + ), + &source, + |b, source| { + b.iter_batched( + || source.triangulation.clone(), + |mut triangulation| { + black_box(triangulation.remove_vertex(source.vertex_key).unwrap_err()); + }, + BatchSize::SmallInput, + ); + }, + ); + + group.finish(); +} + +/// Benchmark successful 2D vertex removal. +fn bench_remove_vertex_success_2d(c: &mut Criterion) { + bench_success_dimension::<2>(c, "2d", &[100, 500, 2_000], 0xD2AA_0000_0000_0001); +} + +/// Benchmark successful 3D vertex removal. +fn bench_remove_vertex_success_3d(c: &mut Criterion) { + bench_success_dimension::<3>(c, "3d", &[50, 150, 500], 0xD3AA_0000_0000_0002); +} + +/// Benchmark successful 4D vertex removal. +fn bench_remove_vertex_success_4d(c: &mut Criterion) { + bench_success_dimension::<4>(c, "4d", &[20, 50, 100], 0xD4AA_0000_0000_0003); +} + +/// Benchmark successful 5D vertex removal. +fn bench_remove_vertex_success_5d(c: &mut Criterion) { + bench_success_dimension::<5>(c, "5d", &[12, 25, 40], 0xD5AA_0000_0000_0004); +} + +/// Benchmark rollback for invalid lower-dimensional remnants. +fn bench_remove_vertex_rollback(c: &mut Criterion) { + bench_rollback_dimension::<2>(c, "2d"); + bench_rollback_dimension::<3>(c, "3d"); + bench_rollback_dimension::<4>(c, "4d"); + bench_rollback_dimension::<5>(c, "5d"); +} + +criterion_group!( + benches, + bench_remove_vertex_success_2d, + bench_remove_vertex_success_3d, + bench_remove_vertex_success_4d, + bench_remove_vertex_success_5d, + bench_remove_vertex_rollback +); +criterion_main!(benches); diff --git a/src/core/repair.rs b/src/core/repair.rs index be191d69..08a76ed0 100644 --- a/src/core/repair.rs +++ b/src/core/repair.rs @@ -4,16 +4,16 @@ //! repair, and vertex-removal cavity retriangulation for [`Triangulation`](crate::core::triangulation::Triangulation). use crate::core::algorithms::incremental_insertion::{ - CavityFillingError, InsertionError, external_facets_for_boundary, repair_neighbor_pointers, - repair_neighbor_pointers_local, wire_cavity_neighbors, + CavityFillingError, InsertionError, external_facets_for_boundary, + fill_cavity_replacing_simplices, repair_neighbor_pointers, repair_neighbor_pointers_local, + wire_cavity_neighbors, }; use crate::core::algorithms::locate::extract_cavity_boundary; use crate::core::collections::{ FacetIssuesMap, FastHasher, MAX_PRACTICAL_DIMENSION_SIZE, SimplexKeyBuffer, SimplexKeySet, - SmallBuffer, fast_hash_map_with_capacity, fast_hash_set_with_capacity, + SmallBuffer, VertexKeySet, fast_hash_map_with_capacity, fast_hash_set_with_capacity, }; use crate::core::facet::FacetHandle; -use crate::core::simplex::Simplex; use crate::core::tds::{InvariantError, SimplexKey, TdsError, VertexKey}; use crate::core::traits::data_type::DataType; use crate::core::triangulation::Triangulation; @@ -287,23 +287,11 @@ where /// triangulation invariants. The error preserves structured information from whichever /// layer (TDS or Topology) detected the failure. pub(crate) fn remove_vertex(&mut self, vertex_key: VertexKey) -> Result { - // Verify the vertex exists if self.tds.vertex(vertex_key).is_none() { return Ok(0); // Vertex not found, nothing to remove } - // Collect all simplices containing this vertex by scanning all simplices - let simplices_to_remove: SimplexKeyBuffer = self - .tds - .simplices() - .filter_map(|(simplex_key, simplex)| { - if simplex.vertices().contains(&vertex_key) { - Some(simplex_key) - } else { - None - } - }) - .collect(); + let simplices_to_remove = self.tds.find_simplices_containing_vertex(vertex_key); if simplices_to_remove.is_empty() { // Vertex exists but has no incident simplices; remove it only if the @@ -311,7 +299,6 @@ where return self.remove_vertex_with_invariant_checks(vertex_key); } - // Extract cavity boundary BEFORE removing simplices let boundary_facets = extract_cavity_boundary(&self.tds, &simplices_to_remove).map_err(|e| { TdsError::InconsistentDataStructure { @@ -319,16 +306,35 @@ where } })?; - // If boundary is empty, we're removing the entire triangulation if boundary_facets.is_empty() { // Use TDS removal for the empty-boundary case, then validate so // lower-dimensional remnants are rejected and rolled back. return self.remove_vertex_with_invariant_checks(vertex_key); } - // Pick apex vertex for fan triangulation (first vertex of first boundary facet) + let affected_vertices = + self.affected_vertices_for_vertex_removal(&simplices_to_remove, vertex_key)?; + let apex_vertex_key = self.pick_fan_apex(&boundary_facets)?; + self.remove_vertex_with_fan_retriangulation( + vertex_key, + &simplices_to_remove, + &boundary_facets, + &affected_vertices, + apex_vertex_key, + ) + } + + /// Removes a vertex through fan cavity filling and rolls back on postcondition failure. + fn remove_vertex_with_fan_retriangulation( + &mut self, + vertex_key: VertexKey, + simplices_to_remove: &SimplexKeyBuffer, + boundary_facets: &[FacetHandle], + affected_vertices: &VertexKeySet, + apex_vertex_key: VertexKey, + ) -> Result { // Snapshot before destructive retriangulation edits so we can roll back if any // subsequent orientation/finalization step fails. let tds_snapshot = self.tds.clone_for_rollback(); @@ -336,11 +342,19 @@ where // Fill cavity with fan triangulation BEFORE removing old simplices // Use fan triangulation that skips boundary facets which already include the apex let new_simplices = self - .fan_fill_cavity(apex_vertex_key, &boundary_facets) + .fan_fill_cavity(apex_vertex_key, boundary_facets) .map_err(|e| insertion_error_to_invariant_error(e, "Fan triangulation failed"))?; + self.canonicalize_positive_orientation_for_simplices(&new_simplices) + .map_err(|e| { + insertion_error_to_invariant_error( + e, + "Orientation canonicalization failed after fan filling", + ) + })?; + // Wire neighbors for the new simplices (while both old and new simplices exist) let external_facets = - external_facets_for_boundary(&self.tds, &simplices_to_remove, &boundary_facets) + external_facets_for_boundary(&self.tds, simplices_to_remove, boundary_facets) .map_err(|e| { insertion_error_to_invariant_error(e, "External-facet collection failed") })?; @@ -348,63 +362,40 @@ where &mut self.tds, &new_simplices, external_facets.iter().copied(), - Some(&simplices_to_remove), + Some(simplices_to_remove), ) .map_err(|e| insertion_error_to_invariant_error(e, "Neighbor wiring failed"))?; // Remove the simplices containing the vertex (now that new simplices are wired up) // Note: remove_simplices_by_keys() automatically clears neighbor pointers in surviving // simplices that reference removed simplices (sets them to None/boundary) - let mut simplices_removed = self.tds.remove_simplices_by_keys(&simplices_to_remove); - - // Validate facet topology for newly created simplices (O(k*D) localized check) - if let Some(issues) = self.detect_local_facet_issues(&new_simplices)? { - #[cfg(debug_assertions)] - tracing::warn!( - "Warning: {} over-shared facets detected after vertex removal, repairing...", - issues.len() - ); - let repair_outcome = self - .repair_local_facet_issues_with_frontier(&issues) - .map_err(InvariantError::Tds)?; - let removed = repair_outcome.removed_count; - simplices_removed += removed; - #[cfg(debug_assertions)] - tracing::debug!("Repaired by removing {removed} additional simplices"); - - // Repair neighbor pointers after removing additional simplices - // This ensures neighbor consistency after repair operations - if removed > 0 { - repair_neighbor_pointers(&mut self.tds).map_err(|e| { - insertion_error_to_invariant_error( - e, - "Neighbor repair after facet issue repair failed", - ) - })?; - } - } - // Normalize coherent orientation, canonicalize global sign, and promote - // simplices to positive orientation (#258). - self.normalize_and_promote_positive_orientation() - .map_err(|e| { - insertion_error_to_invariant_error( - e, - "Orientation canonicalization failed after fan retriangulation", - ) - })?; + let mut simplices_removed = self.tds.remove_simplices_by_keys(simplices_to_remove); + let mut post_repair_frontier = SimplexKeyBuffer::new(); - // Rebuild vertex-simplex incidence for all vertices - self.tds - .assign_incident_simplices() - .map_err(|e| InvariantError::Tds(e.into_inner()))?; + self.repair_vertex_removal_facet_issues( + &new_simplices, + &mut simplices_removed, + &mut post_repair_frontier, + )?; - // Remove the vertex using Tds method (handles internal bookkeeping) self.tds .remove_vertex(vertex_key) .map_err(|e| InvariantError::Tds(e.into_inner()))?; - self.tds.is_valid().map_err(InvariantError::Tds)?; - self.is_valid()?; + let surviving_new_simplices = self.live_simplices_from(&new_simplices); + let validation_scope = self.vertex_removal_validation_scope( + &new_simplices, + &external_facets, + &post_repair_frontier, + ); + self.normalize_vertex_removal_orientation(&validation_scope)?; + self.repair_affected_vertex_incidence_from_scope(affected_vertices, &validation_scope)?; + self.validate_vertex_removal_postconditions( + vertex_key, + affected_vertices, + &surviving_new_simplices, + &validation_scope, + )?; Ok(simplices_removed) })(); @@ -418,6 +409,129 @@ where } } + /// Repairs over-shared facets introduced by vertex-removal fan filling. + fn repair_vertex_removal_facet_issues( + &mut self, + new_simplices: &SimplexKeyBuffer, + simplices_removed: &mut usize, + post_repair_frontier: &mut SimplexKeyBuffer, + ) -> Result<(), InvariantError> { + let Some(issues) = self.detect_local_facet_issues(new_simplices)? else { + return Ok(()); + }; + + #[cfg(debug_assertions)] + tracing::warn!( + "Warning: {} over-shared facets detected after vertex removal, repairing...", + issues.len() + ); + + let repair_outcome = self + .repair_local_facet_issues_with_frontier(&issues) + .map_err(InvariantError::Tds)?; + let removed = repair_outcome.removed_count; + post_repair_frontier.extend(repair_outcome.frontier_simplices.iter().copied()); + *simplices_removed += removed; + + #[cfg(debug_assertions)] + tracing::debug!("Repaired by removing {removed} additional simplices"); + + if removed > 0 { + repair_neighbor_pointers(&mut self.tds).map_err(|e| { + insertion_error_to_invariant_error( + e, + "Neighbor repair after facet issue repair failed", + ) + })?; + } + + Ok(()) + } + + /// Normalizes coherence globally but promotes only the vertex-removal scope geometrically. + fn normalize_vertex_removal_orientation( + &mut self, + validation_scope: &SimplexKeyBuffer, + ) -> Result<(), InvariantError> { + for _ in 0..3 { + self.canonicalize_positive_orientation_for_simplices(validation_scope) + .map_err(|e| { + insertion_error_to_invariant_error( + e, + "Local orientation promotion failed after vertex removal", + ) + })?; + self.tds + .normalize_coherent_orientation() + .map_err(InvariantError::Tds)?; + if self + .validate_geometric_simplex_orientation_for_simplices(validation_scope) + .is_ok() + { + return Ok(()); + } + } + + if self + .validate_geometric_simplex_orientation_for_simplices(validation_scope) + .is_ok() + { + return Ok(()); + } + + self.normalize_and_promote_positive_orientation() + .map_err(|e| { + insertion_error_to_invariant_error( + e, + "Global orientation fallback failed after local vertex-removal normalization", + ) + }) + } + + /// Repairs incident-simplex pointers for vertices touched by vertex removal. + fn repair_affected_vertex_incidence_from_scope( + &mut self, + affected_vertices: &VertexKeySet, + validation_scope: &SimplexKeyBuffer, + ) -> Result<(), InvariantError> { + for &vertex_key in affected_vertices { + let needs_repair = self.tds.vertex(vertex_key).is_some_and(|vertex| { + !vertex.incident_simplex().is_some_and(|simplex_key| { + self.tds + .simplex(simplex_key) + .is_some_and(|simplex| simplex.contains_vertex(vertex_key)) + }) + }); + if !needs_repair { + continue; + } + + let incident_simplex = validation_scope.iter().copied().find(|&simplex_key| { + self.tds + .simplex(simplex_key) + .is_some_and(|simplex| simplex.contains_vertex(vertex_key)) + }); + + let Some(simplex_key) = incident_simplex else { + let Some(vertex) = self.tds.vertex(vertex_key) else { + continue; + }; + return Err(InvariantError::Triangulation( + TriangulationValidationError::IsolatedVertex { + vertex_key, + vertex_uuid: vertex.uuid(), + }, + )); + }; + + if let Some(vertex) = self.tds.vertex_mut(vertex_key) { + vertex.set_incident_simplex(Some(simplex_key)); + } + } + + Ok(()) + } + /// Removes a vertex via direct TDS mutation and rolls back unless all triangulation /// invariants still hold. /// @@ -448,6 +562,148 @@ where } } + /// Collects boundary-star vertices whose incident simplices may change during removal. + fn affected_vertices_for_vertex_removal( + &self, + simplices_to_remove: &[SimplexKey], + removed_vertex: VertexKey, + ) -> Result { + let mut affected_vertices = + fast_hash_set_with_capacity(simplices_to_remove.len().saturating_mul(D)); + for &simplex_key in simplices_to_remove { + let simplex = self.tds.simplex(simplex_key).ok_or_else(|| { + InvariantError::Tds(TdsError::SimplexNotFound { + simplex_key, + context: "collecting affected vertices for vertex removal".to_string(), + }) + })?; + for &vertex_key in simplex.vertices() { + if vertex_key != removed_vertex { + affected_vertices.insert(vertex_key); + } + } + } + Ok(affected_vertices) + } + + /// Returns live simplex keys from a mutation-produced buffer without duplicates. + fn live_simplices_from(&self, simplices: &SimplexKeyBuffer) -> SimplexKeyBuffer { + let mut live_simplices = SimplexKeyBuffer::new(); + let mut seen = SimplexKeySet::default(); + seen.reserve(simplices.len()); + for &simplex_key in simplices { + self.push_live_simplex_once(&mut live_simplices, &mut seen, simplex_key); + } + live_simplices + } + + /// Builds the local simplex scope whose facets, ridges, neighbors, and orientation changed. + fn vertex_removal_validation_scope( + &self, + new_simplices: &SimplexKeyBuffer, + external_facets: &[FacetHandle], + post_repair_frontier: &SimplexKeyBuffer, + ) -> SimplexKeyBuffer { + let mut scope = SimplexKeyBuffer::new(); + let mut seen = SimplexKeySet::default(); + seen.reserve(new_simplices.len() + external_facets.len() + post_repair_frontier.len()); + + for &simplex_key in new_simplices { + self.push_live_simplex_once(&mut scope, &mut seen, simplex_key); + } + for facet in external_facets { + self.push_live_simplex_once(&mut scope, &mut seen, facet.simplex_key()); + } + for &simplex_key in post_repair_frontier { + self.push_live_simplex_once(&mut scope, &mut seen, simplex_key); + } + + scope + } + + /// Pushes a simplex key into a local validation scope only when it still exists. + fn push_live_simplex_once( + &self, + scope: &mut SimplexKeyBuffer, + seen: &mut SimplexKeySet, + simplex_key: SimplexKey, + ) { + if self.tds.contains_simplex(simplex_key) && seen.insert(simplex_key) { + scope.push(simplex_key); + } + } + + /// Validates the local postconditions that make successful vertex removal topology-preserving. + fn validate_vertex_removal_postconditions( + &self, + removed_vertex: VertexKey, + affected_vertices: &VertexKeySet, + surviving_new_simplices: &SimplexKeyBuffer, + validation_scope: &SimplexKeyBuffer, + ) -> Result<(), InvariantError> { + if self.tds.contains_vertex_key(removed_vertex) { + return Err(InvariantError::Tds(TdsError::InconsistentDataStructure { + message: format!( + "Removed vertex {removed_vertex:?} still exists after vertex-removal finalization" + ), + })); + } + + if self.tds.number_of_simplices() == 0 + || surviving_new_simplices.is_empty() + || validation_scope.is_empty() + { + self.tds.is_valid().map_err(InvariantError::Tds)?; + self.is_valid()?; + return Ok(()); + } + + self.validate_connectedness(surviving_new_simplices) + .map_err(|e| { + insertion_error_to_invariant_error( + e, + "Vertex-removal connectedness validation failed", + ) + })?; + self.validate_required_topology_links_for_simplices(validation_scope)?; + self.validate_affected_vertices_non_isolated(affected_vertices)?; + + #[cfg(debug_assertions)] + { + self.tds.is_valid().map_err(InvariantError::Tds)?; + self.is_valid()?; + } + + Ok(()) + } + + /// Ensures every vertex in the removed star still has a live incident simplex. + fn validate_affected_vertices_non_isolated( + &self, + affected_vertices: &VertexKeySet, + ) -> Result<(), InvariantError> { + for &vertex_key in affected_vertices { + let Some(vertex) = self.tds.vertex(vertex_key) else { + continue; + }; + if vertex.incident_simplex().is_some_and(|simplex_key| { + self.tds + .simplex(simplex_key) + .is_some_and(|simplex| simplex.contains_vertex(vertex_key)) + }) { + continue; + } + + return Err(InvariantError::Triangulation( + TriangulationValidationError::IsolatedVertex { + vertex_key, + vertex_uuid: vertex.uuid(), + }, + )); + } + Ok(()) + } + /// Pick an apex vertex for fan triangulation. /// /// Selects the first vertex from the first boundary facet as the apex. @@ -518,7 +774,18 @@ where apex_vertex_key: VertexKey, boundary_facets: &[FacetHandle], ) -> Result { - let mut new_simplices = SimplexKeyBuffer::new(); + let fan_boundary_facets = + self.fan_boundary_facets_excluding_apex(apex_vertex_key, boundary_facets)?; + fill_cavity_replacing_simplices(&mut self.tds, apex_vertex_key, &fan_boundary_facets) + } + + /// Filters out boundary facets that already contain the fan apex. + fn fan_boundary_facets_excluding_apex( + &self, + apex_vertex_key: VertexKey, + boundary_facets: &[FacetHandle], + ) -> Result, InsertionError> { + let mut fan_boundary_facets = SmallBuffer::new(); for facet_handle in boundary_facets { let boundary_simplex = @@ -538,39 +805,21 @@ where .into()); } - // Gather facet vertices (all except the opposite vertex) - let mut facet_vertices = SmallBuffer::::new(); - for (i, &vkey) in boundary_simplex.vertices().iter().enumerate() { - if i != facet_idx { - facet_vertices.push(vkey); - } - } - - // Skip facets that already contain the apex to avoid duplicate vertices - if facet_vertices.contains(&apex_vertex_key) { - continue; + let facet_contains_apex = boundary_simplex + .vertices() + .iter() + .enumerate() + .any(|(i, &vkey)| i != facet_idx && vkey == apex_vertex_key); + if !facet_contains_apex { + fan_boundary_facets.push(*facet_handle); } - - // Build new simplex vertices = facet_vertices + apex - let mut new_simplex_vertices = facet_vertices; - new_simplex_vertices.push(apex_vertex_key); - - // Create and insert the new simplex - let new_simplex = - Simplex::new(new_simplex_vertices, None).map_err(CavityFillingError::from)?; - let simplex_key = self - .tds - .insert_simplex_with_mapping_prechecked_topology(new_simplex) - .map_err(InsertionError::from)?; - - new_simplices.push(simplex_key); } - if new_simplices.is_empty() { + if fan_boundary_facets.is_empty() { return Err(CavityFillingError::EmptyFanTriangulation.into()); } - Ok(new_simplices) + Ok(fan_boundary_facets) } /// Detects over-shared facets @@ -936,7 +1185,7 @@ mod tests { use super::*; use crate::DelaunayTriangulation; use crate::core::collections::{CavityBoundaryBuffer, NeighborBuffer}; - use crate::core::simplex::NeighborSlot; + use crate::core::simplex::{NeighborSlot, Simplex}; use crate::core::tds::Tds; use crate::core::vertex::Vertex; use crate::geometry::kernel::FastKernel; From 3ca92fb5555975a755e0e5e4129f072f66f1ee95 Mon Sep 17 00:00:00 2001 From: Adam Getchell Date: Tue, 19 May 2026 13:34:14 -0700 Subject: [PATCH 2/5] perf(benchmarks): stress remove_vertex cases - Add adversarial remove_vertex benchmark fixtures for near-boundary, cospherical, near-degenerate, and large-coordinate point sets. - Include the selected fixture kind in Criterion case names so benchmark output shows which geometry each run exercises. - Name the vertex-removal orientation normalization budget. --- benches/README.md | 2 +- benches/remove_vertex.rs | 221 +++++++++++++++++++++++++++++++++++++-- src/core/repair.rs | 4 +- 3 files changed, 215 insertions(+), 12 deletions(-) diff --git a/benches/README.md b/benches/README.md index b1a2e190..5b118a5a 100644 --- a/benches/README.md +++ b/benches/README.md @@ -12,7 +12,7 @@ predicates fast across 2D-5D. | `circumsphere_containment.rs` | Compare circumsphere predicate methods | 2D-5D fixed, 3D random, edge cases | ~5 min | Predicate tuning, summaries | | `cold_path_predicates.rs` | Track hot/cold predicate paths | 2D-5D hot queries, near-boundary cases | ~2-5 min | Predicate optimization work | | `profiling_suite.rs` | Large-scale construction, memory, query, validation profiling | 2D/3D 10k, 4D 3k, 5D 1k | ~2-3 hr | Manual/monthly | -| `remove_vertex.rs` | Track topology-preserving vertex removal and invalid-remnant rollback | Deterministic 2D-5D triangulations | ~1-5 min | Vertex-removal/local-validation work | +| `remove_vertex.rs` | Vertex removal and rollback cost | 2D-5D fixed cases | ~1-5 min | Vertex removal | | `tds_clone.rs` | `Tds::clone()` snapshot cost | Deterministic 2D-5D triangulations | ~1-3 min | Rollback design baselines | | `topology_guarantee_construction.rs` | Cost of topology guarantee modes | 2D-5D construction cases | ~5-15 min | Manual topology policy work | diff --git a/benches/remove_vertex.rs b/benches/remove_vertex.rs index 67fed5ee..6a186c73 100644 --- a/benches/remove_vertex.rs +++ b/benches/remove_vertex.rs @@ -31,6 +31,12 @@ use bench_utils::{bench_option, bench_result}; const INTERIOR_BOUNDS: (f64, f64) = (0.0, 1.0); const INTERIOR_RADIUS_MIN: f64 = 0.15; const INTERIOR_RADIUS_SPAN: f64 = 0.70; +const NEAR_BOUNDARY_EPSILON: f64 = 1.0e-9; +const NEAR_DEGENERATE_EPSILON: f64 = 1.0e-10; +const COSPHERICAL_CENTER: f64 = 0.5; +const COSPHERICAL_RADIUS: f64 = 0.25; +const LARGE_COORDINATE_SCALE: f64 = 1.0e6; +const LARGE_COORDINATE_JITTER: f64 = 1.0e3; const SEED_SALT: u64 = 0x9E37_79B9_7F4A_7C15; const SEED_SEARCH_ATTEMPTS: usize = 64; const SAMPLE_SIZE: usize = 10; @@ -42,10 +48,50 @@ type BenchTriangulation = DelaunayTriangulation { vertex_count: usize, simplex_count: usize, + fixture_kind: FixtureKind, triangulation: BenchTriangulation, vertex_key: VertexKey, } +#[derive(Clone, Copy)] +enum FixtureKind { + Interior, + NearBoundary, + Cospherical, + NearDegenerate, + LargeCoordinate, +} + +const FIXTURE_KINDS: [FixtureKind; 5] = [ + FixtureKind::Interior, + FixtureKind::NearBoundary, + FixtureKind::Cospherical, + FixtureKind::NearDegenerate, + FixtureKind::LargeCoordinate, +]; + +impl FixtureKind { + const fn label(self) -> &'static str { + match self { + Self::Interior => "interior", + Self::NearBoundary => "near_boundary", + Self::Cospherical => "cospherical", + Self::NearDegenerate => "near_degenerate", + Self::LargeCoordinate => "large_coordinate", + } + } + + const fn index(self) -> usize { + match self { + Self::Interior => 0, + Self::NearBoundary => 1, + Self::Cospherical => 2, + Self::NearDegenerate => 3, + Self::LargeCoordinate => 4, + } + } +} + /// Derive a deterministic, dimension-specific seed for one benchmark case. fn seed_for_case(requested_vertices: usize, seed_base: u64) -> u64 { let vertices = bench_result( @@ -56,17 +102,48 @@ fn seed_for_case(requested_vertices: usize, seed_base: u64) -> u seed_base ^ vertices.wrapping_mul(SEED_SALT) ^ dimension.rotate_left(32) } -/// Generate a reproducible canonical simplex with interior points. +/// Pick the preferred geometric fixture for one benchmark case. +fn preferred_fixture_kind(case_index: usize) -> FixtureKind { + FIXTURE_KINDS[(D + case_index) % FIXTURE_KINDS.len()] +} + +/// Cycle through all fixture kinds during seed search, starting with the preferred kind. +const fn fixture_kind_for_attempt(preferred_kind: FixtureKind, attempt: usize) -> FixtureKind { + FIXTURE_KINDS[(preferred_kind.index() + attempt) % FIXTURE_KINDS.len()] +} + +/// Convert a bounded benchmark index to `f64` without unchecked casts. +fn usize_to_f64(value: usize, context: &str) -> f64 { + f64::from(bench_result(u32::try_from(value), context)) +} + +/// Generate a reproducible canonical simplex with selected adversarial points. fn generate_vertices( requested_vertices: usize, seed: u64, + fixture_kind: FixtureKind, ) -> Vec> { - let interior_count = requested_vertices.saturating_sub(D + 1); + let generated_count = requested_vertices.saturating_sub(D + 1); let mut points = simplex_points::(); + let generated_points = match fixture_kind { + FixtureKind::Interior => generate_interior_points(generated_count, seed), + FixtureKind::NearBoundary => generate_near_boundary_points(generated_count, seed), + FixtureKind::Cospherical => generate_cospherical_points(generated_count, seed), + FixtureKind::NearDegenerate => generate_near_degenerate_simplex(generated_count, seed), + FixtureKind::LargeCoordinate => generate_large_coordinate_points(generated_count, seed), + }; + + points.extend(generated_points); + Vertex::from_points(&points) +} + +/// Generate well-conditioned interior points inside the canonical simplex. +fn generate_interior_points(count: usize, seed: u64) -> Vec> { let raw_points = bench_result( - generate_random_points_seeded::(interior_count, INTERIOR_BOUNDS, seed), + generate_random_points_seeded::(count, INTERIOR_BOUNDS, seed), format!("failed to generate {D}D interior benchmark points"), ); + let mut points = Vec::with_capacity(count); for (index, raw_point) in raw_points.iter().enumerate() { let radius = interior_radius(index); @@ -78,7 +155,98 @@ fn generate_vertices( points.push(Point::new(coords)); } - Vertex::from_points(&points) + points +} + +/// Generate points close to coordinate-boundary facets of the canonical simplex. +fn generate_near_boundary_points(count: usize, seed: u64) -> Vec> { + let raw_points = bench_result( + generate_random_points_seeded::(count, INTERIOR_BOUNDS, seed), + format!("failed to generate {D}D near-boundary benchmark points"), + ); + let mut points = Vec::with_capacity(count); + + for (index, raw_point) in raw_points.iter().enumerate() { + let mut coords = [0.0; D]; + let direction = normalized_positive_direction(raw_point); + let near_boundary_axis = index % D; + for (coord, direction_coord) in coords.iter_mut().zip(direction) { + *coord = 0.98 * direction_coord; + } + coords[near_boundary_axis] = + NEAR_BOUNDARY_EPSILON * usize_to_f64(index + 1, "near-boundary index too large"); + points.push(Point::new(coords)); + } + + points +} + +/// Generate points on a shared sphere to stress cospherical predicates. +fn generate_cospherical_points(count: usize, seed: u64) -> Vec> { + let raw_points = bench_result( + generate_random_points_seeded::(count, INTERIOR_BOUNDS, seed), + format!("failed to generate {D}D cospherical benchmark points"), + ); + let mut points = Vec::with_capacity(count); + + for raw_point in &raw_points { + let direction = centered_unit_direction(raw_point); + let mut coords = [0.0; D]; + for (coord, direction_coord) in coords.iter_mut().zip(direction) { + *coord = COSPHERICAL_RADIUS.mul_add(direction_coord, COSPHERICAL_CENTER); + } + points.push(Point::new(coords)); + } + + points +} + +/// Generate points close to a lower-dimensional diagonal simplex. +fn generate_near_degenerate_simplex(count: usize, seed: u64) -> Vec> { + let seed_offset = f64::from(bench_result( + u32::try_from(seed % 997), + "near-degenerate seed phase does not fit in u32", + )) * 1.0e-14; + let denominator = usize_to_f64(count + 1, "near-degenerate count too large"); + let mut points = Vec::with_capacity(count); + + for index in 0..count { + let index_factor = usize_to_f64(index + 1, "near-degenerate index too large"); + let diagonal = index_factor / denominator; + let mut coords = [0.0; D]; + for (axis, coord) in coords.iter_mut().enumerate() { + let axis_factor = usize_to_f64(axis + 1, "near-degenerate axis too large"); + *coord = (NEAR_DEGENERATE_EPSILON * axis_factor) + .mul_add(index_factor, diagonal + seed_offset); + } + points.push(Point::new(coords)); + } + + points +} + +/// Generate finite points with large coordinates to stress scale-sensitive paths. +fn generate_large_coordinate_points(count: usize, seed: u64) -> Vec> { + let raw_points = bench_result( + generate_random_points_seeded::(count, INTERIOR_BOUNDS, seed), + format!("failed to generate {D}D large-coordinate benchmark points"), + ); + let mut points = Vec::with_capacity(count); + + for (index, raw_point) in raw_points.iter().enumerate() { + let index_offset = usize_to_f64(index + 1, "large-coordinate index too large"); + let mut coords = [0.0; D]; + for (axis, (coord, raw_coord)) in coords.iter_mut().zip(raw_point.coords()).enumerate() { + let axis_factor = usize_to_f64(axis + 1, "large-coordinate axis too large"); + *coord = LARGE_COORDINATE_SCALE.mul_add( + axis_factor, + LARGE_COORDINATE_JITTER.mul_add(*raw_coord, index_offset), + ); + } + points.push(Point::new(coords)); + } + + points } /// Generate the minimal full-dimensional simplex points. @@ -126,6 +294,29 @@ fn normalized_positive_direction(point: &Point) -> [f64; weights } +/// Convert a random point in `[0, 1]^D` into a unit direction around the origin. +fn centered_unit_direction(point: &Point) -> [f64; D] { + let mut direction = [0.0; D]; + let mut norm_squared = 0.0; + + for (direction_coord, coordinate) in direction.iter_mut().zip(point.coords()) { + *direction_coord = coordinate - COSPHERICAL_CENTER; + norm_squared += *direction_coord * *direction_coord; + } + + if norm_squared <= f64::EPSILON { + direction[0] = 1.0; + return direction; + } + + let norm = norm_squared.sqrt(); + for direction_coord in &mut direction { + *direction_coord /= norm; + } + + direction +} + /// Find a vertex whose removal succeeds for the prepared triangulation. fn successful_removal_vertex( triangulation: &BenchTriangulation, @@ -144,12 +335,14 @@ fn successful_removal_vertex( fn build_success_source( requested_vertices: usize, seed_base: u64, + preferred_kind: FixtureKind, ) -> RemovalSource { for attempt in 0..SEED_SEARCH_ATTEMPTS { let attempt_seed = bench_result(u64::try_from(attempt), "seed attempt does not fit in u64"); let seed = seed_for_case::(requested_vertices, seed_base) ^ attempt_seed.wrapping_mul(SEED_SALT.rotate_left(17)); - let vertices = generate_vertices::(requested_vertices, seed); + let fixture_kind = fixture_kind_for_attempt(preferred_kind, attempt); + let vertices = generate_vertices::(requested_vertices, seed, fixture_kind); let Ok(triangulation) = DelaunayTriangulation::new(&vertices) else { continue; }; @@ -160,6 +353,7 @@ fn build_success_source( return RemovalSource { vertex_count: triangulation.number_of_vertices(), simplex_count: triangulation.number_of_simplices(), + fixture_kind, triangulation, vertex_key, }; @@ -169,7 +363,7 @@ fn build_success_source( None, format!( "no successful {D}D remove_vertex fixture found for {requested_vertices} vertices \ - after {SEED_SEARCH_ATTEMPTS} seeds" + after {SEED_SEARCH_ATTEMPTS} seeds across all fixture kinds" ), ) } @@ -189,6 +383,7 @@ fn build_rollback_source() -> RemovalSource { RemovalSource { vertex_count: triangulation.number_of_vertices(), simplex_count: triangulation.number_of_simplices(), + fixture_kind: FixtureKind::Interior, triangulation, vertex_key, } @@ -215,16 +410,22 @@ fn bench_success_dimension( group.warm_up_time(WARM_UP_TIME); group.measurement_time(MEASUREMENT_TIME); - for &requested_vertices in counts { - let source = build_success_source::(requested_vertices, seed_base); + for (case_index, &requested_vertices) in counts.iter().enumerate() { + let source = build_success_source::( + requested_vertices, + seed_base, + preferred_fixture_kind::(case_index), + ); group.throughput(Throughput::Elements(triangulation_element_count(&source))); group.bench_with_input( BenchmarkId::new( "remove_vertex", format!( - "vertices_{}_simplices_{}", - source.vertex_count, source.simplex_count + "{}_vertices_{}_simplices_{}", + source.fixture_kind.label(), + source.vertex_count, + source.simplex_count ), ), &source, diff --git a/src/core/repair.rs b/src/core/repair.rs index 08a76ed0..b2e15898 100644 --- a/src/core/repair.rs +++ b/src/core/repair.rs @@ -30,6 +30,8 @@ use uuid::Uuid; static FORCE_GLOBAL_NEIGHBOR_REBUILD_ENABLED: OnceLock = OnceLock::new(); +const ORIENTATION_NORMALIZATION_BUDGET: usize = 3; + /// Returns whether local neighbor repair should be bypassed for regression isolation. fn force_global_neighbor_rebuild_enabled() -> bool { *FORCE_GLOBAL_NEIGHBOR_REBUILD_ENABLED @@ -453,7 +455,7 @@ where &mut self, validation_scope: &SimplexKeyBuffer, ) -> Result<(), InvariantError> { - for _ in 0..3 { + for _ in 0..ORIENTATION_NORMALIZATION_BUDGET { self.canonicalize_positive_orientation_for_simplices(validation_scope) .map_err(|e| { insertion_error_to_invariant_error( From 02dad75e53fe635dd406803a50707349fee047ac Mon Sep 17 00:00:00 2001 From: Adam Getchell Date: Tue, 19 May 2026 13:52:27 -0700 Subject: [PATCH 3/5] test(core): cover vertex-removal helper invariants Add focused regression coverage for vertex-removal helper behavior: - affected-vertex collection and missing-simplex error preservation - local validation-scope deduplication and live-simplex filtering - incident-simplex repair, isolated-vertex reporting, and postcondition failure - fan-boundary filtering for facets that already contain the apex --- src/core/repair.rs | 145 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 145 insertions(+) diff --git a/src/core/repair.rs b/src/core/repair.rs index b2e15898..40e89ccb 100644 --- a/src/core/repair.rs +++ b/src/core/repair.rs @@ -1647,6 +1647,151 @@ mod tests { )); } + #[test] + fn test_vertex_removal_affected_vertices_excludes_removed_vertex() { + let (tri, [v0, v1, v2, v3], simplex_key) = build_single_tet(); + + let affected = tri + .affected_vertices_for_vertex_removal(&[simplex_key], v0) + .unwrap(); + + assert_eq!(affected.len(), 3); + assert!(!affected.contains(&v0)); + assert!(affected.contains(&v1)); + assert!(affected.contains(&v2)); + assert!(affected.contains(&v3)); + } + + #[test] + fn test_vertex_removal_affected_vertices_errors_on_missing_simplex() { + let (tri, [v0, ..], _) = build_single_tet(); + let missing_simplex = SimplexKey::from(KeyData::from_ffi(0xBAD)); + + let result = tri.affected_vertices_for_vertex_removal(&[missing_simplex], v0); + + assert!(matches!( + result, + Err(InvariantError::Tds(TdsError::SimplexNotFound { + simplex_key, + .. + })) if simplex_key == missing_simplex + )); + } + + #[test] + fn test_vertex_removal_scope_helpers_keep_live_simplices_once() { + let (tri, _, simplex_key) = build_single_tet(); + let missing_simplex = SimplexKey::from(KeyData::from_ffi(0xBAD)); + let mut candidate_simplices = SimplexKeyBuffer::new(); + candidate_simplices.push(simplex_key); + candidate_simplices.push(missing_simplex); + candidate_simplices.push(simplex_key); + let external_facets = [FacetHandle::new(simplex_key, 0)]; + + let live_simplices = tri.live_simplices_from(&candidate_simplices); + let validation_scope = tri.vertex_removal_validation_scope( + &candidate_simplices, + &external_facets, + &live_simplices, + ); + + assert_eq!(live_simplices.as_slice(), &[simplex_key]); + assert_eq!(validation_scope.as_slice(), &[simplex_key]); + } + + #[test] + fn test_repair_vertex_removal_facet_issues_noops_without_local_issues() { + let (mut tri, _, simplex_key) = build_single_tet(); + let mut new_simplices = SimplexKeyBuffer::new(); + new_simplices.push(simplex_key); + let mut simplices_removed = 0; + let mut post_repair_frontier = SimplexKeyBuffer::new(); + + tri.repair_vertex_removal_facet_issues( + &new_simplices, + &mut simplices_removed, + &mut post_repair_frontier, + ) + .unwrap(); + + assert_eq!(simplices_removed, 0); + assert!(post_repair_frontier.is_empty()); + } + + #[test] + fn test_repair_affected_vertex_incidence_from_scope_repairs_missing_pointer() { + let (mut tri, [_, _, _, v3], simplex_key) = build_single_tet(); + tri.tds.vertex_mut(v3).unwrap().set_incident_simplex(None); + let mut affected_vertices = VertexKeySet::default(); + affected_vertices.insert(v3); + let mut validation_scope = SimplexKeyBuffer::new(); + validation_scope.push(simplex_key); + + tri.repair_affected_vertex_incidence_from_scope(&affected_vertices, &validation_scope) + .unwrap(); + + assert_eq!( + tri.tds.vertex(v3).unwrap().incident_simplex(), + Some(simplex_key) + ); + } + + #[test] + fn test_repair_affected_vertex_incidence_from_scope_reports_isolated_vertex() { + let (mut tri, [_, _, _, v3], _) = build_single_tet(); + tri.tds.vertex_mut(v3).unwrap().set_incident_simplex(None); + let mut affected_vertices = VertexKeySet::default(); + affected_vertices.insert(v3); + let validation_scope = SimplexKeyBuffer::new(); + + let result = + tri.repair_affected_vertex_incidence_from_scope(&affected_vertices, &validation_scope); + + assert!(matches!( + result, + Err(InvariantError::Triangulation( + TriangulationValidationError::IsolatedVertex { vertex_key, .. } + )) if vertex_key == v3 + )); + } + + #[test] + fn test_vertex_removal_postconditions_reject_still_present_vertex() { + let (tri, [v0, ..], simplex_key) = build_single_tet(); + let affected_vertices = VertexKeySet::default(); + let mut surviving_new_simplices = SimplexKeyBuffer::new(); + surviving_new_simplices.push(simplex_key); + let mut validation_scope = SimplexKeyBuffer::new(); + validation_scope.push(simplex_key); + + let result = tri.validate_vertex_removal_postconditions( + v0, + &affected_vertices, + &surviving_new_simplices, + &validation_scope, + ); + + assert!(matches!( + result, + Err(InvariantError::Tds( + TdsError::InconsistentDataStructure { .. } + )) + )); + } + + #[test] + fn test_fan_boundary_facets_excluding_apex_keeps_only_facets_without_apex() { + let (tri, vkeys, simplex_key) = build_single_tet(); + let boundary_facets: CavityBoundaryBuffer = + (0..=3).map(|i| FacetHandle::new(simplex_key, i)).collect(); + + let fan_facets = tri + .fan_boundary_facets_excluding_apex(vkeys[0], &boundary_facets) + .unwrap(); + + assert_eq!(fan_facets.as_slice(), &[FacetHandle::new(simplex_key, 0)]); + } + #[test] fn test_quality_error_to_tds_error_preserves_lookup_variants() { let simplex_key = SimplexKey::from(KeyData::from_ffi(1)); From 53a73a6b5325ed074a85292011d8214c772d3223 Mon Sep 17 00:00:00 2001 From: Adam Getchell Date: Tue, 19 May 2026 15:48:07 -0700 Subject: [PATCH 4/5] fix(core): bound local facet repair removals - Preserve local repair removal budgets before mutating the TDS. - Reuse scoped neighbor repair after vertex-removal facet cleanup. - Install and verify cargo-nextest for faster CI test recipes. - Build examples once before running their compiled binaries. --- .github/workflows/ci.yml | 11 +++++ .github/workflows/codecov.yml | 5 +- docs/dev/tooling-alignment.md | 6 ++- justfile | 86 ++++++++++++++++++++++------------- scripts/run_all_examples.sh | 19 ++++++-- src/core/insertion.rs | 14 +++++- src/core/repair.rs | 80 ++++++++++++++++++++++++-------- 7 files changed, 162 insertions(+), 59 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c0327699..84976461 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -24,6 +24,7 @@ env: ACTIONLINT_VERSION: "1.7.12" DPRINT_VERSION: "0.54.0" JUST_VERSION: "1.51.0" + NEXTEST_VERSION: "0.9.136" RUMDL_VERSION: "0.1.94" SHFMT_VERSION: "3.13.1" SHFMT_SHA256_DARWIN_AMD64: "6feedafc72915794163114f512348e2437d080d0047ef8b8fa2ec63b575f12af" @@ -68,6 +69,16 @@ jobs: with: tool: just@${{ env.JUST_VERSION }} + - name: Install cargo-nextest + if: matrix.os != 'windows-latest' + uses: taiki-e/install-action@b550161ef8a7bc4f2a671c0b03a18ac9ccedea1e # v2.79.1 + with: + tool: nextest@${{ env.NEXTEST_VERSION }} + + - name: Verify cargo-nextest + if: matrix.os != 'windows-latest' + run: cargo nextest --version + - name: Install uv (for Python scripts and pytest) if: matrix.os != 'windows-latest' uses: astral-sh/setup-uv@08807647e7069bb48b6ef5acd8ec9567f424441b # v8.1.0 diff --git a/.github/workflows/codecov.yml b/.github/workflows/codecov.yml index 60aa1b11..e5e94e80 100644 --- a/.github/workflows/codecov.yml +++ b/.github/workflows/codecov.yml @@ -65,11 +65,14 @@ jobs: with: tool: just@${{ env.JUST_VERSION }} - - name: Install nextest + - name: Install cargo-nextest uses: taiki-e/install-action@b550161ef8a7bc4f2a671c0b03a18ac9ccedea1e # v2.79.1 with: tool: nextest@${{ env.NEXTEST_VERSION }} + - name: Verify cargo-nextest + run: cargo nextest --version + - name: Run tests with nextest (for JUnit XML) run: | # Note: We run tests twice in this workflow (nextest + cargo-llvm-cov). diff --git a/docs/dev/tooling-alignment.md b/docs/dev/tooling-alignment.md index 5c4ecc9b..342e668a 100644 --- a/docs/dev/tooling-alignment.md +++ b/docs/dev/tooling-alignment.md @@ -92,7 +92,11 @@ The useful updates ported in this pass are: as `scripts/tests/**`. - CI and local setup pins should track the same supported tool versions when practical. The current workflow pins align coverage and test tooling on - `cargo-llvm-cov` 0.8.7 and `cargo-nextest` 0.9.136, while all uv-backed + `cargo-llvm-cov` 0.8.7 and `cargo-nextest` 0.9.136. Both CI and Codecov + install the same `cargo-nextest` pin with the repository's pinned binary-tool + installer and verify `cargo nextest --version` before nextest-backed recipes + run. Local `just setup-tools` uses `cargo install --locked cargo-nextest` and + verifies the command is available for developer machines. All uv-backed workflows use uv 0.11.15 to match the local Python tooling bootstrap. ## Intentional Differences diff --git a/justfile b/justfile index 54ea39c8..537a6549 100644 --- a/justfile +++ b/justfile @@ -7,6 +7,7 @@ set shell := ["bash", "-euo", "pipefail", "-c"] cargo_llvm_cov_version := "0.8.7" +nextest_version := "0.9.136" # Common cargo-llvm-cov arguments for all coverage runs. # Excludes benches/examples from reports while allowing integration tests to @@ -30,6 +31,15 @@ _ensure-cargo-llvm-cov: exit 1 fi +_ensure-nextest: + #!/usr/bin/env bash + set -euo pipefail + if ! command -v cargo-nextest >/dev/null; then + echo "❌ 'cargo-nextest' not found. See 'just setup-tools' or install:" + echo " cargo install --locked cargo-nextest --version {{ nextest_version }}" + exit 1 + fi + _ensure-cargo-machete: #!/usr/bin/env bash set -euo pipefail @@ -136,9 +146,9 @@ bench-smoke: # Compile benchmarks and integration tests without running. This catches # release-profile-only warnings (e.g. cfg-gated unused-mut) that debug-mode # clippy/test won't see. -bench-test-compile: +bench-test-compile: _ensure-nextest cargo bench --workspace --no-run - cargo test --tests --release --no-run + cargo nextest run --release --tests --no-run # Build commands build: @@ -180,7 +190,7 @@ check: lint check-fast: cargo check -# CI simulation: comprehensive validation (matches .github/workflows/ci.yml) +# CI simulation: comprehensive validation. # Runs: checks + test workflow + examples ci: check test examples @echo "🎯 CI checks complete!" @@ -209,8 +219,10 @@ clean: clippy: cargo clippy --workspace --all-targets -- -D warnings -W clippy::pedantic -W clippy::nursery -W clippy::cargo - # All features - cargo clippy --workspace --all-targets --all-features -- -D warnings -W clippy::pedantic -W clippy::nursery -W clippy::cargo + # All features, split by target class so feature-gated benchmark/example + # code stays covered without rebuilding every target in one oversized graph. + cargo clippy --workspace --lib --tests --all-features -- -D warnings -W clippy::pedantic -W clippy::nursery -W clippy::cargo + cargo clippy --workspace --benches --examples --all-features -- -D warnings -W clippy::pedantic -W clippy::nursery -W clippy::cargo # Coverage analysis for local development (HTML output) coverage: _ensure-cargo-llvm-cov @@ -223,17 +235,17 @@ coverage-ci: _ensure-cargo-llvm-cov mkdir -p coverage cargo llvm-cov {{ _coverage_base_args }} --cobertura --output-path coverage/cobertura.xml -- --skip prop_ -debug-large-scale-2d n="36000" repair_every="1": - DELAUNAY_BULK_PROGRESS_EVERY=2000 DELAUNAY_LARGE_DEBUG_MAX_RUNTIME_SECS=1800 DELAUNAY_LARGE_DEBUG_N_2D={{ n }} DELAUNAY_LARGE_DEBUG_REPAIR_EVERY={{ repair_every }} cargo test --release --test large_scale_debug debug_large_scale_2d -- --ignored --exact --nocapture +debug-large-scale-2d n="36000" repair_every="1": _ensure-nextest + DELAUNAY_BULK_PROGRESS_EVERY=2000 DELAUNAY_LARGE_DEBUG_MAX_RUNTIME_SECS=1800 DELAUNAY_LARGE_DEBUG_N_2D={{ n }} DELAUNAY_LARGE_DEBUG_REPAIR_EVERY={{ repair_every }} cargo nextest run --release --profile ci --test large_scale_debug debug_large_scale_2d -- --ignored --exact --nocapture -debug-large-scale-3d n="7500" repair_every="1": - DELAUNAY_BULK_PROGRESS_EVERY=500 DELAUNAY_LARGE_DEBUG_MAX_RUNTIME_SECS=1800 DELAUNAY_LARGE_DEBUG_N_3D={{ n }} DELAUNAY_LARGE_DEBUG_REPAIR_EVERY={{ repair_every }} cargo test --release --test large_scale_debug debug_large_scale_3d -- --ignored --exact --nocapture +debug-large-scale-3d n="7500" repair_every="1": _ensure-nextest + DELAUNAY_BULK_PROGRESS_EVERY=500 DELAUNAY_LARGE_DEBUG_MAX_RUNTIME_SECS=1800 DELAUNAY_LARGE_DEBUG_N_3D={{ n }} DELAUNAY_LARGE_DEBUG_REPAIR_EVERY={{ repair_every }} cargo nextest run --release --profile ci --test large_scale_debug debug_large_scale_3d -- --ignored --exact --nocapture -debug-large-scale-4d n="900" repair_every="1": - DELAUNAY_BULK_PROGRESS_EVERY=100 DELAUNAY_LARGE_DEBUG_MAX_RUNTIME_SECS=1800 DELAUNAY_LARGE_DEBUG_N_4D={{ n }} DELAUNAY_LARGE_DEBUG_REPAIR_EVERY={{ repair_every }} cargo test --release --test large_scale_debug debug_large_scale_4d -- --ignored --exact --nocapture +debug-large-scale-4d n="900" repair_every="1": _ensure-nextest + DELAUNAY_BULK_PROGRESS_EVERY=100 DELAUNAY_LARGE_DEBUG_MAX_RUNTIME_SECS=1800 DELAUNAY_LARGE_DEBUG_N_4D={{ n }} DELAUNAY_LARGE_DEBUG_REPAIR_EVERY={{ repair_every }} cargo nextest run --release --profile ci --test large_scale_debug debug_large_scale_4d -- --ignored --exact --nocapture -debug-large-scale-5d n="140" repair_every="1": - DELAUNAY_BULK_PROGRESS_EVERY=20 DELAUNAY_LARGE_DEBUG_MAX_RUNTIME_SECS=1800 DELAUNAY_LARGE_DEBUG_N_5D={{ n }} DELAUNAY_LARGE_DEBUG_REPAIR_EVERY={{ repair_every }} cargo test --release --test large_scale_debug debug_large_scale_5d -- --ignored --exact --nocapture +debug-large-scale-5d n="140" repair_every="1": _ensure-nextest + DELAUNAY_BULK_PROGRESS_EVERY=20 DELAUNAY_LARGE_DEBUG_MAX_RUNTIME_SECS=1800 DELAUNAY_LARGE_DEBUG_N_5D={{ n }} DELAUNAY_LARGE_DEBUG_REPAIR_EVERY={{ repair_every }} cargo nextest run --release --profile ci --test large_scale_debug debug_large_scale_5d -- --ignored --exact --nocapture # Default recipe shows available commands default: @@ -414,7 +426,7 @@ perf-help: @echo " just profile 1.95 v0.7.5 # v0.7.5 code on Rust 1.95" # Quick pre-push 2D-5D large-scale wall-clock smoke guard. -perf-large-scale-smoke max_secs="60": +perf-large-scale-smoke max_secs="60": _ensure-nextest #!/usr/bin/env bash set -euo pipefail @@ -441,7 +453,7 @@ perf-large-scale-smoke max_secs="60": DELAUNAY_LARGE_DEBUG_MAX_RUNTIME_SECS="$max_secs" \ "$n_env=$n_points" \ DELAUNAY_LARGE_DEBUG_REPAIR_EVERY=1 \ - cargo test --release --test large_scale_debug "$test_name" -- --ignored --exact --nocapture; then + cargo nextest run --release --profile ci --test large_scale_debug "$test_name" -- --ignored --exact --nocapture; then echo "✅ ${dimension} completed within the ${max_secs}s test-runtime cap" else local code=$? @@ -770,6 +782,13 @@ setup-tools: echo " ✓ llvm-tools-preview" fi + if ! have cargo-nextest; then + echo " ⏳ Installing cargo-nextest {{ nextest_version }} (cargo)..." + cargo install --locked cargo-nextest --version {{ nextest_version }} + else + echo " ✓ cargo-nextest" + fi + if ! have cargo-llvm-cov; then echo " ⏳ Installing cargo-llvm-cov {{ cargo_llvm_cov_version }} (cargo)..." cargo install --locked cargo-llvm-cov --version {{ cargo_llvm_cov_version }} @@ -782,7 +801,7 @@ setup-tools: missing=0 cmds=(uv jq taplo dprint rumdl yamllint shfmt shellcheck actionlint git-cliff typos) - cmds+=(cargo-llvm-cov) + cmds+=(cargo-nextest cargo-llvm-cov) for cmd in "${cmds[@]}"; do if have "$cmd"; then @@ -900,17 +919,17 @@ test: bench-test-compile test-all test-all: test-unit test-integration test-python @echo "✅ All tests passed!" -test-allocation: - cargo test --test allocation_api --features count-allocations -- --nocapture +test-allocation: _ensure-nextest + cargo nextest run --profile ci --test allocation_api --features count-allocations -- --nocapture -test-diagnostics: - cargo test --test circumsphere_debug_tools --features diagnostics -- --nocapture +test-diagnostics: _ensure-nextest + cargo nextest run --profile ci --test circumsphere_debug_tools --features diagnostics -- --nocapture # test-integration: runs all integration tests (includes proptests) in release mode. # Release mode is required because exact-predicate arithmetic in debug mode makes # 3D+ proptests exceed CI timeout limits (>60s debug vs <1s release). -test-integration: - cargo test --tests --release --verbose +test-integration: _ensure-nextest + cargo nextest run --release --profile ci --tests # test-integration-fast: runs integration tests but skips proptests (tests prefixed with `prop_`) # @@ -918,26 +937,29 @@ test-integration: # To run the full (slow) property suite, use: just test-integration # # Note: `--skip prop_` is a substring filter applied by the Rust test harness. -test-integration-fast: - cargo test --tests --release --verbose -- --skip prop_ +test-integration-fast: _ensure-nextest + cargo nextest run --release --profile ci --tests -- --skip prop_ test-python: _ensure-uv uv run pytest -test-release: - cargo test --release +test-release: _ensure-nextest + cargo nextest run --release --profile ci + cargo test --doc --release # Run tests including slow/stress tests (100+ vertices, multiple dimensions) # These are gated behind the 'slow-tests' feature to keep CI fast -test-slow: - cargo test --features slow-tests +test-slow: _ensure-nextest + cargo nextest run --profile ci --features slow-tests + cargo test --doc --features slow-tests -test-slow-release: - cargo test --release --features slow-tests +test-slow-release: _ensure-nextest + cargo nextest run --release --profile ci --features slow-tests + cargo test --doc --release --features slow-tests # test-unit: runs lib and doc tests. -test-unit: - cargo test --lib --verbose +test-unit: _ensure-nextest + cargo nextest run --profile ci --lib cargo test --doc --verbose # Check TOML files parse cleanly. diff --git a/scripts/run_all_examples.sh b/scripts/run_all_examples.sh index 08634f83..02599275 100755 --- a/scripts/run_all_examples.sh +++ b/scripts/run_all_examples.sh @@ -114,7 +114,11 @@ if [ ${#all_examples[@]} -eq 0 ]; then error_exit "No examples found under ${PROJECT_ROOT}/examples" fi -# Run all examples +# Build all examples once, then run the compiled binaries. This keeps the same +# runtime validation while avoiding repeated Cargo planning and relinking. +echo "Building all examples..." +cargo build --release --examples + TIMEOUT_CMD="" if command -v timeout >/dev/null 2>&1; then TIMEOUT_CMD="timeout" @@ -122,16 +126,25 @@ elif command -v gtimeout >/dev/null 2>&1; then TIMEOUT_CMD="gtimeout" fi +EXE_SUFFIX="" +case "$(uname -s)" in +MINGW* | MSYS* | CYGWIN*) EXE_SUFFIX=".exe" ;; +esac + for example in "${all_examples[@]}"; do echo "=== Running $example ===" + example_binary="${PROJECT_ROOT}/target/release/examples/${example}${EXE_SUFFIX}" + if [[ ! -x "$example_binary" ]]; then + error_exit "Built example binary not found or not executable: $example_binary" + fi if [[ -n "$TIMEOUT_CMD" ]]; then DURATION="${EXAMPLE_TIMEOUT:-600s}" # If DURATION has no unit suffix, assume seconds case "$DURATION" in *[a-zA-Z]) ;; *) DURATION="${DURATION}s" ;; esac "$TIMEOUT_CMD" --preserve-status --signal=TERM --kill-after=10s "$DURATION" \ - cargo run --release --example "$example" || error_exit "Example $example failed!" + "$example_binary" || error_exit "Example $example failed!" else - cargo run --release --example "$example" || error_exit "Example $example failed!" + "$example_binary" || error_exit "Example $example failed!" fi done diff --git a/src/core/insertion.rs b/src/core/insertion.rs index c016ab6c..f650802c 100644 --- a/src/core/insertion.rs +++ b/src/core/insertion.rs @@ -1692,6 +1692,7 @@ where // Iteratively repair non-manifold topology until facet sharing is valid let mut total_removed = 0; + let max_repair_simplices_removed = conflict_simplices.len(); let mut facet_sharing_known_valid = true; let mut neighbor_repair_frontier = SimplexKeyBuffer::new(); #[cfg_attr( @@ -1722,7 +1723,10 @@ where issues.len() ); - let repair = self.repair_local_facet_issues_with_frontier(&issues)?; + let repair_budget_remaining = + max_repair_simplices_removed.saturating_sub(total_removed); + let repair = + self.repair_local_facet_issues_with_frontier(&issues, repair_budget_remaining)?; let removed = repair.removed_count; // Early exit if repair made no progress @@ -2503,6 +2507,7 @@ where // Iteratively repair non-manifold topology until facet sharing is valid let mut total_removed = 0; + let max_repair_simplices_removed = new_simplices.len(); let mut facet_sharing_known_valid = true; let mut neighbor_repair_frontier = SimplexKeyBuffer::new(); #[cfg_attr( @@ -2533,7 +2538,12 @@ where issues.len() ); - let repair = self.repair_local_facet_issues_with_frontier(&issues)?; + let repair_budget_remaining = + max_repair_simplices_removed.saturating_sub(total_removed); + let repair = self.repair_local_facet_issues_with_frontier( + &issues, + repair_budget_remaining, + )?; let removed = repair.removed_count; // Early exit if repair made no progress diff --git a/src/core/repair.rs b/src/core/repair.rs index 40e89ccb..7b694c1d 100644 --- a/src/core/repair.rs +++ b/src/core/repair.rs @@ -372,12 +372,14 @@ where // Note: remove_simplices_by_keys() automatically clears neighbor pointers in surviving // simplices that reference removed simplices (sets them to None/boundary) let mut simplices_removed = self.tds.remove_simplices_by_keys(simplices_to_remove); + let max_repair_simplices_removed = simplices_to_remove.len(); let mut post_repair_frontier = SimplexKeyBuffer::new(); self.repair_vertex_removal_facet_issues( &new_simplices, &mut simplices_removed, &mut post_repair_frontier, + max_repair_simplices_removed, )?; self.tds @@ -417,6 +419,7 @@ where new_simplices: &SimplexKeyBuffer, simplices_removed: &mut usize, post_repair_frontier: &mut SimplexKeyBuffer, + max_simplices_removed: usize, ) -> Result<(), InvariantError> { let Some(issues) = self.detect_local_facet_issues(new_simplices)? else { return Ok(()); @@ -429,8 +432,13 @@ where ); let repair_outcome = self - .repair_local_facet_issues_with_frontier(&issues) - .map_err(InvariantError::Tds)?; + .repair_local_facet_issues_with_frontier(&issues, max_simplices_removed) + .map_err(|e| { + insertion_error_to_invariant_error( + e, + "Local facet repair after vertex removal failed", + ) + })?; let removed = repair_outcome.removed_count; post_repair_frontier.extend(repair_outcome.frontier_simplices.iter().copied()); *simplices_removed += removed; @@ -439,12 +447,13 @@ where tracing::debug!("Repaired by removing {removed} additional simplices"); if removed > 0 { - repair_neighbor_pointers(&mut self.tds).map_err(|e| { - insertion_error_to_invariant_error( - e, - "Neighbor repair after facet issue repair failed", - ) - })?; + self.repair_neighbors_after_local_simplex_removal(new_simplices, post_repair_frontier) + .map_err(|e| { + insertion_error_to_invariant_error( + e, + "Neighbor repair after facet issue repair failed", + ) + })?; } Ok(()) @@ -1045,11 +1054,21 @@ where pub(crate) fn repair_local_facet_issues_with_frontier( &mut self, issues: &FacetIssuesMap, - ) -> Result + max_simplices_removed: usize, + ) -> Result where K::Scalar: Div, { - let to_remove = self.simplices_for_local_facet_issue_repair(issues)?; + let to_remove = self + .simplices_for_local_facet_issue_repair(issues) + .map_err(InsertionError::TopologyValidation)?; + let attempted = to_remove.len(); + if attempted > max_simplices_removed { + return Err(InsertionError::MaxSimplicesRemovedExceeded { + max_simplices_removed, + attempted, + }); + } let frontier_simplices = self.collect_local_repair_frontier(issues, &to_remove); let removed_count = self.tds.remove_simplices_by_keys(&to_remove); @@ -1147,15 +1166,8 @@ where { let tds_snapshot = self.tds.clone_for_rollback(); let repair_result = (|| -> Result { - let outcome = self - .repair_local_facet_issues_with_frontier(issues) - .map_err(InsertionError::TopologyValidation)?; - if outcome.removed_count > max_simplices_removed { - return Err(InsertionError::MaxSimplicesRemovedExceeded { - max_simplices_removed, - attempted: outcome.removed_count, - }); - } + let outcome = + self.repair_local_facet_issues_with_frontier(issues, max_simplices_removed)?; if outcome.removed_count == 0 { return Ok(0); } @@ -1711,6 +1723,7 @@ mod tests { &new_simplices, &mut simplices_removed, &mut post_repair_frontier, + usize::MAX, ) .unwrap(); @@ -1997,7 +2010,7 @@ mod tests { .expect("three simplices sharing one edge should be detected as over-shared"); let repair = tri - .repair_local_facet_issues_with_frontier(&issues) + .repair_local_facet_issues_with_frontier(&issues, usize::MAX) .unwrap(); assert_eq!(repair.removed_count, 1); assert!( @@ -2056,6 +2069,33 @@ mod tests { assert!(tri.detect_local_facet_issues(&survivors).unwrap().is_none()); } + #[test] + fn test_local_repair_budget_failure_does_not_remove_simplices() { + let (mut tri, original_simplices, _, _) = build_overshared_edge_fixture(); + let issues = tri + .detect_local_facet_issues(&original_simplices) + .unwrap() + .expect("three simplices sharing one edge should be detected as over-shared"); + let original_simplex_count = tri.tds.number_of_simplices(); + + let result = tri.repair_local_facet_issues_with_frontier(&issues, 0); + + assert!(matches!( + result, + Err(InsertionError::MaxSimplicesRemovedExceeded { + max_simplices_removed: 0, + attempted + }) if attempted > 0 + )); + assert_eq!(tri.tds.number_of_simplices(), original_simplex_count); + for simplex_key in original_simplices { + assert!( + tri.tds.contains_simplex(simplex_key), + "budget failure should happen before simplex removal" + ); + } + } + #[test] fn test_repair_local_facet_issues_rolls_back_invalid_public_repair() { let (mut tri, original_simplices, _, _) = build_overshared_edge_fixture(); From 50ed34718c50180c9d64d50c98bb8f841ce92b74 Mon Sep 17 00:00:00 2001 From: Adam Getchell Date: Tue, 19 May 2026 16:14:14 -0700 Subject: [PATCH 5/5] fix(core): bound vertex-removal facet repair convergence - Iterate vertex-removal facet repair until facet sharing is valid or the explicit simplex-removal budget is exhausted. - Preserve typed invariant failures when local repair cannot converge. - Size insertion repair budgets from the new simplex pool being repaired. --- src/core/insertion.rs | 2 +- src/core/repair.rs | 127 +++++++++++++++++++++++++++++++++--------- 2 files changed, 103 insertions(+), 26 deletions(-) diff --git a/src/core/insertion.rs b/src/core/insertion.rs index f650802c..1bf530a7 100644 --- a/src/core/insertion.rs +++ b/src/core/insertion.rs @@ -1692,7 +1692,7 @@ where // Iteratively repair non-manifold topology until facet sharing is valid let mut total_removed = 0; - let max_repair_simplices_removed = conflict_simplices.len(); + let max_repair_simplices_removed = new_simplices.len(); let mut facet_sharing_known_valid = true; let mut neighbor_repair_frontier = SimplexKeyBuffer::new(); #[cfg_attr( diff --git a/src/core/repair.rs b/src/core/repair.rs index 7b694c1d..d6b67021 100644 --- a/src/core/repair.rs +++ b/src/core/repair.rs @@ -4,7 +4,7 @@ //! repair, and vertex-removal cavity retriangulation for [`Triangulation`](crate::core::triangulation::Triangulation). use crate::core::algorithms::incremental_insertion::{ - CavityFillingError, InsertionError, external_facets_for_boundary, + CavityFillingError, CavityRepairStage, InsertionError, external_facets_for_boundary, fill_cavity_replacing_simplices, repair_neighbor_pointers, repair_neighbor_pointers_local, wire_cavity_neighbors, }; @@ -421,32 +421,51 @@ where post_repair_frontier: &mut SimplexKeyBuffer, max_simplices_removed: usize, ) -> Result<(), InvariantError> { - let Some(issues) = self.detect_local_facet_issues(new_simplices)? else { - return Ok(()); - }; + let mut remaining_budget = max_simplices_removed; - #[cfg(debug_assertions)] - tracing::warn!( - "Warning: {} over-shared facets detected after vertex removal, repairing...", - issues.len() - ); + loop { + if self.tds.validate_facet_sharing().is_ok() { + return Ok(()); + } - let repair_outcome = self - .repair_local_facet_issues_with_frontier(&issues, max_simplices_removed) - .map_err(|e| { - insertion_error_to_invariant_error( - e, - "Local facet repair after vertex removal failed", - ) - })?; - let removed = repair_outcome.removed_count; - post_repair_frontier.extend(repair_outcome.frontier_simplices.iter().copied()); - *simplices_removed += removed; + let Some(issues) = self.detect_local_facet_issues(new_simplices)? else { + return Ok(()); + }; - #[cfg(debug_assertions)] - tracing::debug!("Repaired by removing {removed} additional simplices"); + #[cfg(debug_assertions)] + tracing::warn!( + "Warning: {} over-shared facets detected after vertex removal, repairing...", + issues.len() + ); + + let repair_outcome = self + .repair_local_facet_issues_with_frontier(&issues, remaining_budget) + .map_err(|e| { + insertion_error_to_invariant_error( + e, + "Local facet repair after vertex removal failed", + ) + })?; + let removed = repair_outcome.removed_count; + if removed == 0 { + return Err(insertion_error_to_invariant_error( + CavityFillingError::InvalidFacetSharingAfterRepair { + stage: CavityRepairStage::FanTriangulation, + } + .into(), + "Local facet repair after vertex removal stalled", + )); + } + + remaining_budget = remaining_budget.saturating_sub(removed); + post_repair_frontier.extend(repair_outcome.frontier_simplices.iter().copied()); + *simplices_removed += removed; + + #[cfg(debug_assertions)] + tracing::debug!( + "Repaired by removing {removed} additional simplices ({remaining_budget} budget remaining)" + ); - if removed > 0 { self.repair_neighbors_after_local_simplex_removal(new_simplices, post_repair_frontier) .map_err(|e| { insertion_error_to_invariant_error( @@ -455,8 +474,6 @@ where ) })?; } - - Ok(()) } /// Normalizes coherence globally but promotes only the vertex-removal scope geometrically. @@ -2096,6 +2113,66 @@ mod tests { } } + #[test] + fn test_vertex_removal_facet_repair_reports_budget_exhaustion() { + let (mut tri, original_simplices, _, _) = build_overshared_edge_fixture(); + let original_simplex_count = tri.tds.number_of_simplices(); + let mut new_simplices = SimplexKeyBuffer::new(); + new_simplices.extend(original_simplices); + let mut simplices_removed = 0; + let mut post_repair_frontier = SimplexKeyBuffer::new(); + + let result = tri.repair_vertex_removal_facet_issues( + &new_simplices, + &mut simplices_removed, + &mut post_repair_frontier, + 0, + ); + + assert!(matches!( + result, + Err(InvariantError::Tds(TdsError::InconsistentDataStructure { ref message })) + if message.contains("Local facet repair after vertex removal failed") + && message.contains("Local facet repair removal budget exceeded") + )); + assert_eq!(simplices_removed, 0); + assert!(post_repair_frontier.is_empty()); + assert_eq!(tri.tds.number_of_simplices(), original_simplex_count); + for simplex_key in original_simplices { + assert!(tri.tds.contains_simplex(simplex_key)); + } + } + + #[test] + fn test_vertex_removal_facet_repair_restores_facet_sharing() { + let (mut tri, original_simplices, _, _) = build_overshared_edge_fixture(); + let mut new_simplices = SimplexKeyBuffer::new(); + new_simplices.extend(original_simplices); + let mut simplices_removed = 0; + let mut post_repair_frontier = SimplexKeyBuffer::new(); + + tri.repair_vertex_removal_facet_issues( + &new_simplices, + &mut simplices_removed, + &mut post_repair_frontier, + usize::MAX, + ) + .unwrap(); + + assert_eq!(simplices_removed, 1); + assert!( + !post_repair_frontier.is_empty(), + "removed-simplex neighbors should seed the post-repair frontier" + ); + assert!(tri.tds.validate_facet_sharing().is_ok()); + let survivors: Vec<_> = original_simplices + .into_iter() + .filter(|simplex_key| tri.tds.contains_simplex(*simplex_key)) + .collect(); + assert_eq!(survivors.len(), 2); + assert!(tri.detect_local_facet_issues(&survivors).unwrap().is_none()); + } + #[test] fn test_repair_local_facet_issues_rolls_back_invalid_public_repair() { let (mut tri, original_simplices, _, _) = build_overshared_edge_fixture();