Collapse limit at admission, stuck_refine_cooldown = 1, and no quality veto on splits - #1053
Merged
Merged
Conversation
Reverts the placement introduced in 73ddbba (PR #997) and restores TetWild's: an edge at or above the collapse target is filtered out of the candidate list by is_weight_up_to_date and never offered to collapse_edge_before. The acceptance-time block, which admitted an over-length edge whenever it strictly improved the ring's worst element, is removed. This is what both ancestors do. fTetWild filters at queue admission and re-checks on pop (EdgeCollapsing.cpp:47, :88), and TetWild 2018 does the same with isCollapsable_cd3. The constants and the sizing scaling are unchanged and already matched theirs: 4/5 for collapse, 4/3 for split, stored squared, scaled by the average of the two endpoints' m_sizing_scalar. The pre-existing collapse_quality_allowed per-cell gate is untouched -- it predates #997 (cb9b81a) and is not part of this placement question. Measured on the 31 challenging models at stop_energy 10, num_threads 1 (deterministic), 12 models in parallel, before -> after: iterations 321 -> 319 (-0.6%) elements 1841190 -> 1846503 (+0.3%) wall (sum) 90.1 -> 81.8 min (-9.2%) converged all -> all Per-model differences are noise and go both ways: 191874 costs 6 more iterations, 101168 / 104187 / 184836 / 189017 each save 2. Element counts move +-6% per model and cancel. The wall-time gain is the expected one -- a filtered candidate never reaches collapse_edge_before's ring work. KNOWN REGRESSION, accepted deliberately. triwild20k 189017 at eps_rel 1e-4 -- 10x tighter than the default, and the case #997 was written against -- diverges with this placement: faces, main 16392 ... 54613 157160 179581 179373 181913 178363 converged 9.999, 23 it, 4.8 min faces, this 16392 ... 63736 476001 1576626 ... 5805912 killed at it 19, still growing Its max energy recovers to 5.9e12 and then returns to the inverted sentinel 1e+50 for the last five iterations: the collinear element that motivated #997 goes inverted, exactly as described there. Nothing in the default-tolerance suite sees this, which is why it took a 1e-4 run to find. Both ancestors share the failure, since both filter at admission. The acceptance-time gate is not the shape we want for that divergence. Chasing it separately. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…o collapse-length-gate-at-admission * 'main' of github.com:wildmeshing/wildmeshing-toolkit: update simwild sweep script Add detailed quality stats to SimWild. Throw an error if tags do not match after a surface swap. Do not return, just set the sniffed value to 3. In SimWild, everything that is not OBJ or MSH is assumed to be 3D input. Add missing abstract function to TetOptimizerMesh. Update surface tags after every iteration. Add an `update_attributes` function that SimWild can use to update the surface according to the tet tags. Remove code from collapse_after_vertex in SimWild. Update sweep script. Add scripts for running simwild sweeps. Fix issues with vertex order update and sizing scalar.
Refining the sizing field every iteration can outrun the operations
entirely. On triwild20k 189017 at eps_rel 1e-4 -- the case the previous
commit knowingly regresses -- the field ratchets down faster than split
and collapse can act on it. The mesh holds at ~31k faces for a dozen
iterations, then runs away:
31845 53854 56388 56925 96167 99950 158564 ... 5805912
and never converges, its max energy pinned at the inverted sentinel
1e+50. At cooldown 1 the same run climbs deliberately to 181k and
converges at 9.9994 in 35 iterations -- essentially the mesh the
acceptance-time gate produced (178363).
That makes this the narrow instrument for the divergence: it slows the
refinement, where the acceptance-time gate instead licensed collapse to
undo refinement anywhere the mesh already held a bad element.
Cost over the 31 challenging models at stop_energy 10, cooldown 0 -> 1,
single-threaded so the comparison carries no threading noise:
tetwild iterations 185 -> 196 (+5.9%) elements -1.8% wall -5.3%
triwild iterations 166 -> 173 (+4.2%) elements -0.1% wall -6.2%
All 31 converge at every value swept. A few percent more iterations,
slightly fewer elements, and no wall-time cost -- the extra iterations
are cheaper ones. 1 is the knee: 2 and 3 buy a further 2-4% element
reduction in tetwild for +34% and +76% iterations.
This supersedes the earlier reading of a 468-model triwild20k sweep,
recorded in the comment this replaces, where cooldown 1 cost ~13% wall
for identical mesh sizes and was taken as evidence that a cooldown only
delays the next escape. That holds where the escape hatch is not
load-bearing; it does not hold on the models where it is.
Default changed in OptimizerParameters and in the tetwild, triwild and
simwild specs, so the three applications agree.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…n' into collapse-length-gate-at-admission
…envelope sanity check is not optional here as the tagged surface might be a different one than the input surface.
…ty_checks Remove the parameter for the envelope sanity check from SimWild. The …
Removes the guard added in a4ebcff (#997), which refused a split whose result was degenerate when the region was not degenerate before: if (max_quality_after >= MAX_ENERGY && cache.max_quality_before < MAX_ENERGY) return false; That commit introduced it as insurance rather than as a fix, and said so: "Neither guard is needed for convergence once the envelope is right, and neither changes any integration-test result." Measured now, the first half holds and the second is too generous -- where it does change a result, the guard is the thing making it worse. 31 challenging models at stop_energy 10, num_threads 1 (deterministic), guard -> no guard: identical (iterations AND element count to the unit) 29 of 31 iterations 392 -> 380 (-3.1%) elements 2274988 -> 2269955 (-0.22%) converged 31/31 -> 31/31 The two that move: triwild 189017 @ eps_rel 1e-4 35 -> 20 iterations, 180941 -> 173846 elements tetwild 104187 11 -> 14 iterations, 189033 -> 191095 elements 189017 at 1e-4 is the hardest case in the set, and it converges in 20 iterations instead of 35 for a 3.9% smaller mesh. The mechanism is the obvious one: refusing to split a stuck sliver leaves the sliver, and the run grinds around it. The guard was not preventing degeneracy either. Ten of the triwild models report the MAX_ENERGY sentinel WITH the guard in place, and the sentinel counts are identical model for model without it -- removing it did not create one additional degenerate element anywhere. It only refused the particular split that would have produced one, while degenerate elements arrived by other routes regardless. 101954, the model the guard was written against, is unchanged at 10 iterations and 28925 tets and never reports the sentinel in either build. The failure it was written for -- 4.6e16 on the split line right after "[force-split] 92 worst-tet longest edges force-split" -- does not reproduce, which is what a4ebcff predicted: the envelope fix in that same commit is what fixed it. Also removes the state that existed only to feed the guard: the max_quality_before scan over incident cells at the top of split_edge_before, and the field from both SplitCache structs. Golden hashes will move for the two models above. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three changes to the collapse/refinement loop, each restoring behaviour that a guard or a gate had taken away. All measurements are over the 31 challenging models at
stop_energy 10,num_threads 1(deterministic — no threading noise), 12 in parallel on 16 cores,46024separately at 16 threads.1. The collapse length limit applies to the candidate list, not at acceptance
Reverts the placement introduced in 73ddbba (#997). An edge at or above the collapse target is filtered out by
is_weight_up_to_dateand never offered tocollapse_edge_before; the acceptance-time block — which admitted an over-length edge whenever it strictly improved the ring's worst element — is removed.This is what both ancestors do: fTetWild filters at queue admission and re-checks on pop (
EdgeCollapsing.cpp:47,:88), TetWild 2018 the same viaisCollapsable_cd3. Constants and sizing scaling already matched theirs and are unchanged (4/5collapse,4/3split, squared, scaled by the endpoints' meanm_sizing_scalar). The pre-existingcollapse_quality_allowedper-cell gate predates #997 (cb9b81a) and is untouched.Per-model differences are noise and go both ways. The wall-time gain is the expected one: a filtered candidate never reaches
collapse_edge_before's ring work. By application, −10.2% wall on tetwild, −6.1% on triwild.2.
stuck_refine_cooldowndefaults to 1Refining the sizing field every iteration can outrun the operations. On triwild20k
189017ateps_rel 1e-4— the case change 1 regresses — the field ratchets down faster than split and collapse can act on it: the mesh holds at ~31k faces for a dozen iterations, then runs away54k → 96k → 159k → 5.8Mand never converges, max energy pinned at the inverted sentinel1e+50. At cooldown 1 the same run climbs deliberately to 181k and converges at 9.9994 in 35 iterations.Cooldown is the narrow instrument for this: it slows the refinement, where the acceptance-time gate instead licensed collapse to undo refinement anywhere the mesh already held a bad element.
All 31 converge at every value swept. 1 is the knee: 2 and 3 buy a further 2–4% element reduction in tetwild for +34% and +76% iterations. Default changed in
OptimizerParametersand in the tetwild, triwild and simwild specs so the three agree.This supersedes an earlier reading of a 468-model triwild20k sweep (recorded in the comment change 2 replaces), where cooldown 1 cost ~13% wall for identical mesh sizes and was taken as evidence that a cooldown only delays the next escape. That holds where the escape hatch is not load-bearing; it does not hold on the models where it is.
3. A split is never refused on quality
Removes the guard added in a4ebcff (#997):
That commit introduced it as insurance and said so — "Neither guard is needed for convergence once the envelope is right, and neither changes any integration-test result." The first half holds; the second is too generous, because where it does change a result the guard is what makes it worse.
The two that move:
189017at 1e-4 is the hardest case in the set and converges in 20 iterations instead of 35 for a 3.9% smaller mesh. Mechanism is the obvious one: refusing to split a stuck sliver leaves the sliver, and the run grinds around it.It was not preventing degeneracy either. Ten of the triwild models report the
MAX_ENERGYsentinel with the guard in place, and the per-model sentinel counts are identical without it — removing it created no additional degenerate element anywhere.101954, the model it was written against, is unchanged at 10 iterations / 28 925 tets and never reports the sentinel in either build; the failure it targeted does not reproduce, exactly as a4ebcff predicted (the envelope fix in that same commit is what fixed it).Also removes the state that existed only to feed it: the
max_quality_beforescan at the top ofsplit_edge_beforeand the field in bothSplitCachestructs.What was ruled out
sqrt(2)·l/l/sqrt(2)in place of4/3/4/5, at both collapse placements: aggregate wash (tetwild elements −2.6%, triwild +4.7%), and189017@1e-4still diverges — the wider band only delays it ten iterations.stuck_refine_force_split: falsedelays the same divergence ~4 iterations, then reproduces it. Not the mechanism.Testing
eps_rel 1e-3for every combination above, plus the cooldown sweep 0/1/2/3.189017ateps_rel 1e-4, single-threaded, throughout.🤖 Generated with Claude Code