Sketch: held solves that cannot succeed give up sooner (#6606) (#10208)

* claim: #6606

* Sketch: held solves that cannot succeed give up sooner (#6606)

A status-only solve (the held solves of a drag) only asks whether it
succeeds. When LM sits in a least-squares valley of conflicting
constraints it crawled on to its 500-iteration limit, and a held solve
runs LM up to three times. Such a solve now leaves a component once the
iteration has stalled far from a solution: over the last 20 iterations
the sum of squared residuals fell by at most 1e-5 of itself while the
largest residual is still above 1e-4 mm.

Successful solves are unaffected: on about 62,000 LM runs that succeeded
(the sketch and engine tests and 120 generated sketch scripts with 2,900
drags) the slowest still fell by 1.4e-3 over any 20 iterations while
that far off. Solves with an analysis keep iterating (their report names
the violated constraints), and live frames are unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Markus Kaltenbrunner <kaltenbrunnermarkus@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

crates/sketch/src/profiling.rs

@@ -63,6 +63,8 @@ pub enum StopReason {
     ResidualTolerance,
     NoImprovement,
     IterationLimit,
+    /// A status-only solve gave up: the iteration stalled far from a solution.
+    Stalled,
     /// The shared frame-work budget was exhausted after evaluating the initial residual.
     WorkBudget,
 }

crates/sketch/src/solver.rs

@@ -42,6 +42,19 @@ pub const MAX_COMPONENT_VARS: usize = 4_000;
 const TOL: f64 = 1e-9;
 /// A solution is accepted when every residual is below this (mm).
 const ACCEPT: f64 = 1e-6;
+/// A solve that only asks whether it succeeds ([`Analysis::StatusOnly`], the held solves of a
+/// drag) gives up on a component once LM has stalled far from a solution: over the last
+/// `STALL_ITERS` iterations the sum of squared residuals fell by at most `STALL_DECREASE` of
+/// itself, and the largest residual is still above `STALL_RESIDUAL` (mm). Such a run sits in a
+/// least-squares valley of constraints that conflict, crawling until `MAX_ITERS`, and a held
+/// solve runs LM up to three times (#6606). Measured on about 62,000 LM runs that succeeded
+/// (the sketch and engine tests and 120 generated sketch scripts with 2,900 drags), the slowest
+/// still fell by 1.4e-3 of itself over any 20 iterations while it was that far off: some cross
+/// a plateau for dozens of iterations and then converge, so the threshold stays a hundred times
+/// below that. Other solves keep iterating: their report names what a failure leaves violated.
+const STALL_ITERS: usize = 20;
+const STALL_DECREASE: f64 = 1e-5;
+const STALL_RESIDUAL: f64 = 1e-4;
 /// How much less it costs a solve to move a point held on a curve than any other point: such a
 /// point is placed relative to the curve, so it follows the curve rather than the curve it.
 const SLIDE_WEIGHT: f64 = 0.01;
@@ -1112,13 +1125,11 @@ fn norm2(v: &[f64]) -> f64 {
 }
 
 /// Levenberg–Marquardt on one component (variables `vars`, blocks `blocks`). Returns the
-/// iterations and the largest residual left.
-fn lm(sk: &Sketch, lay: &Layout, blocks: &[(Block, Vec<usize>)], vars: &[usize], x: &mut Vec<f64>, col: &[usize]) -> (usize, f64) {
-    lm_capped(sk, lay, blocks, vars, x, col, MAX_ITERS)
-}
-
-/// [`lm`], counting its work against a live drag frame's numerical allowance when `budget` has
-/// one ([`Budget::frame`]). The solve itself runs as it would without it.
+/// iterations and the largest residual left. Its work counts against a live drag frame's
+/// numerical allowance when `budget` has one ([`Budget::frame`]); the solve itself runs as it
+/// would without it. `give_up`: stop once
+/// stalled (see [`STALL_ITERS`]); not in a live frame, whose allowance is measured in the work
+/// the full iteration does.
 fn lm_in(
     sk: &Sketch,
     lay: &Layout,
@@ -1127,9 +1138,10 @@ fn lm_in(
     x: &mut Vec<f64>,
     col: &[usize],
     budget: &mut Budget,
+    give_up: bool,
 ) -> (usize, f64) {
     if !budget.counts_numeric() {
-        return lm(sk, lay, blocks, vars, x, col);
+        return lm_run(sk, lay, blocks, vars, x, col, MAX_ITERS, None, give_up).unwrap_or((0, f64::INFINITY));
     }
     let mut meter = Budget::new(usize::MAX);
     let out = lm_capped_with_budget(sk, lay, blocks, vars, x, col, MAX_ITERS, Some(&mut meter)).unwrap_or((0, f64::INFINITY));
@@ -1154,6 +1166,20 @@ fn lm_capped(
 /// Deterministic work units, shared with the frame's decomposition budget. Estimates bound
 /// dense arithmetic; they are not a wall-clock deadline. None means no residual was evaluated.
 fn lm_capped_with_budget(
+    sk: &Sketch,
+    lay: &Layout,
+    blocks: &[(Block, Vec<usize>)],
+    vars: &[usize],
+    x: &mut Vec<f64>,
+    col: &[usize],
+    max_iters: usize,
+    budget: Option<&mut Budget>,
+) -> Option<(usize, f64)> {
+    lm_run(sk, lay, blocks, vars, x, col, max_iters, budget, false)
+}
+
+/// [`lm_capped_with_budget`], giving up once stalled when `give_up` (see [`STALL_ITERS`]).
+fn lm_run(
     sk: &Sketch,
     lay: &Layout,
     blocks: &[(Block, Vec<usize>)],
@@ -1162,6 +1188,7 @@ fn lm_capped_with_budget(
     col: &[usize],
     max_iters: usize,
     mut budget: Option<&mut Budget>,
+    give_up: bool,
 ) -> Option<(usize, f64)> {
     #[cfg(feature = "solver-profiling")]
     let _timer = profiling::timer(Phase::Lm);
@@ -1177,12 +1204,25 @@ fn lm_capped_with_budget(
     let mut f = norm2(&r);
     #[cfg(feature = "solver-profiling")]
     let run = profiling::start_run(n, r.len(), max_iters, f);
+    // The sum of squares at the start of each iteration, while giving up is allowed.
+    let mut costs = Vec::new();
     for it in 0..max_iters {
         if max_abs(&r) < TOL {
             #[cfg(feature = "solver-profiling")]
             profiling::finish_run(run, StopReason::ResidualTolerance, max_abs(&r));
             return Some((it, max_abs(&r)));
         }
+        if give_up {
+            if let Some(&earlier) = it.checked_sub(STALL_ITERS).and_then(|k| costs.get(k))
+                && max_abs(&r) > STALL_RESIDUAL
+                && earlier - f <= STALL_DECREASE * earlier
+            {
+                #[cfg(feature = "solver-profiling")]
+                profiling::finish_run(run, StopReason::Stalled, max_abs(&r));
+                return Some((it, max_abs(&r)));
+            }
+            costs.push(f);
+        }
         let normal_work =
             n.saturating_mul(n).saturating_mul(r.len()).saturating_add(evaluation_work.saturating_mul(n.saturating_mul(2).saturating_add(1)));
         if !within_budget(&mut budget, normal_work) {
@@ -2728,6 +2768,10 @@ fn solve_inner(
     }
     // Column of each variable in the system being solved or analysed (usize::MAX outside it).
     let mut col = vec![usize::MAX; lay.n];
+    // Only success matters to a status-only solve: a component LM cannot solve is left as soon
+    // as the iteration stalls (see `STALL_ITERS`). It fails either way, and a failed solve
+    // leaves the geometry as it was.
+    let give_up = analysis == Analysis::StatusOnly;
 
     let mut iterations = 0;
     let mut continuations = 0;
@@ -2763,7 +2807,7 @@ fn solve_inner(
                     let rest = unpinned(vars, &pin, None);
                     let rest_blocks = blocks_reading(&sub, &rest);
                     columns(&mut col, &rest, vars);
-                    let (its, r) = if rest.is_empty() { (0, 0.0) } else { lm_in(sk, &lay, &rest_blocks, &rest, &mut x, &col, budget) };
+                    let (its, r) = if rest.is_empty() { (0, 0.0) } else { lm_in(sk, &lay, &rest_blocks, &rest, &mut x, &col, budget, give_up) };
                     iterations += its;
                     residual = r;
                     solved = residual < ACCEPT;
@@ -2787,7 +2831,7 @@ fn solve_inner(
             // the seed did not move it.
             let reseeded = unseeded.as_ref().filter(|plain| vars.iter().any(|v| plain.get(*v).is_some_and(Option::is_some)));
             if !(solved || pinned_failed && reseeded.is_some()) {
-                let (its, r) = lm_in(sk, &lay, &sub, vars, &mut x, &col, budget);
+                let (its, r) = lm_in(sk, &lay, &sub, vars, &mut x, &col, budget, give_up);
                 iterations += its;
                 residual = r;
             }
@@ -2801,7 +2845,7 @@ fn solve_inner(
                         *s = plain.get(v).copied().flatten().unwrap_or(*s0);
                     }
                 }
-                let (its, r) = lm_in(sk, &lay, &sub, vars, &mut x, &col, budget);
+                let (its, r) = lm_in(sk, &lay, &sub, vars, &mut x, &col, budget, give_up);
                 iterations += its;
                 residual = r;
             }
@@ -2832,7 +2876,7 @@ fn solve_inner(
                 iterations += its;
                 if !done {
                     // The last sub-step's solution, solved for the new values in one jump.
-                    let (its, r) = lm_in(sk, &lay, &sub, vars, &mut from, &col, budget);
+                    let (its, r) = lm_in(sk, &lay, &sub, vars, &mut from, &col, budget, false);
                     iterations += its;
                     done = edit.holds(r, &State { sk, lay: &lay, x: &from });
                 }

crates/sketch/src/tests.rs

@@ -1438,3 +1438,36 @@ fn rounded_rectangles_sharing_their_sides_leave_every_profile_closed() {
         }
     }
 }
+
+/// A drag step that cannot succeed gives up once the iteration has stalled (#6606). Holding
+/// the two right corners of a rectangle further apart than its dimensioned left side leaves the
+/// left corners a least-squares valley (the perpendicular repeats what horizontal and vertical
+/// already say), along which the iteration crawled for its whole allowance.
+#[test]
+fn a_held_solve_that_cannot_succeed_stops_once_it_stalls() {
+    let mut sk = Sketch::new();
+    let [bottom, right, top, left] = rect(&mut sk, [v(-6.0, -20.0), v(23.5, -20.8), v(23.5, 0.0), v(-6.0, 0.0)]);
+    for kind in [
+        ConstraintKind::Horizontal { l: bottom },
+        ConstraintKind::Horizontal { l: top },
+        ConstraintKind::Vertical { l: right },
+        ConstraintKind::Vertical { l: left },
+        ConstraintKind::Length { l: left, value: 20.0 },
+        ConstraintKind::Perpendicular { a: top, b: left },
+    ] {
+        sk.add_constraint(kind, None).unwrap();
+    }
+    let hold = Hold { points: sk.curves[right].kind.point_ids(), radii: vec![] };
+    let reference: Vec<Vec2> = sk.points.iter().map(|p| p.pos).collect();
+    let solve = |analysis| {
+        let mut moved = sk.clone();
+        let rep = solve_with(&mut moved, &hold, Some(&reference), analysis, None);
+        assert!(!rep.ok());
+        assert_eq!(moved.points, sk.points, "a failed solve leaves the geometry as it was");
+        rep.iterations
+    };
+    let stalled = solve(Analysis::StatusOnly);
+    assert!(stalled < 100, "a held solve gives up once stalled: {stalled} iterations");
+    // A solve whose report names the violated constraints still iterates to the end.
+    assert!(solve(Analysis::Full) >= 500);
+}