Skip to content

Commit c23cefb

Browse files
authored
Refactor/validation hierarchy (#157)
* refactor(validation): reduce public validation API to canonical layer methods - Make focused invariant validators internal; keep only `is_valid`/`validate` as the public entry points (plus `DelaunayTriangulation::validation_report` for cumulative diagnostics). - Update docs/README and examples/benches to reflect the 4-level validation hierarchy and correct per-layer semantics. - Adjust tests/proptests/benches to validate at the appropriate layer (e.g., Level 1–2 during bootstrap, Level 1–3 for topology) and avoid deprecated helpers. - Fix commit-check/clippy issues (missing `# Errors` docs, similar-name locals, deprecated `is_delaunay`, private API usage in benches). * refactor: split validation errors by layer (TDS/topology/Delaunay) - Rename TDS validation error to `TdsValidationError` and update call sites across core/geometry - Introduce topology-layer `TriangulationValidationError` with structured manifold + Euler variants - Add `DelaunayTriangulationValidationError` and update `DelaunayTriangulation::{is_valid, validate}` signatures - Update insertion/boundary/facet plumbing to use the correct layer error types - Fix rustdoc links and cspell/markdownlint fallout from the refactor * refactor: layer construction errors (TDS/Triangulation/Delaunay) - Introduce layered construction errors: TdsConstructionError, TriangulationConstructionError, DelaunayTriangulationConstructionError - Update DelaunayTriangulation constructors and insertion plumbing to propagate layered errors consistently - Fix geometry helpers/tests to match wrapped construction error variants - Refresh docs to match current validation hierarchy + error layering; mark superseded topology integration notes as historical * Changed: Refactors validation error hierarchy internally Consolidates and standardizes the validation error types within the triangulation data structure (TDS) to improve internal error handling and consistency. Replaces `TriangulationValidationError` with `TdsValidationError` where appropriate, streamlining the error reporting across the library. Updates error propagation to maintain semantic accuracy. Refs: refactor/validation-hierarchy * refactor: clarify validation hierarchy and expand core test coverage - Make validation layering explicit (elements → TDS structure → topology) and expose a cumulative validation_report API - Preserve underlying layer errors in TriangulationValidationReport; update docs/examples accordingly - Add deterministic unit tests covering core error branches (Cell/TDS/util/incremental insertion), incl. overflow, missing keys, non-manifold cases, and serde failures - Clarify facet-cache error semantics and plumb source errors through convex-hull boundary facet extraction * Fixed: Improve TDS validation and Delaunay error handling Improves triangulation data structure (TDS) validation by adding a check for missing vertex keys in cells, preventing inconsistent state and improving error diagnostics. Enhances Delaunay validation to include cell keys in error messages, aiding debugging. These changes improve the overall robustness and diagnosability of the triangulation process. * Changed: Refactors triangulation validation for clarity Refactors triangulation validation in `DelaunayTriangulation` to improve clarity and correctness. It streamlines the validation process, reusing the lower-layer report more effectively and ensuring that Delaunay property violations are handled correctly. The `TdsValidationError` enum is also updated and a `TdsMutationError` alias is created to provide clearer semantics for mutation operations. A deprecated function `is_delaunay` is updated to reference `DelaunayTriangulation::is_valid()` and `DelaunayTriangulation::validate()` methods. Finally, the 3D insertion order robustness property test is specialized to avoid nearly-coplanar tetrahedra and co-spherical 5-tuples, focusing on pure insertion-order robustness. * Fixed: Correctly classify cavity boundary facets Classifies cavity boundary facets by facet incidence within the conflict region, ensuring accurate boundary detection during incremental updates. This avoids misclassification due to temporarily incomplete neighbor pointers, preventing internal boundary components and Euler/topology validation failures. Also adds debug-only safety net to retry cavity-based insertions with a conservative fallback (star-split) if Level 3 topology validation fails. This ensures robust handling of degenerate cases during incremental Delaunay triangulation construction. Refs: refactor/validation-hierarchy * Changed: Improves robustness of triangulation insertion Enhances triangulation insertion robustness by adding connectedness validation and preventing empty conflict regions. It also introduces star-split fallback for topology preservation and refactors boundary handling. These changes ensure a more reliable triangulation process. * Changed: Refactors TDS validation and mutation errors Refactors the `TdsValidationError` and `TdsMutationError` types to improve clarity and future extensibility. Introduces a wrapper struct for `TdsMutationError` to allow for independent evolution while maintaining API compatibility. Also improves the error handling during triangulation construction and modification. Refs: refactor/validation-hierarchy * perf: Adjust global validation checks Now that we have a validation hierarchy, we want to run expensive global checks, which are often O(N*D^2), only when we have a suspicion that something has gone wrong. Like CGAL, we want to rely on local checks and only call is_valid() when necessary. So, introduce a ValidationPolicy with SuspicionFlags. Then we only call expensive checks when insertions wander off the happy path, or if the caller decides they want different guarantees (e.g. Never, Always, DebugOnly). Also fix a bunch of other fixes related to code reviews. * Fixed: Addresses potential triangulation validation failures Fixes edge cases in triangulation validation and error handling, particularly around star-split fallback scenarios. Improves robustness by refining error messages and ensuring consistent behavior across debug and release builds. Also fixes a potential issue with duplicate points in triangulation. Refs: refactor/validation-hierarchy * Changed: Enhances topology validation and repair Refactors topology validation to include connectedness checks and provides more robust error reporting. Introduces automatic validation policies for incremental insertions, balancing performance and safety. Improves neighbor pointer repair logic and adds validation against neighbor cycles. These changes aim to improve the reliability of the triangulation process by ensuring a valid topology and reducing the likelihood of disconnected components and non-manifold facets. (Internal change: improves validation and repair logic) * Refactor: Improves triangulation validation and error reporting Refactors triangulation validation by restructuring the error hierarchy to avoid circular dependencies between TDS and Triangulation validation errors. This change also improves the documentation around the validation levels and their performance implications, providing better guidance for users. Internally, the `remove_cells_by_keys` method in `Tds` is enhanced to incrementally repair `incident_cell` pointers and neighbor references, optimizing vertex removal operations. This avoids global rebuilds. Refs: refactor/validation-hierarchy * Changed: Enhances triangulation validation and testing Updates the triangulation validation process to use the `validate()` method, providing more comprehensive checks (Levels 1-4). Also improves proptest to avoid pathological cases with duplicate coordinates or points on a coordinate hyperplane, leading to more robust and reliable tests. (Internal) Refs: refactor/validation-hierarchy * Removed: Deprecates global facet sharing fix Removes the deprecated `fix_invalid_facet_sharing` methods from both `DelaunayTriangulation` and `Triangulation`. These methods performed a global O(N·D) scan and have been superseded by the more efficient localized O(k·D) `detect_local_facet_issues` + `repair_local_facet_issues` flow. This change encourages the use of the localized repair approach, which is more performant for incremental updates to the triangulation. * Changed: Improves error reporting with cell UUIDs Enhances error reporting in Delaunay triangulation validation by including cell UUIDs in error messages. This allows for better debugging and log correlation when issues arise during validation. Introduces a helper function to avoid repetition. Refs: refactor/validation-hierarchy * Changed: Removes unused imports in tests (internal) Removes unused imports from several test modules to improve code cleanliness and reduce potential confusion. This change is internal and does not affect the functionality of the code. Refs: refactor/validation-hierarchy
1 parent 5ab8086 commit c23cefb

41 files changed

Lines changed: 6260 additions & 1888 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

Cargo.lock

Lines changed: 16 additions & 13 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ readme = "README.md"
1818
[dependencies]
1919
allocation-counter = { version = "0.8.1", optional = true } # for memory profiling
2020
anyhow = "1.0.100"
21-
arc-swap = "1.7.1"
21+
arc-swap = "1.8.0"
2222
approx = "0.5.1"
2323
clap = { version = "4.5.53", features = ["derive"] }
2424
derive_builder = "0.20.2"
@@ -29,15 +29,15 @@ num-traits = "0.2.19"
2929
ordered-float = { version = "5.1.0", features = ["serde"] }
3030
rand = "0.9.2"
3131
serde = { version = "1.0.228", features = ["derive"] }
32-
serde_json = "1.0.145"
32+
serde_json = "1.0.147"
3333
serde_test = "1.0.177"
3434
slotmap = { version = "1.1.1", features = ["serde"] }
3535
thiserror = "2.0.17"
3636
uuid = { version = "1.19.0", features = ["v4", "serde"] }
3737

3838
[dev-dependencies]
3939
criterion = { version = "0.8.1", features = ["html_reports"] }
40-
pastey = "0.2.0"
40+
pastey = "0.2.1"
4141
proptest = "1.9.0"
4242
sysinfo = "0.37.2" # Process memory monitoring for benchmarks
4343

README.md

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,13 +54,14 @@ For details, see: [Issue #120 Investigation](docs/issue_120_investigation.md)
5454
**Validation**: You can verify your triangulation meets your requirements using the library's
5555
[4-level validation hierarchy](docs/validation.md):
5656

57-
- **Level 2** (`dt.is_valid()`) - Structural correctness (expected to pass when using public APIs; not affected by Issue #120)
58-
- **Level 3** (`dt.triangulation().validate_manifold()`) - Manifold topology + Euler characteristic
59-
- **Level 4** (`dt.validate_delaunay()`) - Delaunay property (may fail in rare cases per Issue #120)
57+
- **Level 2** (`dt.tds().is_valid()`) - Structural correctness (expected to pass when using public APIs; not affected by Issue #120)
58+
- **Level 3** (`dt.triangulation().is_valid()`) - Manifold topology + Euler characteristic
59+
- **Level 4** (`dt.is_valid()`) - Delaunay property only (may fail in rare cases per Issue #120)
60+
- **All levels (1–4)** (`dt.validate()`) - Elements + structure + topology + Delaunay property
6061

6162
For applications requiring strict Delaunay guarantees:
6263

63-
- Use `validate_delaunay()` to check your specific triangulation
64+
- Use `dt.is_valid()` (Level 4 only) or `dt.validate()` (Levels 1–4) to check your specific triangulation
6465
- Use smaller point sets (violations are rarer)
6566
- Filter degenerate configurations when possible
6667
- Monitor for updates in future releases

WARP.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,10 @@ Essential guidance for AI assistants working in this repository.
77
### Git Operations
88

99
- **NEVER** run `git commit`, `git push`, `git tag`, or any git commands that modify version control state
10+
- **ALLOWED**: Run read-only git commands (e.g. `git --no-pager status`, `git --no-pager diff`,
11+
`git --no-pager log`, `git --no-pager show`, `git --no-pager blame`) to inspect changes/history
1012
- **ALWAYS** use `git --no-pager` when reading git output
11-
- Suggest git commands for the user but never execute them
13+
- Suggest git commands that modify version control state for the user to run manually
1214

1315
### Commit Messages
1416

benches/large_scale_performance.rs

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -246,26 +246,16 @@ fn bench_validation<const D: usize>(c: &mut Criterion, dimension_name: &str, n_p
246246
.expect("Failed to generate points");
247247
let vertices: Vec<_> = points.into_iter().map(|p| vertex!(p)).collect();
248248
let dt = DelaunayTriangulation::new(&vertices).expect("Failed to create triangulation");
249-
let tds = dt.tds();
249+
let tri = dt.triangulation();
250250

251251
// Throughput in terms of cells we actually validate
252-
group.throughput(Throughput::Elements(tds.number_of_cells() as u64));
253-
254-
// Collect cell keys once to avoid re-enumeration overhead in the hot loop
255-
let cell_keys: Vec<_> = tds.cell_keys().collect();
252+
group.throughput(Throughput::Elements(tri.number_of_cells() as u64));
256253

257254
group.bench_function("validate_topology", |b| {
258255
b.iter(|| {
259-
// Measure validation time for all cells
260-
let mut all_valid = true;
261-
for &cell_key in &cell_keys {
262-
let neighbors = tds.find_neighbors_by_key(cell_key);
263-
if let Err(e) = tds.validate_neighbor_topology(cell_key, &neighbors) {
264-
all_valid = false;
265-
black_box(e);
266-
}
267-
}
268-
black_box(all_valid)
256+
// Level 3 topology check (manifold-with-boundary + Euler characteristic)
257+
tri.is_valid()
258+
.expect("triangulation should be structurally valid during validation benchmark");
269259
});
270260
});
271261

benches/microbenchmarks.rs

Lines changed: 42 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,9 @@
44
//! in the delaunay triangulation library, particularly those that are performance-critical:
55
//!
66
//! 1. **`DelaunayTriangulation::with_kernel`**: Complete triangulation creation
7-
//! 2. **`is_valid`**: Complete triangulation validation performance
8-
//! 3. **Individual validation components**: Mapping validation, duplicate detection, etc.
9-
//! 4. **Incremental construction**: Performance of `insert()` method for vertex insertion
10-
//! 5. **Memory usage patterns**: Allocation and deallocation patterns
7+
//! 2. **Layered validation**: `dt.tds().is_valid()/validate()`, `dt.triangulation().is_valid()/validate()`, `dt.is_valid()`, `dt.validate()`
8+
//! 3. **Incremental construction**: Performance of `insert()` method for vertex insertion
9+
//! 4. **Memory usage patterns**: Allocation and deallocation patterns
1110
//!
1211
//! These benchmarks measure the effectiveness of the optimization implementations
1312
//! completed as part of the Pure Incremental Delaunay Triangulation refactoring project.
@@ -143,7 +142,26 @@ macro_rules! generate_validation_benchmarks {
143142
group.throughput(Throughput::Elements(throughput));
144143

145144
group.bench_with_input(
146-
BenchmarkId::new("is_valid", n_points),
145+
BenchmarkId::new("validate", n_points),
146+
&n_points,
147+
|b, &n_points| {
148+
b.iter_batched(
149+
|| {
150+
let points: Vec<Point<f64, $dim>> = generate_random_points_seeded(n_points, (-100.0, 100.0), seed).unwrap();
151+
let vertices: Vec<_> = points.iter().map(|p| vertex!(*p)).collect();
152+
DelaunayTriangulation::<RobustKernel<f64>, (), (), $dim>::with_kernel(RobustKernel::new(), &vertices).unwrap()
153+
},
154+
|dt| {
155+
dt.validate().unwrap();
156+
black_box(dt);
157+
},
158+
BatchSize::LargeInput,
159+
);
160+
},
161+
);
162+
163+
group.bench_with_input(
164+
BenchmarkId::new("is_valid_delaunay", n_points),
147165
&n_points,
148166
|b, &n_points| {
149167
b.iter_batched(
@@ -175,24 +193,37 @@ macro_rules! generate_validation_benchmarks {
175193

176194
let mut group = c.benchmark_group(&format!("validation_components_{}d", $dim));
177195

178-
group.bench_function("validate_vertex_mappings", |b| {
196+
group.bench_function("tds_is_valid", |b| {
197+
b.iter(|| {
198+
dt.tds().is_valid().unwrap();
199+
// Black box to prevent dead code elimination
200+
black_box(());
201+
});
202+
});
203+
204+
group.bench_function("tri_is_valid", |b| {
179205
b.iter(|| {
180-
dt.validate_vertex_mappings().unwrap();
206+
dt.triangulation().is_valid().unwrap();
181207
// Black box to prevent dead code elimination
182208
black_box(());
183209
});
184210
});
185211

186-
group.bench_function("validate_cell_mappings", |b| {
212+
group.bench_function("is_valid_delaunay", |b| {
187213
b.iter(|| {
188-
dt.validate_cell_mappings().unwrap();
214+
dt.is_valid().unwrap();
189215
// Black box to prevent dead code elimination
190216
black_box(());
191217
});
192218
});
193219

194-
// Note: validate_no_duplicate_cells and validate_facet_sharing are private methods
195-
// They are tested indirectly through the full is_valid() benchmark
220+
group.bench_function("validate", |b| {
221+
b.iter(|| {
222+
dt.validate().unwrap();
223+
// Black box to prevent dead code elimination
224+
black_box(());
225+
});
226+
});
196227

197228
group.finish();
198229
}

cspell.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@
9696
"RLENGTH",
9797
"RSTART",
9898
"RUSTDOCFLAGS",
99+
"RUSTFLAGS",
99100
"Roundtable",
100101
"Ryzen",
101102
"SBATCH",
@@ -362,6 +363,7 @@
362363
"startswith",
363364
"stdlib",
364365
"strsim",
366+
"stringifying",
365367
"subnormals",
366368
"supercell",
367369
"supercells",

docs/OPTIMIZATION_ROADMAP.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,8 @@ Eliminate UUID→Key lookups in hot paths by implementing direct key-based opera
8686

8787
```rust
8888
// New key-based helper method
89-
fn get_cell_vertex_keys(&self, cell_key: CellKey)
90-
-> Result<VertexKeyBuffer, TriangulationValidationError>
89+
fn get_cell_vertices(&self, cell_key: CellKey)
90+
-> Result<VertexKeyBuffer, TdsValidationError>
9191
```
9292

9393
#### Key-Based Methods Added
@@ -106,7 +106,7 @@ contains_vertex_key() / vertex_keys()
106106
find_neighbors_by_key() // #[must_use]
107107
set_neighbors_by_key() // No panics, proper error handling
108108
find_cells_containing_vertex_by_key() // #[must_use]
109-
get_cell_vertex_keys()
109+
get_cell_vertices()
110110
```
111111

112112
#### Zero-Allocation Iterator Optimization
@@ -120,7 +120,7 @@ vertex_uuid_iter() -> impl ExactSizeIterator<Item = Uuid>
120120

121121
#### Algorithms Optimized
122122

123-
- `assign_neighbors()` - Uses `get_cell_vertex_keys()`
123+
- `assign_neighbors()` - Uses `get_cell_vertices()`
124124
- `assign_incident_cells()` - Direct key operations
125125
- `remove_duplicate_cells()` - Key-based duplicate detection
126126
- `build_facet_to_cells_map_lenient()` - Optimized with keys (deprecated lenient version)

docs/numerical_robustness_guide.md

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ match insertion_result {
157157
// Retryable via perturbation
158158
}
159159
Err(InsertionError::ConflictRegion(e)) => {
160-
// Duplicate boundary facets or ridge fans - also retryable
160+
// Non-manifold facets or ridge fans - also retryable
161161
}
162162
Err(e) if e.is_retryable() => {
163163
// Automatic retry with perturbation
@@ -229,8 +229,8 @@ The `is_retryable()` method classifies errors:
229229
// Retryable errors (geometric degeneracies)
230230
- InsertionError::NonManifoldTopology { .. } // Facet sharing violation
231231
- InsertionError::Location(CycleDetected { .. }) // Point location cycle
232-
- InsertionError::ConflictRegion(DuplicateBoundaryFacets { .. })
233-
- InsertionError::ConflictRegion(RidgeFan { .. })
232+
- InsertionError::ConflictRegion(NonManifoldFacet { .. }) // Facet shared by >2 conflict cells
233+
- InsertionError::ConflictRegion(RidgeFan { .. }) // Ridge fan degeneracy
234234
- InsertionError::TopologyValidation(_) // Repair failure
235235

236236
// Non-retryable errors (structural failures)
@@ -511,15 +511,19 @@ let dt_robust: DelaunayTriangulation<RobustKernel<f64>, (), (), 3> =
511511
```rust
512512
use delaunay::prelude::*;
513513
use delaunay::geometry::kernel::{FastKernel, RobustKernel};
514-
use delaunay::core::triangulation_data_structure::TriangulationConstructionError;
515514
use delaunay::core::vertex::Vertex;
516515

517516
pub fn create_triangulation_with_fallback(
518-
vertices: &[Vertex<f64, (), 3>]
519-
) -> Result<DelaunayTriangulation<RobustKernel<f64>, (), (), 3>, TriangulationConstructionError> {
517+
vertices: &[Vertex<f64, (), 3>],
518+
) -> Result<
519+
DelaunayTriangulation<RobustKernel<f64>, (), (), 3>,
520+
DelaunayTriangulationConstructionError,
521+
> {
520522
// Strategy 1: Try fast kernel first for performance
521-
let fast_result: Result<DelaunayTriangulation<FastKernel<f64>, (), (), 3>, _> =
522-
DelaunayTriangulation::new(vertices);
523+
let fast_result: Result<
524+
DelaunayTriangulation<FastKernel<f64>, (), (), 3>,
525+
DelaunayTriangulationConstructionError,
526+
> = DelaunayTriangulation::new(vertices);
523527

524528
if fast_result.is_ok() {
525529
println!("Fast kernel succeeded");
@@ -547,14 +551,14 @@ pub fn create_triangulation_with_fallback(
547551

548552
```rust
549553
fn remove_duplicate_and_near_duplicate_points(
550-
vertices: &[Vertex<f64, Option<()>, 3>]
551-
) -> Vec<Vertex<f64, Option<()>, 3>> {
554+
vertices: &[Vertex<f64, (), 3>]
555+
) -> Vec<Vertex<f64, (), 3>> {
552556
// Simple deduplication based on coordinate proximity
553557
let mut filtered = Vec::new();
554558
let tolerance = 1e-10;
555559

556560
for vertex in vertices {
557-
let is_duplicate = filtered.iter().any(|existing: &Vertex<f64, Option<()>, 3>| {
561+
let is_duplicate = filtered.iter().any(|existing: &Vertex<f64, (), 3>| {
558562
let existing_coords: [f64; 3] = existing.point().into();
559563
let vertex_coords: [f64; 3] = vertex.point().into();
560564

@@ -681,8 +685,8 @@ The robust predicates system has comprehensive test coverage demonstrating real-
681685
- Demonstrates consistent results across dimensions (2D-5D)
682686
- Validates error recovery for "No cavity boundary facets found" scenarios
683687
- Confirms that structural invariants checked by `Tds::is_valid()` remain intact
684-
and can be further inspected via `Tds::validation_report()` and
685-
`Tds::validate_delaunay()` in targeted tests.
688+
and can be further inspected via `DelaunayTriangulation::validation_report()` and
689+
(when needed) `DelaunayTriangulation::is_valid()` / `DelaunayTriangulation::validate()` in targeted tests.
686690

687691
### Degenerate Case Tests
688692

docs/property_testing_summary.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ This document summarizes the property-based testing infrastructure added to the
77

88
For the full set of structural and geometric invariants referenced here, see the
99
"Triangulation Invariants" section in the crate-level docs (`src/lib.rs`) and
10-
`Tds::validation_report()` on `Tds`.
10+
`DelaunayTriangulation::validation_report()`.
1111

1212
## Implementation Date
1313

0 commit comments

Comments
 (0)