From ed4b7e6c0f7a5a17a07706467b683dc362a5f625 Mon Sep 17 00:00:00 2001 From: Adam Getchell Date: Tue, 5 May 2026 02:00:59 -0700 Subject: [PATCH 1/3] feat(cdt): implement explicit CDT strip construction - Replace the placeholder strip constructor with deterministic layered connectivity, direct time labels, overflow checks, and strict CDT validation. - Persist Up/Down cell classifications for explicit strip and toroidal constructors while preserving backend payloads on mutation failures. - Update foliation documentation for explicit open-boundary strips and toroidal meshes. - Tighten benchmark tooling diagnostics and Python exception boundaries so stdout remains data-only and broad support-script catches stay blocked. --- docs/dev/tooling-alignment.md | 5 +- docs/foliation.md | 44 +- docs/project.md | 3 +- scripts/benchmark_models.py | 6 +- scripts/benchmark_utils.py | 44 +- scripts/performance_analysis.py | 2 +- scripts/tests/test_benchmark_models.py | 4 +- scripts/tests/test_benchmark_utils.py | 16 +- semgrep.yaml | 12 +- src/cdt/ergodic_moves.rs | 9 +- src/cdt/triangulation.rs | 714 +++++++++++++++---------- tests/proptest_foliation.rs | 75 ++- 12 files changed, 530 insertions(+), 404 deletions(-) diff --git a/docs/dev/tooling-alignment.md b/docs/dev/tooling-alignment.md index 83497b6..dfe375d 100644 --- a/docs/dev/tooling-alignment.md +++ b/docs/dev/tooling-alignment.md @@ -21,9 +21,9 @@ CDT's Python tooling is broader than MCMC's. MCMC currently has changelog and ta - secure subprocess wrappers in `scripts/subprocess_utils.py`; - typed `subprocess.CompletedProcess[str]` helpers in tests; - Ruff, Ty, pytest, and uv-managed development dependencies in `pyproject.toml`; -- Python Semgrep rules and fixtures for broad exception catches, raw `Exception` in tests, ad hoc subprocess mocks, and missing return annotations. +- Python Semgrep rules and fixtures for broad exception catches across all `scripts/**/*.py`, raw `Exception` in tests, ad hoc subprocess mocks, and missing return annotations. -The broad-exception Semgrep rule is intentionally scoped to the smaller changelog, coverage, tag, subprocess, and model helper scripts in this pass. `scripts/benchmark_utils.py`, `scripts/hardware_utils.py`, and `scripts/performance_analysis.py` still contain broad recovery paths that need a separate cleanup before the rule can cover every script without creating noisy findings. +The broad-exception Semgrep rule now covers the full Python support-script tree. `scripts/benchmark_utils.py`, `scripts/hardware_utils.py`, and `scripts/performance_analysis.py` use typed recoverable exception boundaries, so new broad `except Exception` recovery paths are treated as tooling regressions rather than accepted legacy cleanup. ## Intentional Differences @@ -53,4 +53,3 @@ These were evaluated but not ported in this pass: - `codacy.yml`: defer until the repository has an intentional Codacy project token and a decision about whether Codacy should upload repository-owned OpenGrep/Semgrep findings in addition to `.github/workflows/semgrep-sarif.yml`. - `validate-examples`: defer until example outputs have stable, documented success markers. For now, `just examples` validates compilation and successful execution of all discovered examples. - broad no-`unwrap`/`panic` Semgrep rules: defer because current production and doctest code intentionally uses many invariant checks and examples. These need a separate cleanup plan before becoming blocking policy. -- broad Python exception coverage for all scripts: defer until the benchmark, hardware, and performance-analysis scripts have typed recoverable exception boundaries. diff --git a/docs/foliation.md b/docs/foliation.md index 284d4e5..7161d07 100644 --- a/docs/foliation.md +++ b/docs/foliation.md @@ -13,7 +13,7 @@ For 1+1 CDT: - **Edge classification**: spacelike (both endpoints at same t) or timelike (endpoints at t and t±1) - **Causality constraint**: no edge may span more than one time slice (|Δt| ≤ 1) -This implementation uses **cylinder topology** (S¹ × [0,T]) — spatial slices are open chains within the Delaunay triangulation, time runs from 0 to T−1. Toroidal topology (periodic time, full S¹ spatial slices) requires upstream support for periodic Delaunay triangulation (see issue #61). +This implementation supports explicit open-boundary strips and explicit toroidal meshes. `from_cdt_strip()` builds an open spatial interval over discrete time, while `from_toroidal_cdt()` builds S¹ × S¹ with periodic space and time. ## Architecture @@ -33,23 +33,17 @@ Vertex data is set at construction time via `VertexBuilder::data(t)`. For post-c ## Time Label Assignment -For `from_foliated_cylinder()`, time labels are assigned by **y-coordinate bucketing**: each vertex's y-coordinate is rounded to the nearest integer, giving the time slice index. The label is embedded directly as vertex data at construction. - -- Bucket for slice t: y ∈ [t − 0.5, t + 0.5) -- Conversion uses `y_to_time_bucket()` from `src/util.rs` via `ToPrimitive::to_u32` -- Values are clamped to [0, num_slices − 1] +For `from_cdt_strip()` and `from_toroidal_cdt()`, time labels are assigned directly while building vertices. Vertex `(i, t)` receives label `t`, so each slice starts with exactly `vertices_per_slice` vertices and every constructed triangle spans adjacent slices. `assign_foliation_by_y()` uses band-based bucketing and writes labels directly to vertex data via `set_vertex_data`. -## Grid Construction (`from_foliated_cylinder`) +## Grid Construction (`from_cdt_strip`) -The constructor places vertices on a grid with: +The open-boundary strip constructor places vertices on a grid with: -- **Spatial extent**: 1.0 (fixed, below the √3 ≈ 1.73 threshold that guarantees no Delaunay edge skips a time slice) -- **Temporal gap**: 1.0 (integer y-coordinates: y = 0, 1, 2, ...) -- **Perturbation**: small deterministic perturbation breaks co-circular degeneracy - - Interior vertices: hash-based random perturbation in x and y - - Boundary vertices (i=0, i=last): concave √(t+1) x-offset pushed outward, ensuring every row's extreme vertices are on the convex hull (no hull edge skips a time slice) +- **Spatial extent**: 1.0, with `vertices_per_slice` evenly spaced vertices per slice +- **Temporal gap**: 1.0, with integer y-coordinates `0, 1, 2, ...` +- **Connectivity**: each quad between adjacent slices is split into one Up `(2,1)` and one Down `(1,2)` triangle Parameters: `vertices_per_slice ≥ 4`, `num_slices ≥ 2`. @@ -71,7 +65,7 @@ Classification is done by `classify_edge(t0, t1)`, which reads time labels from Classification is done by `classify_cell(t0, t1, t2)`. Triangles that don’t span exactly one time slice (e.g., all vertices at the same time, or spanning >1 slice) return `None`. -Cell types are encoded as `i32` cell data (`Up = 1`, `Down = -1`) and can be bulk-written via `classify_all_cells()` using `set_cell_data`. +Cell types are encoded as `i32` cell data (`Up = 1`, `Down = -1`) and can be bulk-written via `classify_all_cells()` using `set_cell_data`. For foliated triangulations this bulk path is strict: every face must classify as `Up` or `Down`, otherwise `classify_all_cells()` and `validate_cell_classification()` return a validation error. ## Validation @@ -87,19 +81,27 @@ Structural checks: ### `validate_causality_delaunay()` -Edge-level check reading time labels directly from vertex data: +Face-level check reading time labels directly from vertex data: + +- Every triangle must contain exactly one spacelike edge and two timelike edges +- Returns `CdtError::CausalityViolation { time_0, time_1 }` if any triangle spans >1 slice +- Returns `CdtError::ValidationFailed { check: "causality", .. }` if a triangle is not a strict CDT cell + +### `validate_cell_classification()` + +Strict cell-classification check: -- Every edge must connect vertices within the same slice or adjacent slices -- Returns `CdtError::CausalityViolation { time_0, time_1 }` if any edge spans >1 slice +- Succeeds vacuously when no foliation is present +- Requires every foliated face to classify as `Up` or `Down` +- Returns `CdtError::ValidationFailed { check: "cell_classification", .. }` for same-slice or otherwise unclassifiable triangles ## Error Handling -- `CdtError::CausalityViolation { time_0, time_1 }` — structured error for edges violating causality -- `CdtError::DelaunayGenerationFailed` — from `from_foliated_cylinder()` when builder output is inconsistent (for example missing or out-of-range per-vertex time labels), with detailed construction context +- `CdtError::CausalityViolation { time_0, time_1 }` — structured error for time labels spanning more than one slice step +- `CdtError::DelaunayGenerationFailed` — from explicit CDT constructors when builder output is inconsistent, with detailed construction context - `CdtError::ValidationFailed { check, detail }` — for structural foliation issues and foliation-assignment failures (for example unreadable vertex coordinates) - `CdtError::InvalidGenerationParameters` — for invalid constructor parameters ## Future Work -- **Toroidal topology** (S¹ × S¹): requires periodic Delaunay construction (issue #61) -- **Foliation-aware ergodic moves**: moves that preserve or update the foliation during MCMC steps (#55) +- **Foliation-aware ergodic moves**: continue broadening topology-preservation tests for accepted move sequences diff --git a/docs/project.md b/docs/project.md index 96b46be..d683de3 100644 --- a/docs/project.md +++ b/docs/project.md @@ -52,7 +52,8 @@ Assigns each vertex to a discrete time slice, enabling classification of edges a ### `cdt/triangulation.rs` — Foliation integration -- `from_foliated_cylinder(vertices_per_slice, num_slices, seed)` _(crate-internal, provisional)_ — point-set strip constructor used for internal diagnostics while explicit strip construction lands +- `from_cdt_strip(vertices_per_slice, num_slices)` — explicit open-boundary 1+1 CDT strip with strict Up/Down cell classification +- `from_foliated_cylinder(vertices_per_slice, num_slices, seed)` _(crate-internal)_ — compatibility wrapper around `from_cdt_strip`; the seed is ignored because explicit construction is deterministic - `from_toroidal_cdt(vertices_per_slice, num_slices)` — explicit S¹×S¹ toroidal CDT (χ = 0); requires `vertices_per_slice ≥ 3` and `num_slices ≥ 3` - `assign_foliation_by_y(num_slices)` — bin existing vertices into time slices - Query methods: `time_label`, `edge_type`, `vertices_at_time`, `slice_sizes`, `has_foliation` diff --git a/scripts/benchmark_models.py b/scripts/benchmark_models.py index 43a667a..524c858 100644 --- a/scripts/benchmark_models.py +++ b/scripts/benchmark_models.py @@ -7,6 +7,7 @@ """ import re +import sys from dataclasses import dataclass @@ -217,7 +218,10 @@ def _append_complete_benchmark(benchmarks: list[BenchmarkData], benchmark: Bench benchmarks.append(benchmark) return - print(f"Warning: skipping incomplete benchmark section for {benchmark.points} Points ({benchmark.dimension}): missing valid Time line") + print( + f"Warning: skipping incomplete benchmark section for {benchmark.points} Points ({benchmark.dimension}): missing valid Time line", + file=sys.stderr, + ) def extract_benchmark_data(baseline_content: str) -> list[BenchmarkData]: diff --git a/scripts/benchmark_utils.py b/scripts/benchmark_utils.py index f37fdd5..5d96f63 100755 --- a/scripts/benchmark_utils.py +++ b/scripts/benchmark_utils.py @@ -30,7 +30,7 @@ from urllib.parse import urlparse from uuid import uuid4 -from packaging.version import Version +from packaging.version import InvalidVersion, Version logger = logging.getLogger(__name__) @@ -182,7 +182,7 @@ def generate_summary(self, output_path: Path | None = None, run_benchmarks: bool print(f"📊 Generated performance summary: {output_path}") return True - except Exception as e: + except OSError as e: print(f"❌ Failed to generate performance summary: {e}", file=sys.stderr) return False @@ -215,7 +215,7 @@ def _generate_markdown_content(self, generator_name: str | None = None) -> str: commit_hash = get_git_commit_hash(cwd=self.project_root) if commit_hash and commit_hash != "unknown": lines.append(f"**Git Commit**: {commit_hash}") - except Exception as e: + except (ExecutableNotFoundError, subprocess.CalledProcessError, subprocess.TimeoutExpired, OSError) as e: logging.debug("Could not get git commit hash: %s", e) # Add hardware information @@ -230,7 +230,7 @@ def _generate_markdown_content(self, generator_name: str | None = None) -> str: f"**Rust**: {hw_info['RUST']}", ], ) - except Exception as e: + except (ExecutableNotFoundError, subprocess.CalledProcessError, subprocess.TimeoutExpired, KeyError, OSError, ValueError) as e: logging.debug("Could not get hardware info: %s", e) lines.append("**Hardware**: Unknown") @@ -281,7 +281,7 @@ def _get_current_version(self) -> str: if result.startswith("v"): return result[1:] # Remove 'v' prefix return "unknown" - except Exception: + except (ExecutableNotFoundError, subprocess.CalledProcessError, subprocess.TimeoutExpired, OSError): # Fallback: try to get any recent tag try: cp = run_git_command(["tag", "-l", "--sort=-version:refname"], cwd=self.project_root) @@ -292,7 +292,7 @@ def _get_current_version(self) -> str: if tag.startswith("v") and len(tag) > 1: return tag[1:] return "unknown" - except Exception: + except (ExecutableNotFoundError, subprocess.CalledProcessError, subprocess.TimeoutExpired, OSError): return "unknown" def _get_version_date(self) -> str: @@ -313,7 +313,7 @@ def _get_version_date(self) -> str: # Fallback to current date return datetime.now(UTC).strftime("%Y-%m-%d") - except Exception: + except (ExecutableNotFoundError, subprocess.CalledProcessError, subprocess.TimeoutExpired, OSError): return datetime.now(UTC).strftime("%Y-%m-%d") def _run_circumsphere_benchmarks(self) -> tuple[bool, dict[str, str] | None]: @@ -340,7 +340,7 @@ def _run_circumsphere_benchmarks(self) -> tuple[bool, dict[str, str] | None]: print("✅ Circumsphere benchmarks completed successfully") return True, numerical_accuracy_data - except Exception as e: + except (ExecutableNotFoundError, subprocess.CalledProcessError, subprocess.TimeoutExpired, OSError) as e: print(f"❌ Error running circumsphere benchmarks: {e}") return False, None @@ -382,7 +382,7 @@ def _parse_numerical_accuracy_output(self, stdout: str) -> dict[str, str] | None return accuracy_data if accuracy_data else None - except Exception: + except ValueError: return None def _get_numerical_accuracy_analysis(self) -> list[str]: @@ -594,7 +594,7 @@ def _parse_single_method_result(self, criterion_path: Path, method_name: str) -> mean_ns = estimates["mean"]["point_estimate"] return CircumspherePerformanceData(method=method_name, time_ns=mean_ns) - except Exception as e: + except (OSError, json.JSONDecodeError, KeyError, TypeError, ValueError) as e: print(f"⚠️ Could not parse {estimates_file}: {e}") return None @@ -853,7 +853,7 @@ def _parse_baseline_results(self) -> list[str]: if benchmarks: lines.extend(format_benchmark_tables(benchmarks)) - except Exception as e: + except (OSError, TypeError, ValueError) as e: lines.extend( [ "### Baseline Results", @@ -902,7 +902,7 @@ def _parse_comparison_results(self) -> list[str]: ], ) - except Exception: + except OSError: lines.extend( [ "### Comparison Results", @@ -1461,7 +1461,7 @@ def generate_baseline(self, dev_mode: bool = False, output_file: Path | None = N print("=== end stdout ===\n", file=sys.stderr) logging.exception("Error in generate_baseline") return False - except Exception: + except (ExecutableNotFoundError, OSError, ValueError): logging.exception("Error in generate_baseline") return False @@ -1475,7 +1475,7 @@ def _write_baseline_file(self, benchmark_results: list[BenchmarkData], output_fi try: # Use secure subprocess wrapper for git command git_commit = get_git_commit_hash(cwd=self.project_root) - except Exception: + except (ExecutableNotFoundError, subprocess.CalledProcessError, subprocess.TimeoutExpired, OSError): git_commit = "unknown" hardware_info = self.hardware.format_hardware_info(cwd=self.project_root) @@ -1586,7 +1586,7 @@ def compare_with_baseline( self._write_error_file(output_file, "Benchmark execution error", str(e)) logging.exception("Error in compare_with_baseline") return False, False - except Exception as e: + except (ExecutableNotFoundError, OSError, ValueError) as e: self._write_error_file(output_file, "Benchmark execution error", str(e)) logging.exception("Error in compare_with_baseline") return False, False @@ -1699,7 +1699,7 @@ def _prepare_comparison_metadata(self, baseline_content: str) -> dict[str, str]: try: git_commit = get_git_commit_hash(cwd=self.project_root) - except Exception: + except (ExecutableNotFoundError, subprocess.CalledProcessError, subprocess.TimeoutExpired, OSError): git_commit = "unknown" # Parse baseline metadata @@ -1937,7 +1937,7 @@ def _write_error_file(self, output_file: Path, error_title: str, error_detail: s f.write(f"Details: {error_detail}\n\n") f.write("This error prevented the benchmark comparison from completing successfully.\n") f.write("Please check the CI logs for more information.\n") - except Exception: + except OSError: logging.exception("Failed to write error file") @@ -2016,7 +2016,7 @@ def create_metadata(tag_name: str, output_dir: Path) -> bool: print(f"📦 Created metadata file: {metadata_file}") return True - except Exception as e: + except (OSError, TypeError, ValueError) as e: print(f"❌ Failed to create metadata: {e}", file=sys.stderr) return False @@ -2052,7 +2052,7 @@ def display_baseline_summary(baseline_file: Path) -> bool: return True - except Exception as e: + except (OSError, UnicodeError) as e: print(f"❌ Failed to display baseline summary: {e}", file=sys.stderr) return False @@ -2222,7 +2222,7 @@ def _version_key(p: Path) -> tuple[int, Version | str, str]: version = Version(version_str) # Valid version: priority 1 (sorts first when reversed) return (1, version, p.name) - except Exception as e: + except InvalidVersion as e: # Invalid version format, treat as non-semver logging.debug("Invalid version format in %s: %s", p.name, e) # Fallback: put non-matching names last (priority 0, sorts after valid versions when reversed) @@ -2358,7 +2358,7 @@ def determine_benchmark_skip(baseline_commit: str, current_commit: str) -> tuple except subprocess.CalledProcessError: return False, "baseline_commit_not_found" - except Exception: + except (ExecutableNotFoundError, subprocess.TimeoutExpired, OSError, ValueError): return False, "error_checking_changes" @staticmethod @@ -2425,7 +2425,7 @@ def run_regression_test(baseline_path: Path, bench_timeout: int = 1800, dev_mode print("✅ No significant performance regressions detected") return True - except Exception as e: + except (ProjectRootNotFoundError, OSError, ValueError) as e: print(f"❌ Error running regression test: {e}", file=sys.stderr) return False diff --git a/scripts/performance_analysis.py b/scripts/performance_analysis.py index 7544297..440e92d 100755 --- a/scripts/performance_analysis.py +++ b/scripts/performance_analysis.py @@ -215,7 +215,7 @@ def run_benchmarks(self) -> bool: except subprocess.TimeoutExpired: print("❌ Benchmark execution timed out") return False - except Exception as exc: + except OSError as exc: print(f"❌ Error running benchmarks: {exc}") return False diff --git a/scripts/tests/test_benchmark_models.py b/scripts/tests/test_benchmark_models.py index 680382b..a933fde 100644 --- a/scripts/tests/test_benchmark_models.py +++ b/scripts/tests/test_benchmark_models.py @@ -231,7 +231,9 @@ def test_extract_benchmark_data_skips_incomplete_sections(self, capsys) -> None: assert len(benchmarks) == 1 assert benchmarks[0].points == 5000 - assert "skipping incomplete benchmark section for 1000 Points (2D)" in capsys.readouterr().out + captured = capsys.readouterr() + assert "skipping incomplete benchmark section for 1000 Points (2D)" in captured.err + assert captured.out == "" def test_parse_benchmark_header(self): """Test parsing benchmark header lines.""" diff --git a/scripts/tests/test_benchmark_utils.py b/scripts/tests/test_benchmark_utils.py index 002898e..9f9476f 100644 --- a/scripts/tests/test_benchmark_utils.py +++ b/scripts/tests/test_benchmark_utils.py @@ -568,7 +568,7 @@ def test_prepare_comparison_metadata(self, mock_datetime, mock_git, comparator, @patch("benchmark_utils.get_git_commit_hash") def test_prepare_comparison_metadata_git_failure(self, mock_git, comparator, sample_baseline_content): """Test metadata preparation when git command fails.""" - mock_git.side_effect = Exception("Git not available") + mock_git.side_effect = subprocess.CalledProcessError(1, ["git", "rev-parse"], stderr="Git not available") metadata = comparator._prepare_comparison_metadata(sample_baseline_content) @@ -1883,7 +1883,7 @@ def side_effect(*args, **kwargs): @patch("benchmark_utils.run_git_command") def test_get_current_version_no_tags(self, mock_git_command): """Test version detection when no tags are found.""" - mock_git_command.side_effect = Exception("No tags found") + mock_git_command.side_effect = subprocess.CalledProcessError(1, ["git", "describe"], stderr="No tags found") with tempfile.TemporaryDirectory() as temp_dir: project_root = Path(temp_dir) @@ -1911,7 +1911,7 @@ def test_get_version_date_with_tag(self, mock_datetime, mock_git_command): # no @patch("benchmark_utils.datetime") def test_get_version_date_fallback(self, mock_datetime, mock_git_command): """Test version date fallback to current date.""" - mock_git_command.side_effect = Exception("Git command failed") + mock_git_command.side_effect = subprocess.CalledProcessError(1, ["git", "log"], stderr="Git command failed") mock_now = Mock() mock_now.strftime.return_value = "2024-01-15" mock_datetime.now.return_value = mock_now @@ -2025,7 +2025,7 @@ def test_parse_comparison_results_no_regression(self): def test_generate_markdown_content(self, mock_datetime, mock_run_git, mock_git_commit): """Test generating complete markdown content.""" # Avoid calling actual git in __init__ helpers - mock_run_git.side_effect = Exception("git unavailable in test") + mock_run_git.side_effect = subprocess.CalledProcessError(1, ["git"], stderr="git unavailable in test") mock_git_commit.return_value = "abc123def456" mock_now = Mock() mock_now.strftime.return_value = "2024-01-15 10:30:00 UTC" @@ -2182,7 +2182,7 @@ def test_run_circumsphere_benchmarks_with_numerical_data(self, mock_cargo): @patch("benchmark_utils.run_cargo_command") def test_run_circumsphere_benchmarks_failure(self, mock_cargo, capsys): """Test handling circumsphere benchmark failures.""" - mock_cargo.side_effect = Exception("Benchmark failed") + mock_cargo.side_effect = subprocess.CalledProcessError(1, ["cargo", "bench"], stderr="Benchmark failed") with tempfile.TemporaryDirectory() as temp_dir: project_root = Path(temp_dir) @@ -2200,7 +2200,7 @@ def test_run_circumsphere_benchmarks_failure(self, mock_cargo, capsys): @patch("benchmark_utils.run_git_command") def test_generate_summary_success(self, mock_git, capsys): """Test successful generation of performance summary.""" - mock_git.side_effect = Exception("git unavailable in test") + mock_git.side_effect = subprocess.CalledProcessError(1, ["git"], stderr="git unavailable in test") with tempfile.TemporaryDirectory() as temp_dir: project_root = Path(temp_dir) generator = PerformanceSummaryGenerator(project_root) @@ -2325,8 +2325,8 @@ def test_missing_git_info_edge_case(self): patch("benchmark_utils.run_git_command") as mock_git, patch("benchmark_utils.get_git_commit_hash") as mock_commit, ): - mock_git.side_effect = Exception("Git not available") - mock_commit.side_effect = Exception("Git not available") + mock_git.side_effect = subprocess.CalledProcessError(1, ["git"], stderr="Git not available") + mock_commit.side_effect = subprocess.CalledProcessError(1, ["git", "rev-parse"], stderr="Git not available") generator = PerformanceSummaryGenerator(project_root) success = generator.generate_summary(output_file) diff --git a/semgrep.yaml b/semgrep.yaml index d09a8af..7b7ec3d 100644 --- a/semgrep.yaml +++ b/semgrep.yaml @@ -121,6 +121,9 @@ rules: - pattern: $VALUE.unwrap_or_else(|| f64::NAN) - pattern: $VALUE.unwrap_or_else(|| f64::INFINITY) - pattern: $VALUE.unwrap_or_else(|| f64::NEG_INFINITY) + - pattern: $VALUE.unwrap_or_else(|| std::f64::NAN) + - pattern: $VALUE.unwrap_or_else(|| std::f64::INFINITY) + - pattern: $VALUE.unwrap_or_else(|| std::f64::NEG_INFINITY) - id: causal-triangulations.rust.no-direct-delaunay-imports-outside-geometry languages: @@ -318,13 +321,8 @@ rules: of swallowing unrelated errors. paths: include: - - "/scripts/archive_changelog.py" - - "/scripts/benchmark_models.py" - - "/scripts/coverage_report.py" - - "/scripts/postprocess_changelog.py" - - "/scripts/subprocess_utils.py" - - "/scripts/tag_release.py" - - "/scripts/tests/**/*.py" + - "/scripts/**/*.py" + - "/tests/semgrep/scripts/tests/**/*.py" pattern-regex: '^\s*except\s+Exception(?:\s+as\s+\w+)?\s*:' - id: causal-triangulations.python.no-adhoc-completedprocess-mock diff --git a/src/cdt/ergodic_moves.rs b/src/cdt/ergodic_moves.rs index d8858c6..fa8eb68 100644 --- a/src/cdt/ergodic_moves.rs +++ b/src/cdt/ergodic_moves.rs @@ -13,7 +13,6 @@ use crate::errors::CdtError; use crate::geometry::CdtTriangulation2D; use crate::geometry::backends::delaunay::{DelaunayFaceHandle, DelaunayVertexHandle}; use crate::geometry::traits::{EdgeAdjacentFaces, TriangulationMut, TriangulationQuery}; -use num_traits::ToPrimitive; use rand::{RngExt, SeedableRng, rngs::StdRng}; /// Types of ergodic moves available in 2D CDT. @@ -187,8 +186,12 @@ impl MoveStatistics { } /// Converts an accumulated move counter to a finite value for rate reporting. -fn count_to_f64(count: u64) -> f64 { - count.to_f64().map_or(f64::MAX, |value| value) +#[expect( + clippy::cast_precision_loss, + reason = "move counters are converted only for aggregate acceptance-rate reporting" +)] +const fn count_to_f64(count: u64) -> f64 { + count as f64 } /// Ergodic move system for CDT triangulations. diff --git a/src/cdt/triangulation.rs b/src/cdt/triangulation.rs index 417a24c..d97f16b 100644 --- a/src/cdt/triangulation.rs +++ b/src/cdt/triangulation.rs @@ -10,8 +10,10 @@ use crate::geometry::DelaunayBackend2D; use crate::geometry::backends::delaunay::{ DelaunayEdgeHandle, DelaunayFaceHandle, DelaunayVertexHandle, }; +#[cfg(test)] +use crate::geometry::generators::build_delaunay2_with_data; use crate::geometry::generators::{ - build_delaunay2_with_data, build_toroidal_delaunay2, generate_delaunay2, + build_delaunay2_from_cells, build_toroidal_delaunay2, generate_delaunay2, }; use crate::geometry::traits::{TriangulationMut, TriangulationQuery}; use crate::util::f64_band_to_u32; @@ -93,6 +95,25 @@ fn remap_toroidal_generation_error(error: CdtError, total_vertices: u32) -> CdtE } } +/// Rewrites explicit strip builder failures with CDT-level generation context. +fn remap_strip_generation_error( + error: CdtError, + total_vertices: u32, + coordinate_max: f64, +) -> CdtError { + match error { + CdtError::DelaunayGenerationFailed { + underlying_error, .. + } => CdtError::DelaunayGenerationFailed { + vertex_count: total_vertices, + coordinate_range: (0.0, coordinate_max), + attempt: 1, + underlying_error, + }, + other => other, + } +} + /// Events in simulation history #[derive(Debug, Clone)] pub enum SimulationEvent { @@ -1009,6 +1030,7 @@ impl CdtTriangulation { // ------------------------------------------------------------------------- /// Normalizes provisional cylinder-construction failures into the public generation error. + #[cfg(test)] fn foliated_cylinder_generation_error( total_vertices: u32, num_slices: u32, @@ -1023,6 +1045,7 @@ impl CdtTriangulation { } /// Reconstructs generated foliation counts while reporting missing labels with examples. + #[cfg(test)] fn slice_sizes_from_vertex_labels( backend: &DelaunayBackend2D, total_vertices: u32, @@ -1244,36 +1267,27 @@ impl CdtTriangulation { /// Construct a foliated 1+1 CDT triangulation on a finite strip. /// - /// Places `vertices_per_slice` vertices per time slice on a regular grid at - /// coordinates `(x_i, t)` where `x_i` is evenly spaced in `[0, 1]` and - /// `t` is an integer time coordinate. Time labels are assigned by - /// y-coordinate bucket: slice `t` owns vertices with `y ∈ [t − 0.5, t + 0.5)`. + /// Places `vertices_per_slice` vertices per time slice on a regular grid + /// and connects adjacent slices combinatorially. Time labels are stored + /// directly in vertex data. /// /// **Note:** Despite the name, this builds an open strip `[0,1] × [0,T]` /// without spatial periodic identification. True cylinder topology /// (S¹ × \[0,T\]) is planned for a future release. /// - /// The spatial extent is kept to 1.0 (well below the √3 ≈ 1.73 threshold - /// that prevents Delaunay edges from skipping a time slice), but generic - /// Delaunay triangulation can still introduce same-slice triangles. This - /// constructor therefore validates the result and returns an error unless - /// the output is a genuine 1+1 CDT strip. - /// - /// This constructor is provisional and crate-internal until the explicit - /// strip builder path is implemented in [`from_cdt_strip`](Self::from_cdt_strip). + /// This crate-internal compatibility constructor now delegates to the + /// explicit strip builder in [`from_cdt_strip`](Self::from_cdt_strip). /// /// # Arguments /// /// * `vertices_per_slice` — Number of vertices in each spatial slice (≥ 4). /// * `num_slices` — Number of time slices (≥ 2). - /// * `seed` — Optional seed for deterministic vertex perturbation. + /// * `seed` — Ignored; retained for compatibility with older diagnostics. /// /// # Errors /// - /// Returns error if parameters are invalid, vertex construction fails, - /// triangulation construction fails, or the builder output does not retain - /// valid per-vertex time labels. Builder-label failures are surfaced as - /// [`CdtError::DelaunayGenerationFailed`] with detailed context. + /// Returns error if parameters are invalid, explicit mesh construction + /// fails, or the output violates CDT strip invariants. /// /// # Internal /// @@ -1282,7 +1296,7 @@ impl CdtTriangulation { not(test), expect( dead_code, - reason = "provisional internal strip constructor remains intentionally crate-internal until explicit strip construction lands" + reason = "compatibility constructor is crate-internal; public callers should use from_cdt_strip" ) )] pub(crate) fn from_foliated_cylinder( @@ -1290,8 +1304,37 @@ impl CdtTriangulation { num_slices: u32, seed: Option, ) -> CdtResult { - // TODO(#57): Remove this provisional point-set constructor once the - // explicit combinatorial strip builder is available. + let _ = seed; + Self::from_cdt_strip(vertices_per_slice, num_slices) + } + + /// Construct a true 1+1 CDT strip by explicit layered connectivity. + /// + /// Places `vertices_per_slice` vertices on each open spatial slice and + /// connects adjacent time slices into quads. Each quad is split into one + /// Up `(2,1)` triangle and one Down `(1,2)` triangle, so every finite face + /// is classifiable by construction. + /// + /// # Errors + /// + /// Returns [`CdtError::InvalidGenerationParameters`] if `vertices_per_slice < 4`, + /// `num_slices < 2`, or the derived vertex or cell count overflows `u32`. + /// Returns [`CdtError::DelaunayGenerationFailed`] if the underlying + /// explicit builder rejects the mesh, and [`CdtError::ValidationFailed`] if + /// the constructed strip fails CDT validation. + /// + /// # Examples + /// + /// ``` + /// use causal_triangulations::prelude::triangulation::*; + /// + /// let tri = CdtTriangulation::from_cdt_strip(4, 2) + /// .expect("build explicit CDT strip"); + /// assert_eq!(tri.vertex_count(), 8); + /// assert_eq!(tri.face_count(), 6); + /// assert!(tri.validate_cell_classification().is_ok()); + /// ``` + pub fn from_cdt_strip(vertices_per_slice: u32, num_slices: u32) -> CdtResult { if vertices_per_slice < 4 { return Err(CdtError::InvalidGenerationParameters { issue: "Insufficient vertices per slice".to_string(), @@ -1307,20 +1350,6 @@ impl CdtTriangulation { }); } - // Spatial extent is fixed at 1.0 so that the maximum within-strip - // circumradius stays below 1.0 (the temporal gap between slices). - // This guarantees the Delaunay property cannot create edges that - // skip a time slice. - // - // Small deterministic perturbation is applied to break co-circular - // degeneracy in the grid. Boundary vertices (i=0 and i=last) keep - // their exact x so they remain collinear on the convex hull, - // preventing hull edges from skipping intermediate time slices. - let spatial_extent = 1.0_f64; - let spacing = spatial_extent / f64::from(vertices_per_slice - 1); - let perturbation_seed = seed.unwrap_or(0); - let perturbation_scale = spacing * 0.02; - let total_vertices = vertices_per_slice.checked_mul(num_slices).ok_or_else(|| { CdtError::InvalidGenerationParameters { issue: "Vertex count overflow".to_string(), @@ -1328,161 +1357,85 @@ impl CdtTriangulation { expected_range: "product ≤ u32::MAX".to_string(), } })?; - let mut vertex_specs = Vec::with_capacity(total_vertices as usize); - let last_i = vertices_per_slice - 1; + let spatial_quads = vertices_per_slice - 1; + let temporal_quads = num_slices - 1; + let total_quads = spatial_quads.checked_mul(temporal_quads).ok_or_else(|| { + CdtError::InvalidGenerationParameters { + issue: "Cell count overflow".to_string(), + provided_value: format!("{spatial_quads} × {temporal_quads}"), + expected_range: "product ≤ u32::MAX".to_string(), + } + })?; + let total_cells = + total_quads + .checked_mul(2) + .ok_or_else(|| CdtError::InvalidGenerationParameters { + issue: "Cell count overflow".to_string(), + provided_value: format!("2 × {total_quads}"), + expected_range: "product ≤ u32::MAX".to_string(), + })?; + + let n = vertices_per_slice as usize; + let t_count = num_slices as usize; + let index = |i: usize, t: usize| -> usize { t * n + i }; + + let spacing = 1.0_f64 / f64::from(vertices_per_slice - 1); + let mut vertex_specs: Vec<([f64; 2], u32)> = Vec::with_capacity(total_vertices as usize); for t in 0..num_slices { for i in 0..vertices_per_slice { - let x_base = f64::from(i) * spacing; - let y_base = f64::from(t); // integer time coordinate - - // Deterministic perturbation keyed on vertex index + seed - let hash = (u64::from(t) * u64::from(vertices_per_slice) + u64::from(i)) - .wrapping_mul(6_364_136_223_846_793_005) - .wrapping_add(perturbation_seed) - .wrapping_mul(1_442_695_040_888_963_407); - - // Perturbation strategy: - // - Interior vertices: hash-based perturbation in x only - // to break co-circular grid degeneracy while preserving - // exact per-slice collinearity in y. - // - Boundary vertices (i=0, i=last): deterministic concave - // x-offset via √(t+1) pushes each row further outward. - // The concave (sub-linear) progression ensures every - // intermediate boundary vertex bulges past the line - // connecting its neighbors, so the convex hull includes - // every row's extremes and no hull edge skips a time slice. - let hash_frac = f64::from((hash & 0xFFFF) as u16) / 65535.0; - let hull_offset = f64::from(t + 1).sqrt(); - let px = if i == 0 { - // Push left — concave √(t+1) ensures hull membership - -hull_offset * perturbation_scale - } else if i == last_i { - // Push right — mirror of left - hull_offset * perturbation_scale - } else { - (hash_frac - 0.5) * perturbation_scale - }; + vertex_specs.push(([f64::from(i) * spacing, f64::from(t)], t)); + } + } - // Keep y exactly on integer slices so same-slice triangles - // are geometrically impossible (three same-slice points are - // collinear), enforcing one-spacelike-two-timelike structure. - vertex_specs.push(([x_base + px, y_base], t)); + let mut cells: Vec> = Vec::with_capacity(total_cells as usize); + for t in 0..(t_count - 1) { + let t_next = t + 1; + for i in 0..(n - 1) { + let i_next = i + 1; + cells.push(vec![index(i, t), index(i_next, t), index(i, t_next)]); + cells.push(vec![ + index(i_next, t), + index(i_next, t_next), + index(i, t_next), + ]); } } - // Delegate low-level Delaunay construction to the utility layer. - let dt = build_delaunay2_with_data(&vertex_specs).map_err(|e| match e { - CdtError::DelaunayGenerationFailed { - underlying_error, .. - } => Self::foliated_cylinder_generation_error( - total_vertices, - num_slices, - underlying_error, - ), - other => other, - })?; + let coordinate_max = f64::from(num_slices - 1).max(1.0); + let dt = build_delaunay2_from_cells(&vertex_specs, &cells) + .map_err(|err| remap_strip_generation_error(err, total_vertices, coordinate_max))?; let backend = DelaunayBackend2D::from_triangulation(dt); - - // Verify the builder inserted all vertices. if backend.vertex_count() != total_vertices as usize { - return Err(Self::foliated_cylinder_generation_error( - total_vertices, - num_slices, - format!( - "builder inserted only {} of {} vertices (possible degeneracy)", + return Err(CdtError::DelaunayGenerationFailed { + vertex_count: total_vertices, + coordinate_range: (0.0, coordinate_max), + attempt: 1, + underlying_error: format!( + "explicit strip builder produced {} vertices, expected {}", backend.vertex_count(), total_vertices, ), - )); + }); } - // Compute per-slice vertex counts from vertex data stored in the backend. - let slice_sizes = - Self::slice_sizes_from_vertex_labels(&backend, total_vertices, num_slices)?; - + let slice_sizes = vec![n; t_count]; let foliation = Foliation::from_slice_sizes(slice_sizes, num_slices).map_err(CdtError::from)?; + let mut tri = Self::try_new(backend, num_slices, 2)?; tri.foliation = Some(foliation); tri.mark_foliation_synchronized(); - tri.validate_foliation().map_err(|err| { - Self::foliated_cylinder_generation_error( - total_vertices, - num_slices, - format!("constructed strip has invalid foliation: {err}"), - ) - })?; - - tri.validate_causality_delaunay().map_err(|err| { - Self::foliated_cylinder_generation_error( - total_vertices, - num_slices, - format!( - "point-set Delaunay produced a non-CDT triangulation; explicit CDT strip construction is required: {err}" - ), - ) - })?; + tri.validate_foliation()?; + tri.validate_causality_delaunay()?; + tri.validate_topology()?; + tri.classify_all_cells()?; Ok(tri) } - /// Construct a true 1+1 CDT strip by explicit layered connectivity. - /// - /// Unlike `from_foliated_cylinder`, this does NOT rely on Delaunay triangulation. - /// Instead it constructs the CDT combinatorially so every triangle is guaranteed - /// to satisfy the CDT invariant (1 spacelike edge, 2 timelike edges). - /// - /// NOTE: This requires backend support for explicit face construction. - /// Currently this is a placeholder until such support is implemented. - /// - /// # Errors - /// - /// Returns [`CdtError::InvalidGenerationParameters`] if `vertices_per_slice < 4` or - /// `num_slices < 2`. - /// Returns [`CdtError::UnsupportedOperation`] because explicit CDT mesh construction is - /// not yet implemented. - /// - /// # Examples - /// - /// ``` - /// use causal_triangulations::prelude::triangulation::*; - /// - /// // Placeholder API: currently validates inputs, then returns a construction error. - /// let result = CdtTriangulation::from_cdt_strip(4, 2); - /// assert!(result.is_err()); - /// ``` - pub fn from_cdt_strip(vertices_per_slice: u32, num_slices: u32) -> CdtResult { - if vertices_per_slice < 4 { - return Err(CdtError::InvalidGenerationParameters { - issue: "Insufficient vertices per slice".to_string(), - provided_value: vertices_per_slice.to_string(), - expected_range: "≥ 4".to_string(), - }); - } - if num_slices < 2 { - return Err(CdtError::InvalidGenerationParameters { - issue: "Insufficient number of time slices".to_string(), - provided_value: num_slices.to_string(), - expected_range: "≥ 2".to_string(), - }); - } - - // TODO: Implement explicit CDT mesh construction. - // This requires: - // 1. Creating vertices per slice - // 2. Connecting adjacent slices into quads - // 3. Splitting quads into valid CDT triangles - // 4. Building backend without relying on Delaunay - - Err(CdtError::UnsupportedOperation { - operation: "CdtTriangulation::from_cdt_strip".to_string(), - reason: "explicit CDT strip construction is not yet implemented; requires explicit mesh backend".to_string(), - }) - } - /// Construct a foliated 1+1 CDT on a torus (S¹×S¹). /// /// Places `vertices_per_slice` vertices per time slice, uniformly spaced @@ -1629,6 +1582,7 @@ impl CdtTriangulation { tri.validate_foliation()?; tri.validate_causality_delaunay()?; tri.validate_topology()?; + tri.classify_all_cells()?; Ok(tri) } @@ -1650,7 +1604,9 @@ impl CdtTriangulation { /// # Errors /// /// Returns error if `num_slices` is zero, if vertex coordinates cannot be - /// read, or if y-bucket assignment would leave any time slice empty. + /// read, if y-bucket assignment would leave any time slice empty, if the + /// requested slice count violates the triangulation topology, or if writing + /// vertex labels or clearing stale cell labels in the backend fails. /// /// # Examples /// @@ -2093,8 +2049,7 @@ impl CdtTriangulation { /// use causal_triangulations::prelude::geometry::*; /// use causal_triangulations::CdtTriangulation; /// - /// let mut tri = CdtTriangulation::from_seeded_points(12, 3, 2, 42).unwrap(); - /// tri.assign_foliation_by_y(3).unwrap(); + /// let mut tri = CdtTriangulation::from_cdt_strip(4, 2).unwrap(); /// tri.classify_all_cells().unwrap(); /// let face = tri.geometry().faces().next().unwrap(); /// let _stored = tri.cell_type_from_data(&face); @@ -2174,28 +2129,68 @@ impl CdtTriangulation { ]) } + /// Validates that every finite face has a strict CDT cell classification. + /// + /// If no foliation is present, succeeds vacuously. Otherwise every face + /// must classify as [`CellType::Up`] or [`CellType::Down`]. + /// + /// # Errors + /// + /// Returns [`CdtError::ValidationFailed`] if any face is unclassifiable. + /// + /// # Examples + /// + /// ``` + /// use causal_triangulations::prelude::triangulation::*; + /// + /// let tri = CdtTriangulation::from_cdt_strip(4, 2) + /// .expect("build explicit CDT strip"); + /// tri.validate_cell_classification() + /// .expect("all strip cells classify"); + /// ``` + pub fn validate_cell_classification(&self) -> CdtResult<()> { + if !self.has_foliation() { + return Ok(()); + } + + for face in self.geometry.faces() { + if self.cell_type(&face).is_none() { + return Err(CdtError::ValidationFailed { + check: "cell_classification".to_string(), + detail: format!( + "face {:?} is not a strict CDT cell (expected Up or Down)", + face.cell_key() + ), + }); + } + } + + Ok(()) + } + /// Classifies every triangle and stores the result as cell data. /// /// Each classifiable cell receives `Some(CellType::to_i32())` via - /// `set_cell_data`. Boundary cells that do not span exactly one - /// time slice are skipped. + /// `set_cell_data`. On a foliated triangulation, all finite faces must be + /// classifiable; unclassifiable same-slice or multi-slice triangles are + /// reported as validation failures. /// /// Requires a foliation to be present; returns `Ok(None)` if there is none. /// /// # Errors /// - /// Returns [`CdtError::BackendMutationFailed`] if writing cell payloads - /// to the backend fails. + /// Returns [`CdtError::ValidationFailed`] if any foliated face is not an + /// Up or Down CDT triangle. Returns [`CdtError::BackendMutationFailed`] if + /// writing cell payloads to the backend fails, or + /// [`CdtError::BackendRollbackFailed`] if restoring previous payloads also fails. /// /// # Examples /// /// ``` /// use causal_triangulations::prelude::triangulation::*; /// - /// let mut tri = CdtTriangulation::from_seeded_points(12, 3, 2, 42) - /// .expect("create seeded triangulation"); - /// tri.assign_foliation_by_y(3) - /// .expect("assign foliation from y-coordinates"); + /// let mut tri = CdtTriangulation::from_cdt_strip(4, 2) + /// .expect("create explicit strip"); /// let classified = tri /// .classify_all_cells() /// .expect("classify cells") @@ -2207,45 +2202,90 @@ impl CdtTriangulation { return Ok(None); } - // Collect (CellKey, CellType) pairs first to avoid borrow conflict. - let classifications: Vec<_> = self - .geometry - .faces() - .filter_map(|face| { - let ct = self.cell_type(&face)?; - Some((face, ct)) + let faces: Vec<_> = self.geometry.faces().collect(); + let mut classifications = Vec::with_capacity(faces.len()); + for face in &faces { + let Some(ct) = self.cell_type(face) else { + return Err(CdtError::ValidationFailed { + check: "cell_classification".to_string(), + detail: format!( + "face {:?} is not a strict CDT cell (expected Up or Down)", + face.cell_key() + ), + }); + }; + classifications.push((face.cell_key(), ct)); + } + + let count = classifications.len(); + let previous_cell_data: Vec<_> = faces + .iter() + .map(|face| { + let key = face.cell_key(); + (key, self.geometry.cell_data_by_key(key)) }) .collect(); + let rollback_cell_payloads = |geometry: &mut DelaunayBackend2D| -> Vec { + let mut rollback_errors = Vec::new(); - // Also collect all face keys to clear stale data from unclassifiable faces. - let all_face_keys: Vec<_> = self.geometry.faces().map(|f| f.cell_key()).collect(); + for &(key, data) in &previous_cell_data { + if let Err(err) = geometry.set_cell_data_by_key(key, data) { + rollback_errors.push(format!("face {key:?}: {err}")); + } + } - let count = classifications.len(); + rollback_errors + }; // Clear all cell data first, then write fresh classifications. - for &key in &all_face_keys { - self.geometry - .set_cell_data_by_key(key, None) - .map_err(|err| CdtError::BackendMutationFailed { - operation: "set_cell_data_by_key".to_string(), - target: format!("face {key:?}"), - detail: format!( - "failed to clear existing cell payload before classification: {err}" - ), - })?; - } - for (face, ct) in classifications { + for face in &faces { let key = face.cell_key(); - self.geometry - .set_cell_data_by_key(key, Some(ct.to_i32())) - .map_err(|err| CdtError::BackendMutationFailed { - operation: "set_cell_data_by_key".to_string(), - target: format!("face {key:?}"), - detail: format!( - "failed to store classified cell payload {}: {err}", - ct.to_i32() - ), - })?; + if let Err(err) = self.geometry.set_cell_data_by_key(key, None) { + let operation = "set_cell_data_by_key".to_string(); + let target = format!("face {key:?}"); + let detail = + format!("failed to clear existing cell payload before classification: {err}"); + let rollback_errors = rollback_cell_payloads(&mut self.geometry); + return if rollback_errors.is_empty() { + Err(CdtError::BackendMutationFailed { + operation, + target, + detail, + }) + } else { + Err(CdtError::BackendRollbackFailed { + operation, + target, + detail, + rollback_errors: rollback_errors.join("; "), + }) + }; + } + } + for (key, ct) in classifications { + if let Err(err) = self.geometry.set_cell_data_by_key(key, Some(ct.to_i32())) { + let operation = "set_cell_data_by_key".to_string(); + let target = format!("face {key:?}"); + let detail = format!( + "failed to store classified cell payload {}: {err}", + ct.to_i32() + ); + let rollback_errors = rollback_cell_payloads(&mut self.geometry); + return if rollback_errors.is_empty() { + Err(CdtError::BackendMutationFailed { + operation, + target, + detail, + }) + } else { + Err(CdtError::BackendRollbackFailed { + operation, + target, + detail, + rollback_errors: rollback_errors.join("; "), + }) + }; + } } Ok(Some(count)) } @@ -2321,6 +2361,7 @@ impl CdtTriangulation { self.validate_topology()?; self.validate_foliation()?; self.validate_causality()?; + self.validate_cell_classification()?; Ok(()) } @@ -3379,52 +3420,68 @@ mod tests { // Foliation tests // ========================================================================= - /// Accepts the current point-set cylinder limitations while explicit strip construction is pending. - fn assert_foliated_cylinder_known_failure( + /// Verifies the crate-internal strip compatibility constructor returns a strict CDT mesh. + fn assert_valid_foliated_cylinder( result: CdtResult>, - ) { - match result { - Err(CdtError::DelaunayGenerationFailed { - underlying_error, .. - }) => { - let rejected_as_non_cdt = underlying_error.contains("non-CDT triangulation") - || underlying_error.contains("invalid CDT triangle"); - let rejected_for_vertex_drop = underlying_error.contains("builder inserted only"); - assert!( - rejected_as_non_cdt || rejected_for_vertex_drop, - "Expected non-CDT or vertex-drop rejection, got: {underlying_error}" - ); - } - Ok(_) => panic!("Expected point-set strip construction rejection"), - Err(other) => panic!("Expected DelaunayGenerationFailed, got {other:?}"), + vertices_per_slice: u32, + num_slices: u32, + ) -> CdtTriangulation { + let tri = result.expect("explicit strip construction should succeed"); + assert_eq!( + tri.vertex_count(), + vertices_per_slice as usize * num_slices as usize + ); + assert_eq!( + tri.face_count(), + 2 * (vertices_per_slice as usize - 1) * (num_slices as usize - 1) + ); + assert_eq!( + tri.slice_sizes(), + vec![vertices_per_slice as usize; num_slices as usize].as_slice() + ); + tri.validate_foliation() + .expect("explicit strip foliation should validate"); + tri.validate_causality_delaunay() + .expect("explicit strip causality should validate"); + tri.validate_topology() + .expect("explicit strip topology should validate"); + tri.validate_cell_classification() + .expect("all explicit strip cells should classify"); + for face in tri.geometry().faces() { + assert!(tri.cell_type(&face).is_some()); + assert!(tri.cell_type_from_data(&face).is_some()); } + tri } #[test] fn test_from_foliated_cylinder_basic() { - assert_foliated_cylinder_known_failure(CdtTriangulation::from_foliated_cylinder( + assert_valid_foliated_cylinder( + CdtTriangulation::from_foliated_cylinder(5, 3, Some(42)), 5, 3, - Some(42), - )); + ); } #[test] fn test_from_foliated_cylinder_vertex_counts_per_slice() { - assert_foliated_cylinder_known_failure(CdtTriangulation::from_foliated_cylinder( + assert_valid_foliated_cylinder( + CdtTriangulation::from_foliated_cylinder(4, 3, Some(1)), 4, 3, - Some(1), - )); + ); } #[test] fn test_from_foliated_cylinder_all_vertices_labeled() { - assert_foliated_cylinder_known_failure(CdtTriangulation::from_foliated_cylinder( + let tri = assert_valid_foliated_cylinder( + CdtTriangulation::from_foliated_cylinder(5, 3, Some(99)), 5, 3, - Some(99), - )); + ); + for vertex in tri.geometry().vertices() { + assert!(tri.time_label(&vertex).is_some()); + } } #[test] @@ -3713,43 +3770,53 @@ mod tests { #[test] fn test_from_foliated_cylinder_edge_classification() { - assert_foliated_cylinder_known_failure(CdtTriangulation::from_foliated_cylinder( + let tri = assert_valid_foliated_cylinder( + CdtTriangulation::from_foliated_cylinder(5, 3, Some(42)), 5, 3, - Some(42), - )); + ); + for edge in tri.geometry().edges() { + assert!(matches!( + tri.edge_type(&edge), + Some(EdgeType::Spacelike | EdgeType::Timelike) + )); + } } #[test] fn test_from_foliated_cylinder_validate_foliation() { - assert_foliated_cylinder_known_failure(CdtTriangulation::from_foliated_cylinder( + assert_valid_foliated_cylinder( + CdtTriangulation::from_foliated_cylinder(5, 3, Some(42)), 5, 3, - Some(42), - )); + ); } #[test] fn test_from_foliated_cylinder_validate_causality() { - assert_foliated_cylinder_known_failure(CdtTriangulation::from_foliated_cylinder( + assert_valid_foliated_cylinder( + CdtTriangulation::from_foliated_cylinder(5, 3, Some(42)), 5, 3, - Some(42), - )); + ); } #[test] fn test_from_foliated_cylinder_seed_determinism() { - assert_foliated_cylinder_known_failure(CdtTriangulation::from_foliated_cylinder( + let left = assert_valid_foliated_cylinder( + CdtTriangulation::from_foliated_cylinder(5, 3, Some(42)), 5, 3, - Some(42), - )); - assert_foliated_cylinder_known_failure(CdtTriangulation::from_foliated_cylinder( + ); + let right = assert_valid_foliated_cylinder( + CdtTriangulation::from_foliated_cylinder(5, 3, Some(42)), 5, 3, - Some(42), - )); + ); + assert_eq!(left.vertex_count(), right.vertex_count()); + assert_eq!(left.edge_count(), right.edge_count()); + assert_eq!(left.face_count(), right.face_count()); + assert_eq!(left.slice_sizes(), right.slice_sizes()); } #[test] @@ -3777,7 +3844,7 @@ mod tests { } #[test] - fn test_from_cdt_strip_rejects_invalid_params_before_placeholder_error() { + fn test_from_cdt_strip_rejects_invalid_params() { let few_vertices = CdtTriangulation::from_cdt_strip(3, 3); assert!(matches!( few_vertices, @@ -3804,24 +3871,42 @@ mod tests { } #[test] - fn test_from_cdt_strip_reports_placeholder_for_valid_params() { - let result = CdtTriangulation::from_cdt_strip(4, 2); + fn test_from_cdt_strip_rejects_cell_count_overflow() { + let result = CdtTriangulation::from_cdt_strip(65_535, 65_537); assert!(matches!( result, - Err(CdtError::UnsupportedOperation { ref operation, ref reason }) - if operation == "CdtTriangulation::from_cdt_strip" - && reason.contains("not yet implemented") + Err(CdtError::InvalidGenerationParameters { + ref issue, + ref provided_value, + ref expected_range, + }) if issue == "Cell count overflow" + && provided_value == "2 × 4294836224" + && expected_range == "product ≤ u32::MAX" )); } + #[test] + fn test_from_cdt_strip_builds_valid_mesh() { + let tri = CdtTriangulation::from_cdt_strip(4, 2).expect("explicit strip should build"); + assert_eq!(tri.vertex_count(), 8); + assert_eq!(tri.face_count(), 6); + assert!(tri.validate_topology().is_ok()); + assert!(tri.validate_foliation().is_ok()); + assert!(tri.validate_causality_delaunay().is_ok()); + assert!(tri.validate_cell_classification().is_ok()); + } + #[test] fn test_vertices_at_time() { - assert_foliated_cylinder_known_failure(CdtTriangulation::from_foliated_cylinder( + let tri = assert_valid_foliated_cylinder( + CdtTriangulation::from_foliated_cylinder(4, 3, Some(1)), 4, 3, - Some(1), - )); + ); + for t in 0..3 { + assert_eq!(tri.vertices_at_time(t).len(), 4); + } } #[test] @@ -3945,36 +4030,50 @@ mod tests { #[test] fn test_from_foliated_cylinder_minimum_size() { - assert_foliated_cylinder_known_failure(CdtTriangulation::from_foliated_cylinder( + assert_valid_foliated_cylinder( + CdtTriangulation::from_foliated_cylinder(4, 2, Some(1)), 4, 2, - Some(1), - )); + ); } #[test] fn test_from_foliated_cylinder_full_validate() { - assert_foliated_cylinder_known_failure(CdtTriangulation::from_foliated_cylinder( + let tri = assert_valid_foliated_cylinder( + CdtTriangulation::from_foliated_cylinder(5, 3, Some(42)), 5, 3, - Some(42), - )); + ); + assert!(tri.geometry().is_valid()); } #[test] fn test_from_foliated_cylinder_no_seed() { - assert_foliated_cylinder_known_failure(CdtTriangulation::from_foliated_cylinder( - 5, 3, None, - )); + assert_valid_foliated_cylinder(CdtTriangulation::from_foliated_cylinder(5, 3, None), 5, 3); } #[test] fn test_all_faces_are_valid_cdt_triangles() { - assert_foliated_cylinder_known_failure(CdtTriangulation::from_foliated_cylinder( + let tri = assert_valid_foliated_cylinder( + CdtTriangulation::from_foliated_cylinder(5, 3, Some(42)), 5, 3, - Some(42), - )); + ); + for face in tri.geometry().faces() { + let edge_types = tri + .face_edge_types(&face) + .expect("explicit strip face should expose edge types"); + let spacelike = edge_types + .iter() + .filter(|edge_type| matches!(edge_type, EdgeType::Spacelike)) + .count(); + let timelike = edge_types + .iter() + .filter(|edge_type| matches!(edge_type, EdgeType::Timelike)) + .count(); + assert_eq!(spacelike, 1); + assert_eq!(timelike, 2); + } } #[test] @@ -3995,19 +4094,11 @@ mod tests { #[test] fn test_from_foliated_cylinder_larger_grid() { - let result = CdtTriangulation::from_foliated_cylinder(10, 8, Some(7)); - match result { - Err(CdtError::DelaunayGenerationFailed { - underlying_error, .. - }) => { - assert!( - underlying_error.contains("Delaunay repair postcondition failed"), - "Expected explicit Delaunay repair failure, got: {underlying_error}" - ); - } - Ok(_) => panic!("Expected larger grid generation to fail"), - Err(other) => panic!("Expected DelaunayGenerationFailed, got {other:?}"), - } + assert_valid_foliated_cylinder( + CdtTriangulation::from_foliated_cylinder(10, 8, Some(7)), + 10, + 8, + ); } #[test] @@ -4033,11 +4124,17 @@ mod tests { #[test] fn test_cell_type_returns_up_or_down() { - assert_foliated_cylinder_known_failure(CdtTriangulation::from_foliated_cylinder( + let tri = assert_valid_foliated_cylinder( + CdtTriangulation::from_foliated_cylinder(5, 3, Some(42)), 5, 3, - Some(42), - )); + ); + for face in tri.geometry().faces() { + assert!(matches!( + tri.cell_type(&face), + Some(CellType::Up | CellType::Down) + )); + } } #[test] @@ -4052,11 +4149,16 @@ mod tests { #[test] fn test_classify_all_cells_stores_data() { - assert_foliated_cylinder_known_failure(CdtTriangulation::from_foliated_cylinder( + let mut tri = assert_valid_foliated_cylinder( + CdtTriangulation::from_foliated_cylinder(5, 3, Some(42)), 5, 3, - Some(42), - )); + ); + let classified = tri + .classify_all_cells() + .expect("strict strip cells should classify") + .expect("foliation is present"); + assert_eq!(classified, tri.face_count()); } #[test] @@ -4069,6 +4171,14 @@ mod tests { ); } + #[test] + fn test_validate_cell_classification_no_foliation_succeeds() { + let tri = CdtTriangulation::from_random_points(5, 2, 2).unwrap(); + + tri.validate_cell_classification() + .expect("missing foliation should validate vacuously"); + } + #[test] fn test_cell_type_from_data_before_classify_returns_none() { let dt = build_delaunay2_with_data(&[([0.0, 0.0], 0), ([1.0, 0.0], 0), ([0.5, 1.0], 1)]) @@ -4160,6 +4270,28 @@ mod tests { ); } + #[test] + fn test_validate_cell_classification_rejects_same_slice_triangle() { + let dt = build_delaunay2_with_data(&[([0.0, 0.0], 0), ([1.0, 0.0], 0), ([0.5, 1.0], 0)]) + .expect("Should build same-slice triangle"); + let backend = DelaunayBackend2D::from_triangulation(dt); + let mut tri = CdtTriangulation::from_labeled_delaunay(backend, 1, 2) + .expect("single-slice labels should build foliation"); + + let validation = tri.validate_cell_classification(); + assert!(matches!( + validation, + Err(CdtError::ValidationFailed { ref check, .. }) + if check == "cell_classification" + )); + let classification = tri.classify_all_cells(); + assert!(matches!( + classification, + Err(CdtError::ValidationFailed { ref check, .. }) + if check == "cell_classification" + )); + } + #[test] fn test_vertices_at_time_with_foliation() { let mut tri = CdtTriangulation::from_seeded_points(15, 3, 2, 42) @@ -4199,8 +4331,8 @@ mod tests { #[test] fn test_assign_foliation_by_y_reassignment() { - let mut tri = CdtTriangulation::from_seeded_points(15, 3, 2, 42) - .expect("Failed to create deterministic triangulation"); + let mut tri = + CdtTriangulation::from_cdt_strip(5, 3).expect("Failed to create deterministic strip"); // First assignment: 3 slices tri.assign_foliation_by_y(3) diff --git a/tests/proptest_foliation.rs b/tests/proptest_foliation.rs index 352cd80..ce4a30c 100644 --- a/tests/proptest_foliation.rs +++ b/tests/proptest_foliation.rs @@ -1,27 +1,22 @@ #![forbid(unsafe_code)] //! Property-based tests for CDT foliation construction and validation. -use causal_triangulations::prelude::geometry::DelaunayBackend2D; use causal_triangulations::prelude::triangulation::*; use proptest::prelude::*; -fn assert_cdt_strip_known_failure(result: CdtResult>) { - match result { - Err(CdtError::UnsupportedOperation { operation, reason }) => { - assert!( - operation == "CdtTriangulation::from_cdt_strip" - && reason.contains("not yet implemented"), - "Expected explicit strip-construction placeholder failure, got operation={operation:?}, reason={reason}" - ); - } - Ok(_) => panic!("Expected explicit strip-construction rejection"), - Err(other) => panic!("Expected UnsupportedOperation, got {other:?}"), - } -} - #[test] -fn cdt_strip_known_limitation_regression_guard() { - assert_cdt_strip_known_failure(CdtTriangulation::from_cdt_strip(5, 3)); +fn cdt_strip_builds_explicit_mesh() { + let tri = CdtTriangulation::from_cdt_strip(5, 3).expect("explicit CDT strip should build"); + assert_eq!(tri.vertex_count(), 15); + assert_eq!(tri.face_count(), 16); + tri.validate_topology() + .expect("explicit CDT strip topology should validate"); + tri.validate_foliation() + .expect("explicit CDT strip foliation should validate"); + tri.validate_causality_delaunay() + .expect("explicit CDT strip causality should validate"); + tri.validate_cell_classification() + .expect("explicit CDT strip cells should classify"); } proptest! { @@ -90,27 +85,24 @@ proptest! { /// Property: Explicit CDT strip construction always produces valid foliation and causality. /// - /// For any valid (vertices_per_slice, num_slices, seed): + /// For any valid (vertices_per_slice, num_slices): /// - vertex count == vertices_per_slice × num_slices /// - every slice has exactly vertices_per_slice vertices /// - foliation and causality validation both pass /// - /// TODO(#57): Re-enable this as an active invariant when explicit CDT strip - /// construction is available (blocked on delaunay/293). #[test] - #[ignore = "TODO(#57): blocked on delaunay/293 explicit strip construction"] fn cdt_strip_invariants( vertices_per_slice in 4u32..10, num_slices in 2u32..6, - seed in 0u64..1000 ) { - let _ = seed; // deterministic seed reserved for future explicit-strip implementation let tri = CdtTriangulation::from_cdt_strip(vertices_per_slice, num_slices) - .expect("TODO(#57): expected to pass once explicit strip construction lands"); + .expect("valid explicit strip construction should pass"); // Vertex count must match grid let expected_v = vertices_per_slice as usize * num_slices as usize; prop_assert_eq!(tri.vertex_count(), expected_v, "Vertex count should match grid"); + let expected_f = 2 * (vertices_per_slice as usize - 1) * (num_slices as usize - 1); + prop_assert_eq!(tri.face_count(), expected_f, "Face count should match split quads"); // Must have foliation prop_assert!(tri.has_foliation(), "CDT strip must have foliation"); @@ -128,38 +120,36 @@ proptest! { // Causality passes (no edges spanning >1 slice) prop_assert!(tri.validate_causality_delaunay().is_ok(), - "Causality should hold for foliated cylinder with {} vertices/slice, {} slices, seed {}", - vertices_per_slice, num_slices, seed); + "Causality should hold for explicit CDT strip with {} vertices/slice, {} slices", + vertices_per_slice, num_slices); + prop_assert!(tri.validate_cell_classification().is_ok(), + "Every explicit strip face should classify as Up or Down"); } /// Property: Every edge in an explicit CDT strip is classifiable and /// spacelike + timelike == total edges. /// - /// TODO(#57): Re-enable this as an active invariant when explicit CDT strip - /// construction is available (blocked on delaunay/293). #[test] - #[ignore = "TODO(#57): blocked on delaunay/293 explicit strip construction"] fn cdt_strip_edge_classification_complete( vertices_per_slice in 4u32..8, num_slices in 2u32..5, - seed in 0u64..500 ) { - let _ = seed; // deterministic seed reserved for future explicit-strip implementation let tri = CdtTriangulation::from_cdt_strip(vertices_per_slice, num_slices) - .expect("TODO(#57): expected to pass once explicit strip construction lands"); + .expect("valid explicit strip construction should pass"); let mut spacelike = 0usize; let mut timelike = 0usize; for edge in tri.geometry().edges() { - let et = tri.edge_type(&edge); - prop_assert!(et.is_some(), "Every edge should be classifiable"); - match et.unwrap() { - EdgeType::Spacelike => spacelike += 1, - EdgeType::Timelike => timelike += 1, - EdgeType::Acausal => { + match tri.edge_type(&edge) { + Some(EdgeType::Spacelike) => spacelike += 1, + Some(EdgeType::Timelike) => timelike += 1, + Some(EdgeType::Acausal) => { prop_assert!(false, "CDT strip should not have acausal edges"); } + None => { + prop_assert!(false, "Every edge should be classifiable"); + } } } @@ -171,20 +161,15 @@ proptest! { /// Property: Explicit CDT strip construction is deterministic for fixed inputs. /// - /// TODO(#57): Re-enable this as an active invariant when explicit CDT strip - /// construction is available (blocked on delaunay/293). #[test] - #[ignore = "TODO(#57): blocked on delaunay/293 explicit strip construction"] fn cdt_strip_determinism( vertices_per_slice in 4u32..8, num_slices in 2u32..5, - seed in 0u64..500 ) { - let _ = seed; // deterministic seed reserved for future explicit-strip implementation let t1 = CdtTriangulation::from_cdt_strip(vertices_per_slice, num_slices) - .expect("TODO(#57): expected to pass once explicit strip construction lands"); + .expect("valid explicit strip construction should pass"); let t2 = CdtTriangulation::from_cdt_strip(vertices_per_slice, num_slices) - .expect("TODO(#57): expected to pass once explicit strip construction lands"); + .expect("valid explicit strip construction should pass"); prop_assert_eq!(t1.vertex_count(), t2.vertex_count()); prop_assert_eq!(t1.edge_count(), t2.edge_count()); From 7040ff914c11d9e9d20be7ba904e79136bd5dba0 Mon Sep 17 00:00:00 2001 From: Adam Getchell Date: Tue, 5 May 2026 08:06:31 -0700 Subject: [PATCH 2/3] fix(cdt): validate stored foliation after backend mutation - Keep live foliation queries and cell classification active when mutable backend access invalidates sync bookkeeping without removing the foliation. - Clarify periodic and open-boundary CDT foliation docs, including `from_toroidal_cdt()` and `from_cdt_strip()` behavior. --- docs/foliation.md | 4 ++-- docs/project.md | 2 +- src/cdt/triangulation.rs | 49 ++++++++++++++++++++++++++++++++++------ 3 files changed, 45 insertions(+), 10 deletions(-) diff --git a/docs/foliation.md b/docs/foliation.md index 7161d07..0916d3a 100644 --- a/docs/foliation.md +++ b/docs/foliation.md @@ -6,14 +6,14 @@ Per-vertex time labels, edge classification, and causal validation for 1+1 CDT. In Causal Dynamical Triangulations (Ambjørn, Jurkiewicz, Loll 2001), spacetime is built from simplices arranged in a **foliation** — a layered structure where each time slice is a spatial manifold and adjacent slices are connected by timelike edges. -For 1+1 CDT: +For the periodic 1+1 CDT cases: - **Spatial topology**: S¹ (circle) — each time slice is a ring of spacelike edges - **Time direction**: [0, T] (cylinder) or S¹ (torus, periodic time) - **Edge classification**: spacelike (both endpoints at same t) or timelike (endpoints at t and t±1) - **Causality constraint**: no edge may span more than one time slice (|Δt| ≤ 1) -This implementation supports explicit open-boundary strips and explicit toroidal meshes. `from_cdt_strip()` builds an open spatial interval over discrete time, while `from_toroidal_cdt()` builds S¹ × S¹ with periodic space and time. +This implementation also supports open-boundary strip variants. `from_toroidal_cdt()` builds the periodic S¹ × S¹ toroidal case, while `from_cdt_strip()` builds open spatial-interval strip geometries over discrete time. Both constructor families use the same edge classification and causality constraint, but their topology metadata and boundary expectations differ. ## Architecture diff --git a/docs/project.md b/docs/project.md index d683de3..905624a 100644 --- a/docs/project.md +++ b/docs/project.md @@ -57,7 +57,7 @@ Assigns each vertex to a discrete time slice, enabling classification of edges a - `from_toroidal_cdt(vertices_per_slice, num_slices)` — explicit S¹×S¹ toroidal CDT (χ = 0); requires `vertices_per_slice ≥ 3` and `num_slices ≥ 3` - `assign_foliation_by_y(num_slices)` — bin existing vertices into time slices - Query methods: `time_label`, `edge_type`, `vertices_at_time`, `slice_sizes`, `has_foliation` -- Validation: `validate_topology()` (χ expectation depends on `CdtTopology`), `validate_foliation()` (structural; closed S¹ spacelike rings on toroidal), `validate_causality()` (no edge spans >1 slice) +- Validation: `validate_topology()` (χ expectation depends on `CdtTopology`), `validate_foliation()` (structural; closed S¹ spacelike rings on toroidal), `validate_causality()` (no edge spans >1 slice), `validate_cell_classification()` (strict Up/Down cell classification and validation pass) ### `config.rs` — `CdtTopology` enum diff --git a/src/cdt/triangulation.rs b/src/cdt/triangulation.rs index d97f16b..434cd73 100644 --- a/src/cdt/triangulation.rs +++ b/src/cdt/triangulation.rs @@ -1840,7 +1840,7 @@ impl CdtTriangulation { /// ``` #[must_use] pub fn time_label(&self, vertex: &DelaunayVertexHandle) -> Option { - self.foliation()?; + self.foliation.as_ref()?; self.geometry.vertex_data_by_key(vertex.vertex_key()) } @@ -1857,7 +1857,7 @@ impl CdtTriangulation { /// ``` #[must_use] pub fn vertices_at_time(&self, t: u32) -> Vec { - if !self.has_foliation() { + if self.foliation.is_none() { return vec![]; } self.geometry @@ -1988,7 +1988,7 @@ impl CdtTriangulation { /// ``` #[must_use] pub fn edge_type(&self, edge: &DelaunayEdgeHandle) -> Option { - self.foliation()?; + self.foliation.as_ref()?; let (v0, v1) = self.geometry.edge_endpoints(edge)?; let t0 = self.geometry.vertex_data_by_key(v0.vertex_key())?; @@ -2024,7 +2024,7 @@ impl CdtTriangulation { /// ``` #[must_use] pub fn cell_type(&self, face: &DelaunayFaceHandle) -> Option { - self.foliation()?; + self.foliation.as_ref()?; let verts = self.geometry.face_vertices(face).ok()?; if verts.len() != 3 { return None; @@ -2101,7 +2101,7 @@ impl CdtTriangulation { /// ``` #[must_use] pub fn face_edge_types(&self, face: &DelaunayFaceHandle) -> Option<[EdgeType; 3]> { - self.foliation()?; + self.foliation.as_ref()?; let verts = self.geometry.face_vertices(face).ok()?; if verts.len() != 3 { @@ -2149,7 +2149,7 @@ impl CdtTriangulation { /// .expect("all strip cells classify"); /// ``` pub fn validate_cell_classification(&self) -> CdtResult<()> { - if !self.has_foliation() { + if self.foliation.is_none() { return Ok(()); } @@ -2198,7 +2198,7 @@ impl CdtTriangulation { /// assert!(classified > 0); /// ``` pub fn classify_all_cells(&mut self) -> CdtResult> { - if !self.has_foliation() { + if self.foliation.is_none() { return Ok(None); } @@ -4179,6 +4179,41 @@ mod tests { .expect("missing foliation should validate vacuously"); } + #[test] + fn test_validate_and_classify_use_stored_foliation_after_mutable_access() { + let mut tri = CdtTriangulation::from_cdt_strip(4, 2).expect("Should build CDT strip"); + let vertex = tri + .geometry() + .vertices() + .next() + .expect("CDT strip should contain vertices") + .vertex_key(); + let label = tri + .geometry() + .vertex_data_by_key(vertex) + .expect("CDT strip vertices should be labeled"); + + { + let mut geometry_mut = tri.geometry_mut(); + let _previous = geometry_mut + .set_vertex_data_by_key(vertex, Some(label)) + .expect("Expected valid vertex key while preserving label"); + } + + assert!( + !tri.has_foliation(), + "mutable backend access should invalidate synchronized foliation bookkeeping" + ); + tri.validate_cell_classification() + .expect("stored foliation should still drive live cell validation"); + + let classified = tri + .classify_all_cells() + .expect("stored foliation should still drive live cell classification") + .expect("foliation is still present"); + assert_eq!(classified, tri.face_count()); + } + #[test] fn test_cell_type_from_data_before_classify_returns_none() { let dt = build_delaunay2_with_data(&[([0.0, 0.0], 0), ([1.0, 0.0], 0), ([0.5, 1.0], 1)]) From cdbf3632412bf3e3939be426a10eeb6640b6f248 Mon Sep 17 00:00:00 2001 From: Adam Getchell Date: Tue, 5 May 2026 15:09:05 -0700 Subject: [PATCH 3/3] fix(cdt): harden from_cdt_strip generation - Reserve explicit strip construction buffers fallibly and report CDT generation errors with strip context when allocation fails. - Keep strip triangles in fixed-size cells until the backend boundary and reject generated backends whose vertex or face counts differ from the requested strip. --- src/cdt/triangulation.rs | 175 +++++++++++++++++++++++++++++++++------ 1 file changed, 148 insertions(+), 27 deletions(-) diff --git a/src/cdt/triangulation.rs b/src/cdt/triangulation.rs index 434cd73..ef5d1a4 100644 --- a/src/cdt/triangulation.rs +++ b/src/cdt/triangulation.rs @@ -114,6 +114,65 @@ fn remap_strip_generation_error( } } +/// Builds a CDT-level generation error for explicit strip construction failures. +const fn strip_generation_error( + total_vertices: u32, + coordinate_max: f64, + underlying_error: String, +) -> CdtError { + CdtError::DelaunayGenerationFailed { + vertex_count: total_vertices, + coordinate_range: (0.0, coordinate_max), + attempt: 1, + underlying_error, + } +} + +/// Verifies that the explicit strip builder returned the requested mesh size. +#[expect( + clippy::too_many_arguments, + reason = "count mismatch diagnostics preserve both requested CDT parameters and expected builder counts" +)] +fn validate_strip_counts( + backend: &DelaunayBackend2D, + total_vertices: u32, + total_cells: u32, + expected_vertices: usize, + expected_faces: usize, + vertices_per_slice: u32, + num_slices: u32, + coordinate_max: f64, +) -> CdtResult<()> { + if backend.vertex_count() != expected_vertices { + return Err(strip_generation_error( + total_vertices, + coordinate_max, + format!( + "build_delaunay2_from_cells()/from_cdt_strip() produced {} vertices, expected {} for vertices_per_slice={} and num_slices={}", + backend.vertex_count(), + total_vertices, + vertices_per_slice, + num_slices, + ), + )); + } + if backend.face_count() != expected_faces { + return Err(strip_generation_error( + total_vertices, + coordinate_max, + format!( + "build_delaunay2_from_cells()/from_cdt_strip() produced {} faces, expected {} for vertices_per_slice={} and num_slices={}", + backend.face_count(), + total_cells, + vertices_per_slice, + num_slices, + ), + )); + } + + Ok(()) +} + /// Events in simulation history #[derive(Debug, Clone)] pub enum SimulationEvent { @@ -1319,9 +1378,12 @@ impl CdtTriangulation { /// /// Returns [`CdtError::InvalidGenerationParameters`] if `vertices_per_slice < 4`, /// `num_slices < 2`, or the derived vertex or cell count overflows `u32`. - /// Returns [`CdtError::DelaunayGenerationFailed`] if the underlying - /// explicit builder rejects the mesh, and [`CdtError::ValidationFailed`] if - /// the constructed strip fails CDT validation. + /// Returns [`CdtError::DelaunayGenerationFailed`] if constructor storage cannot + /// be reserved, if the underlying explicit builder rejects the mesh, or if + /// `build_delaunay2_from_cells()` returns a vertex or face count that does not + /// match the requested strip. Returns [`CdtError::Foliation`], + /// [`CdtError::CausalityViolation`], or [`CdtError::ValidationFailed`] if the + /// constructed strip fails CDT validation. /// /// # Examples /// @@ -1334,6 +1396,10 @@ impl CdtTriangulation { /// assert_eq!(tri.face_count(), 6); /// assert!(tri.validate_cell_classification().is_ok()); /// ``` + #[expect( + clippy::too_many_lines, + reason = "explicit strip construction includes fallible allocation handling and post-build validation" + )] pub fn from_cdt_strip(vertices_per_slice: u32, num_slices: u32) -> CdtResult { if vertices_per_slice < 4 { return Err(CdtError::InvalidGenerationParameters { @@ -1376,49 +1442,85 @@ impl CdtTriangulation { expected_range: "product ≤ u32::MAX".to_string(), })?; - let n = vertices_per_slice as usize; - let t_count = num_slices as usize; + let coordinate_max = f64::from(num_slices - 1).max(1.0); + let generation_failed = |underlying_error: String| { + strip_generation_error(total_vertices, coordinate_max, underlying_error) + }; + + let expected_vertices = + usize::try_from(total_vertices).map_err(|err| generation_failed(err.to_string()))?; + let expected_faces = + usize::try_from(total_cells).map_err(|err| generation_failed(err.to_string()))?; + + let n = usize::try_from(vertices_per_slice) + .map_err(|err| generation_failed(err.to_string()))?; + let t_count = + usize::try_from(num_slices).map_err(|err| generation_failed(err.to_string()))?; let index = |i: usize, t: usize| -> usize { t * n + i }; let spacing = 1.0_f64 / f64::from(vertices_per_slice - 1); - let mut vertex_specs: Vec<([f64; 2], u32)> = Vec::with_capacity(total_vertices as usize); + let mut vertex_specs: Vec<([f64; 2], u32)> = Vec::new(); + vertex_specs + .try_reserve_exact(expected_vertices) + .map_err(|err| { + generation_failed(format!( + "from_cdt_strip() failed to reserve {expected_vertices} vertex specs for vertices_per_slice={vertices_per_slice}, num_slices={num_slices}: {err}" + )) + })?; for t in 0..num_slices { for i in 0..vertices_per_slice { vertex_specs.push(([f64::from(i) * spacing, f64::from(t)], t)); } } - let mut cells: Vec> = Vec::with_capacity(total_cells as usize); + let mut cells: Vec<[usize; 3]> = Vec::new(); + cells.try_reserve_exact(expected_faces).map_err(|err| { + generation_failed(format!( + "from_cdt_strip() failed to reserve {expected_faces} triangle cells for vertices_per_slice={vertices_per_slice}, num_slices={num_slices}: {err}" + )) + })?; for t in 0..(t_count - 1) { let t_next = t + 1; for i in 0..(n - 1) { let i_next = i + 1; - cells.push(vec![index(i, t), index(i_next, t), index(i, t_next)]); - cells.push(vec![ - index(i_next, t), - index(i_next, t_next), - index(i, t_next), - ]); + cells.push([index(i, t), index(i_next, t), index(i, t_next)]); + cells.push([index(i_next, t), index(i_next, t_next), index(i, t_next)]); } } - let coordinate_max = f64::from(num_slices - 1).max(1.0); - let dt = build_delaunay2_from_cells(&vertex_specs, &cells) + // delaunay 0.7.6 accepts explicit cells as Vec-backed index lists. + // Keep the strip working set compact, then adapt fallibly at the API boundary. + let mut cell_specs: Vec> = Vec::new(); + cell_specs.try_reserve_exact(expected_faces).map_err(|err| { + generation_failed(format!( + "from_cdt_strip() failed to reserve {expected_faces} builder cell specs for build_delaunay2_from_cells(): {err}" + )) + })?; + for cell in &cells { + let mut cell_spec = Vec::new(); + cell_spec.try_reserve_exact(3).map_err(|err| { + generation_failed(format!( + "from_cdt_strip() failed to reserve a build_delaunay2_from_cells() triangle cell spec: {err}" + )) + })?; + cell_spec.extend_from_slice(cell); + cell_specs.push(cell_spec); + } + + let dt = build_delaunay2_from_cells(&vertex_specs, &cell_specs) .map_err(|err| remap_strip_generation_error(err, total_vertices, coordinate_max))?; let backend = DelaunayBackend2D::from_triangulation(dt); - if backend.vertex_count() != total_vertices as usize { - return Err(CdtError::DelaunayGenerationFailed { - vertex_count: total_vertices, - coordinate_range: (0.0, coordinate_max), - attempt: 1, - underlying_error: format!( - "explicit strip builder produced {} vertices, expected {}", - backend.vertex_count(), - total_vertices, - ), - }); - } + validate_strip_counts( + &backend, + total_vertices, + total_cells, + expected_vertices, + expected_faces, + vertices_per_slice, + num_slices, + coordinate_max, + )?; let slice_sizes = vec![n; t_count]; let foliation = @@ -3897,6 +3999,25 @@ mod tests { assert!(tri.validate_cell_classification().is_ok()); } + #[test] + fn test_explicit_strip_count_validation_rejects_face_mismatch() { + let tri = CdtTriangulation::from_cdt_strip(4, 2).expect("explicit strip should build"); + let result = validate_strip_counts(tri.geometry(), 8, 7, 8, 7, 4, 2, 1.0); + + assert!(matches!( + result, + Err(CdtError::DelaunayGenerationFailed { + vertex_count: 8, + coordinate_range: (0.0, 1.0), + attempt: 1, + ref underlying_error, + }) if underlying_error.contains("build_delaunay2_from_cells()/from_cdt_strip()") + && underlying_error.contains("produced 6 faces, expected 7") + && underlying_error.contains("vertices_per_slice=4") + && underlying_error.contains("num_slices=2") + )); + } + #[test] fn test_vertices_at_time() { let tri = assert_valid_foliated_cylinder(