Smooth by color class instead of under ring locks

[danielepanozzo]

Rebased onto #1084, which is now merged into main. The stack is main ← #1079 ← #1080 ← #1081, to be merged in that order. The measurements in the original description below predate the rebase (against main 54ed1a2); #1081 has the current numbers for the whole stack against #1084.

Review fixes (2026-10-08)

An independent review of the stack found one real problem in this PR, and some gaps. All are fixed in new commits:

• topological_offset's smoothing hooks wrote neighbors back. Its front and plastic smoothers, and its invariants(), read neighboring vertices through non-const attribute access. Under colored smoothing, a rejected smooth wrote those neighbors back while another thread of the same class could be doing the same: a data race, and a possible double free of a vertex's on_bbox_faces. It only happened when topological_offset ran with num_threads > 0. Those reads now go through const access.

• A check that makes this class of bug fail at once. In a colored pass, each smooth checks that everything it reached through non-const access is its own vertex, or an edge, face or cell touching it. If a hook reaches further, the pass throws immediately, at any thread count.

• colored_smoothing is now a JSON option in tetwild, triwild, simwild and topological_offset. It defaults to true; false brings back the locked pass.

• New tests:

  • thread-count determinism with a real surface, an envelope and unrounded vertices, in 3D and 2D;
  • the check firing on a hook that touches a neighbor;
  • unit tests for the coloring and the class runner.

• Cleanups: an early-out when there is nothing to smooth, no barrier after the last class, an assert against duplicate vertices, and comments on how the barrier waits.

• Build fix for Homebrew Macs. On such a Mac, FindGMP took GMP's include directory from the shared /opt/homebrew/include. That put every Homebrew-installed library ahead of the copies the build fetches. With Homebrew's Abseil installed, topological_offset and manifold_extraction compiled against its headers but linked ipc-toolkit's fetched Abseil. The link then failed on an undefined absl::lts_20260817::hash_internal::MixingHashState::kSeed. FindGMP now looks in GMP's own Homebrew prefix first, and drops a stale cached value, so existing build directories pick the fix up.

• topological_offset tested in parallel. With the build fix, its 42 unit tests and manifold_extraction's tests pass. The five topological_offset integration configs run cleanly at 4 and 8 threads, on this PR alone and on the whole stack, with no error from the new check:

  • 2D: square, circle, triangle;
  • 3D: double sphere and convex.

  The two 3D configs in the data repository use keys the spec no longer accepts (offset_gradient_rel, offset_residual_rel, relative_ball_threshold), so they fail on any build. For these runs I dropped those keys and capped them at 3 turns.

• The check costs nothing measurable. Smoothing time per operation with the check on, against off: 0.99× geomean over 4 tetwild and 2 triwild models, 16 threads, mirrored order.

---

Stack: #1079 (colored smoothing) ← #1080 (screening) ← #1081 (persistent worker pool). Merge in that order.

First of three stacked PRs that speed up the parallel optimization passes. This one replaces the ring locks of the parallel smoothing pass with a vertex coloring. #1080 (screening) builds on it, and #1081 (persistent worker pool) on that.

What changes

A parallel smoothing pass splits its vertices into classes of pairwise non-adjacent vertices and smooths the classes one after another. Each class runs fully in parallel with no locks: two vertices of a class share no edge and no cell, so smoothing one neither moves a vertex the other reads nor writes a cell attribute the other writes.

• Deterministic. The coloring is greedy and serial in vertex order, so the classes, and with them the result, do not depend on the number of threads. New tetwild and triwild tests check it bit for bit at 1, 3, 8 and 16 threads.
• Const neighbour reads in smooth_vertex_3d. A non-const AttributeCollection::operator[] inside the protect window snapshots the vertex it touches, and a rolled-back smooth writes every snapshot back. Here that means two threads of the same class writing back the same neighbour, which both of them read, at the same time: before this fix, 4 of 10 colored runs aborted.
• One parallel region per pass. The classes meet at a barrier instead of each starting its own threads. task_group starts a thread per task, and a thread's thread-local Newton solver lives only as long as the thread, so the threads are started once per pass, as the locked pass started them.
• Load balance within a class. A thread takes about an eighth of its share at a time, surface vertices first. Those project onto the envelope and cost the most, so a class does not wait at its barrier for one expensive chunk.

OptimizerParameters::colored_smoothing (default on) switches it off. Serial runs keep the single queue and stay byte-identical to main.

12 files, +668/-12, of which ~255 lines are the two tests.

Measurements

16 threads, Apple M3 Max, against main 54ed1a2. The challenging models (the [challenging] integration group, run here at 16 threads instead of serially), 2 runs per model in mirrored order. Some background indexing ran during parts of the benchmark.

	main	this PR	speedup
tetwild, 14 models, total wall	685 s	634 s	1.08x (geomean 1.10x)
tetwild, smoothing passes	193 s	137 s	1.41x
triwild, 16 models, total wall	210 s	202 s	1.04x (geomean 1.02x)
triwild, smoothing passes	60 s	50 s	1.19x

The wall-time gain is modest on its own: smoothing is a third of tetwild's optimization time, and several models land on a different iteration count than main (1017020: 0.85x, 1017012: 0.87x). The pass times above are the cleaner measure. All runs reach stop energy 10, and the final max/avg energies and cell counts are within main's run-to-run spread.

Tests

wmtk_tests; the tetwild, triwild, simwild, isotropic_remeshing, qslim and shortest_edge_collapse component tests; and Integration_Tests all pass. The isotropic remeshing assertion count is identical to main's (352). Serial output is byte-identical to main (tetwild 101954, triwild 191265). ThreadSanitizer on tetwild 101955 and triwild 194286 reports nothing from the coloring code: the only reports are main's existing unlocked Tuple::is_valid checks in the queue path.

🤖 Generated with Claude Code

https://claude.ai/code/session_01BJZ1VpHeewY9QZVM2AHWNU

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

🟡 Changes recommended

Partial worker-launch failure can permanently deadlock the color-class barrier.

1 open finding

• Worker launch failure can permanently hang at the barrier · New

🧠 Review effort: Balanced

[danielepanozzo]

Fixed in "Color classes: start no task before every task is launched": tasks now wait at a start gate until every launch has succeeded. If one launch throws, the tasks already running are told to leave without touching the barrier, and the launch failure is rethrown. There's no test for this path, since making task_group::run fail would need a test-only hook.