Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions benches/PERFORMANCE_RESULTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -179,11 +179,11 @@ insphere query performance independently from full triangulation workflows.

## Implementation Notes

### Performance Advantages of `insphere_lifted`
### Dimension-Dependent InSphere Predicate Performance

1. More efficient matrix formulation using relative coordinates
2. Avoids redundant circumcenter calculations
3. Optimized determinant computation
The tables above are the source of truth for predicate timing. `insphere_lifted`
shows advantages in lower dimensions such as 2D/3D, while `insphere_distance`
often wins in 4D/5D; boundary cases may favor `insphere` because of early exits.

## Benchmark Structure

Expand Down
2 changes: 1 addition & 1 deletion benches/ci_performance_suite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -592,7 +592,7 @@ fn interior_facets_4d(dt: &FlipTriangulation4) -> Vec<FacetHandle> {
let mut facets = Vec::new();
for (cell_key, cell) in dt.cells() {
if let Some(neighbors) = cell.neighbors() {
for (facet_index, neighbor) in neighbors.iter().enumerate() {
for (facet_index, neighbor) in neighbors.enumerate() {
if neighbor.is_some() {
let Ok(facet_index) = u8::try_from(facet_index) else {
continue;
Expand Down
2 changes: 1 addition & 1 deletion examples/delaunayize_repair.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ fn flip_then_repair_2d() -> Result<(), DelaunayizeRepairExampleError> {
let mut facets: Vec<_> = Vec::new();
for (ck, cell) in dt.cells() {
if let Some(neighbors) = cell.neighbors() {
for (i, n) in neighbors.iter().enumerate() {
for (i, n) in neighbors.enumerate() {
if let (Some(_), Ok(idx)) = (n, u8::try_from(i)) {
facets.push(FacetHandle::new(ck, idx));
}
Expand Down
2 changes: 1 addition & 1 deletion examples/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ fn build_non_delaunay_triangulation_2d()

for (cell_key, cell) in dt.cells() {
if let Some(neighbors) = cell.neighbors() {
for (facet_index, neighbor) in neighbors.iter().enumerate() {
for (facet_index, neighbor) in neighbors.enumerate() {
if neighbor.is_none() {
continue;
}
Expand Down
2 changes: 1 addition & 1 deletion examples/pachner_roundtrip_4d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ fn collect_interior_facets(dt: &Dt4) -> Vec<FacetHandle> {
let mut facets = Vec::new();
for (cell_key, cell) in dt.cells() {
if let Some(neighbors) = cell.neighbors() {
for (facet_index, neighbor) in neighbors.iter().enumerate() {
for (facet_index, neighbor) in neighbors.enumerate() {
if neighbor.is_some() {
let Ok(facet_index) = u8::try_from(facet_index) else {
continue;
Expand Down
4 changes: 2 additions & 2 deletions examples/topology_editing_2d_3d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -560,7 +560,7 @@ fn find_interior_facet_2d<K: Kernel<2>>(
) -> Option<FacetHandle> {
for (cell_key, cell) in dt.cells() {
if let Some(neighbors) = cell.neighbors() {
for (facet_idx, neighbor) in neighbors.iter().enumerate() {
for (facet_idx, neighbor) in neighbors.enumerate() {
if neighbor.is_some() {
let Ok(facet_idx) = u8::try_from(facet_idx) else {
continue;
Expand All @@ -578,7 +578,7 @@ fn find_interior_facet_3d<K: Kernel<3>>(
) -> Option<FacetHandle> {
for (cell_key, cell) in dt.cells() {
if let Some(neighbors) = cell.neighbors() {
for (facet_idx, neighbor) in neighbors.iter().enumerate() {
for (facet_idx, neighbor) in neighbors.enumerate() {
if neighbor.is_some() {
let Ok(facet_idx) = u8::try_from(facet_idx) else {
continue;
Expand Down
77 changes: 60 additions & 17 deletions scripts/benchmark_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ def _load_ci_performance_manifest_ids(criterion_dir: Path) -> set[str] | None:
return manifest_ids or None


def _load_ci_performance_metrics(criterion_dir: Path) -> dict[str, dict[str, int]]:
def _load_ci_performance_metrics(criterion_dir: Path) -> dict[str, Mapping[str, object]]:
"""Load ci_performance_suite construction metrics when present."""
metrics_path = _ci_performance_metrics_path(criterion_dir)
if not metrics_path.exists():
Expand All @@ -349,17 +349,12 @@ def _load_ci_performance_metrics(criterion_dir: Path) -> dict[str, dict[str, int
msg = f"malformed ci_performance_suite metrics sidecar {metrics_path}: expected JSON object"
raise TypeError(msg)

metrics: dict[str, dict[str, int]] = {}
metrics: dict[str, Mapping[str, object]] = {}
for benchmark_id, values in data.items():
if not isinstance(benchmark_id, str) or not isinstance(values, dict):
msg = f"malformed ci_performance_suite metrics sidecar {metrics_path}: invalid entry {benchmark_id!r}"
raise TypeError(msg)
vertices = values.get("vertices")
simplices = values.get("simplices")
if not isinstance(vertices, int) or not isinstance(simplices, int):
msg = f"malformed ci_performance_suite metrics sidecar {metrics_path}: invalid counts for {benchmark_id!r}"
raise TypeError(msg)
metrics[benchmark_id] = {"vertices": vertices, "simplices": simplices}
logger.debug("Skipping malformed ci_performance_suite metric entry %r from %s", benchmark_id, metrics_path)
continue
metrics[benchmark_id] = values
return metrics


Expand Down Expand Up @@ -1850,11 +1845,11 @@ def _get_implementation_notes() -> list[str]:
return [
"## Implementation Notes",
"",
"### Performance Advantages of `insphere_lifted`",
"### Dimension-Dependent InSphere Predicate Performance",
"",
"1. More efficient matrix formulation using relative coordinates",
"2. Avoids redundant circumcenter calculations",
"3. Optimized determinant computation",
"The tables above are the source of truth for predicate timing. `insphere_lifted`",
"shows advantages in lower dimensions such as 2D/3D, while `insphere_distance`",
"often wins in 4D/5D; boundary cases may favor `insphere` because of early exits.",
"",
]

Expand Down Expand Up @@ -1980,6 +1975,48 @@ def _ci_suite_input_points(path_parts: tuple[str, ...]) -> int | None:
return int(path_parts[-1])
return None

@staticmethod
def _ci_suite_metric_simplices(
metric: Mapping[str, object] | None,
*,
benchmark_id: str,
path_parts: tuple[str, ...],
points: int | None,
dimension: str,
) -> int | None:
"""Return sidecar simplex counts only when they match the Criterion result."""
if metric is None:
return None

expected_dimension = ci_suite_dimension(benchmark_id)
expected_points = CriterionParser._ci_suite_input_points(path_parts)
if expected_dimension != dimension or expected_points != points:
logger.debug("Skipping stale ci_performance_suite metric for %s", benchmark_id)
return None

vertices = metric.get("vertices")
simplices = metric.get("simplices")
if (
not isinstance(vertices, int)
or isinstance(vertices, bool)
or not isinstance(simplices, int)
or isinstance(simplices, bool)
or simplices < 0
):
logger.debug("Skipping malformed ci_performance_suite metric for %s", benchmark_id)
return None

if points is None or vertices != points:
logger.debug(
"Skipping stale ci_performance_suite metric for %s: vertices=%s, Criterion input=%s",
benchmark_id,
vertices,
points,
)
return None

return simplices

@staticmethod
def _process_ci_performance_suite_results(criterion_dir: Path) -> list[BenchmarkData]:
"""Discover ci_performance_suite Criterion results with expanded benchmark IDs."""
Expand All @@ -1997,9 +2034,15 @@ def _process_ci_performance_suite_results(criterion_dir: Path) -> list[Benchmark
continue

benchmark_data.benchmark_id = benchmark_id
metric = metrics.get(benchmark_id)
if metric is not None:
benchmark_data.simplices = metric["simplices"]
metric_simplices = CriterionParser._ci_suite_metric_simplices(
metrics.get(benchmark_id),
benchmark_id=benchmark_id,
path_parts=path_parts,
points=points,
dimension=dimension,
)
if metric_simplices is not None:
benchmark_data.simplices = metric_simplices
results.append(benchmark_data)

group_order = {group: index for index, group in enumerate(CI_PERFORMANCE_SUITE_GROUP_ORDER)}
Expand Down
44 changes: 43 additions & 1 deletion scripts/tests/test_benchmark_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,47 @@ def test_find_criterion_results_preserves_ci_suite_ids(self) -> None:
assert roundtrip.dimension == "4D"
assert roundtrip.throughput_mean is None

def test_find_criterion_results_skips_stale_ci_suite_metrics(self) -> None:
"""Test ci_performance_suite simplex counts require matching current inputs."""
with tempfile.TemporaryDirectory() as temp_dir:
target_dir = Path(temp_dir) / "target"
benchmark_id = "tds_new_2d/tds_new/10"
write_estimate(target_dir, tuple(benchmark_id.split("/")), 10_000.0)
write_ci_performance_metrics(
target_dir,
{
benchmark_id: {
"vertices": 25,
"simplices": 999,
},
},
)

results = CriterionParser.find_criterion_results(target_dir)

assert len(results) == 1
assert results[0].comparison_key == benchmark_id
assert results[0].points == 10
assert results[0].simplices is None

def test_find_criterion_results_skips_malformed_ci_suite_metrics(self) -> None:
"""Test malformed ci_performance_suite simplex counts are not applied."""
with tempfile.TemporaryDirectory() as temp_dir:
target_dir = Path(temp_dir) / "target"
benchmark_id = "tds_new_2d/tds_new/10"
write_estimate(target_dir, tuple(benchmark_id.split("/")), 10_000.0)
metrics_path = target_dir / "criterion" / _CI_PERFORMANCE_SUITE_METRICS_FILE
metrics_path.write_text(
json.dumps({benchmark_id: {"vertices": 10, "simplices": "stale"}}),
encoding="utf-8",
)

results = CriterionParser.find_criterion_results(target_dir)

assert len(results) == 1
assert results[0].comparison_key == benchmark_id
assert results[0].simplices is None

def test_find_criterion_results_filters_stale_ci_suite_ids_with_manifest(self) -> None:
"""Test ci_performance_suite parsing ignores stale Criterion files outside the manifest."""
with tempfile.TemporaryDirectory() as temp_dir:
Expand Down Expand Up @@ -3341,7 +3382,8 @@ def test_get_implementation_notes(self) -> None:
content = "\n".join(lines)

assert "## Implementation Notes" in content
assert "### Performance Advantages of `insphere_lifted`" in content
assert "### Dimension-Dependent InSphere Predicate Performance" in content
assert "`insphere_distance`" in content
assert "### Method Disagreements" not in content

def test_empty_benchmark_results_edge_case(self) -> None:
Expand Down
Loading
Loading