Fix fitted point optimisation#133
Open
terryspitz wants to merge 25 commits into
Open
Conversation
Add FontTests.O_And_o_Outline_IsHorizontallyAndVerticallySymmetric which verifies that every outline knot of 'O' and 'o' has a corresponding mirror point within 1 unit, both horizontally (about the bounding-box cx) and vertically (about cy). The test revealed that the DactylSpline solver's Nelder-Mead optimiser was perturbing the initial tangent angles and converging to a slightly asymmetric solution for these four-point oval glyphs. Fixed by adding explicit cardinal tangents (N/E/S/W) to the 'O' and 'o' backbone definitions so the solver treats the tangent directions as fixed and only optimises handle lengths. https://claude.ai/code/session_013obEC5m12xpz7Rcoz1NKH3
'0' and 'Q' share the same four-point symmetric oval (hl~tc~hr~bc~) as 'O'. Without explicit directions the Nelder-Mead optimiser perturbs the initial tangent angles and converges to a slightly asymmetric solution, for the same reason as was fixed for 'O' and 'o'. Adds N/E/S/W tangent hints to the oval part of each definition so the DactylSpline solver treats those directions as fixed, producing a geometrically correct symmetric oval outline. https://claude.ai/code/session_013obEC5m12xpz7Rcoz1NKH3
Any point using bracket notation (fitted coordinates) should carry an explicit cardinal tangent — this pins the optimizer's direction at extremal points and prevents the Nelder-Mead solver from drifting off the symmetric solution. Geometric rule: if y is fitted (y_fit=true) the point is a left/right extremum → tangent is vertical (S); if x is fitted (x_fit=true) the point is a top/bottom extremum → tangent is horizontal (W or E). - 'a': x(c) → x(c)W (top of bowl, going right→left) - 'B': (bh)r → (bh)rS, (th)r → (th)rS (rightmost of each bowl) - '@': te(c) → te(c)W, be(c) → be(c)E (top/bottom of inner loop) 'e', 'G', 'n', 'P', 's' already followed this rule correctly. https://claude.ai/code/session_013obEC5m12xpz7Rcoz1NKH3
Fitted coordinates (bracket notation) mark extremal points where the solver optimises one coordinate. The correct tangent direction follows mechanically from which coordinate is fitted and the direction of travel through the point, so there is no reason to repeat it in every glyph string definition. Rule added to parse_curve: - y_fit=true (point slides along fixed x, a left/right extremum) → vertical tangent: S if prev.y > next.y, else N - x_fit=true (point slides along fixed y, a top/bottom extremum) → horizontal tangent: E if next.x > prev.x, else W Only applied to interior points (or all points of a closed curve) where both neighbours are available. Explicit tangents already in the string definitions always take precedence; the auto-assignment is a fallback for None slots. Remove the now-redundant explicit tangent suffixes from the eight affected glyph definitions: @, a, B, e, G, n, P, s. https://claude.ai/code/session_013obEC5m12xpz7Rcoz1NKH3
Extends the bracket-notation pattern to every glyph with extremal curve points: bowl letters b/c/d/p/q, capitals C/D/G/R, arches h/J/U/u, descenders g/j/y, digits 2/3/6/8/9, S/$/?/t. At each geometric extremum (top, bottom, left, right of an arc), the fixed coordinate is replaced with a fitted coord so the DactylSpline solver optimises it, and the auto-tangent rule assigns the correct cardinal direction — eliminating the need for explicit N/S/E/W suffixes at these points. https://claude.ai/code/session_013obEC5m12xpz7Rcoz1NKH3
From main: j redesigned (2-point arc with explicit dlE), t simplified (xblc start, bccrW endpoint), u uses b(llcr) fitted coord. Our change: D uses (h)r for the right extremum, combined with main's tlE end-tangent. https://claude.ai/code/session_013obEC5m12xpz7Rcoz1NKH3
….com/terryspitz/dactyl-font into claude/add-glyph-symmetry-test-WIZzS
Replaces explicit cardinal tangents (hlN, tcE, hrS, bcW) with bracket notation ((h)l, t(c), (h)r, b(c)) so the auto-assignment rule handles the N/E/S/W directions. Consistent with the approach applied to all other curved glyphs. https://claude.ai/code/session_013obEC5m12xpz7Rcoz1NKH3
When a point had x fixed (non-NaN) but y free (NaN) — e.g. `(xb)l` with x_fit=false, y_fit=true — the copy condition `if IsNaN result.x` was false so the solver's optimised y was never written back, leaving y=NaN in the output. Extend the guard to `IsNaN x || IsNaN y`. Add BracketFittingTests to verify: - x(c) and x(cr) produce identical solved results (parsed value inside brackets is correctly discarded in favour of the solver-driven init) - A bracket-free point keeps its fixed x=C after solving https://claude.ai/code/session_013FzUDrEU4omtyJJtnSaqrP
Two bugs in the parseGlyph response handler: 1. Missing setSolveResult(null) meant the previous glyph's SVG path persisted on screen until the new solve completed, making it appear the tab didn't update. 2. Errors for id=-3 (parseGlyph) were silently ignored; now logged. https://claude.ai/code/session_015jzWn7EFDMMYsmWwYsKn8F
…yph-symmetry-test-WIZzS
…nes-tab-update-GsP0S
When a knot has a fitted y-coordinate (y=null) but a fixed x-coordinate, the result BezierPoint's y stayed NaN after solving because the copy condition only checked isNaN(x). Affected glyphs like 'o', 'p', 'q' that use parenthesised y-coordinates (e.g. "(xb)l") for extremum positions. Change the combined if/then to two independent checks so each coordinate is independently populated from the solver's output when NaN. https://claude.ai/code/session_015jzWn7EFDMMYsmWwYsKn8F
…com/terryspitz/dactyl-font into claude/fix-splines-tab-update-GsP0S
…n solveAndGetPoints DControlPoint gains x_init/y_init (float option) fields that carry the bracket-syntax hint value (e.g. the parsed x of `x(c)`) for potential future use without affecting current behaviour. Solver.initialise() still uses the neighbour-average as the Nelder-Mead starting point — using the raw hint causes a degenerate simplex when the hint is 0 (e.g. `b(l)` with L=0). Also fix the copy-back condition in solveAndGetPoints: the previous `if IsNaN result.[resIdx].x` guard missed points where x is fixed but y is free, leaving y=NaN for glyphs like B, G, P that use `(y)x` patterns. New BracketFittingTests fixture verifies that bracket content is ignored for the free coordinate (x(c) ≡ x(cr)) and that a fixed coordinate is unaffected by brackets. https://claude.ai/code/session_013FzUDrEU4omtyJJtnSaqrP
…em size
The two paths diverge significantly:
- dotnet test → MathNet.Numerics.Optimization.NelderMeadSimplex
- browser/Fable → fmin/nelderMead.js
fmin was under-iterated (fixed maxIter=100 vs its own default of N*200) and
its initial simplex was proportionally tiny (5% of starting value), making it
liable to get stuck in wrong local minima that MathNet would escape.
Changes:
1. Initialise free coordinates from the bracket hint when it is non-zero;
fall back to neighbour-average only for zero hints (e.g. x='(l)' where
L=0 would produce a degenerate 1e-3-unit fmin simplex). The hint is the
designer's intent for where in the search space the optimum lies.
2. In the FABLE path, scale maxIterations = N * max(maxIter, 200) so fmin
gets at least as many iterations as its own heuristic would provide.
Update bracket tests: bracket content now intentionally steers the initial
search, so replace "content is ignored" assertions with "converges to finite,
non-degenerate values inside glyph bounds" assertions.
https://claude.ai/code/session_013FzUDrEU4omtyJJtnSaqrP
…minima Single-start Nelder-Mead can get stuck in different local minima depending on the starting point. Fix: after the hint-based run, also run from the neighbour-average starting point and keep whichever achieves lower error. Applied to both the FABLE (fmin) and .NET (MathNet) paths for consistency. The new BracketX_MultiStartAvoidsDegenerate test proves what multi-start guarantees: even the hardest hint (x(r)W starting at x=R=300) is pulled back from the degenerate "collapsed first segment" solution (x≈R). Different hints still legitimately find different local minima (~153, ~188, ~263 for C, (C+R)/2, R hints) — these are genuine objective-function minima with different values, not a convergence failure. https://claude.ai/code/session_013FzUDrEU4omtyJJtnSaqrP
…P0S' into claude/debug-bracket-syntax-1GJ1K
… limit Solver.Solve (.NET path): wrap FindMinimum in try/catch so hitting the iteration limit doesn't crash the caller — fall back to the last evaluated candidate rather than propagating MaximumIterationsException. Add PlotObjectiveVsTopX (Explicit/diagnostic) to BracketFittingTests: sweeps the top-point x of the 'c' glyph across [0,R] with x fixed and other parameters optimised, printing CSV + ASCII bar chart of the objective function landscape. https://claude.ai/code/session_013FzUDrEU4omtyJJtnSaqrP
The bracket syntax (e.g. x(cr)) sets x=None (free) with a hint for the initial Nelder-Mead start. Previously, starting from a far hint like cr=225 gave a different result than c=150, because the inconsistent hint/avg pair landed in different objective-function basins. Key changes: - Add `reinitAnglesAndDistances()` to recompute tangent angles and Bezier handle distances from current _points geometry, independent of the hint-based initialisation. - In Solver.Solve(), build a 3rd start (initialAvgConsistent) by applying the neighbour-average x/y to _points, calling reinitAnglesAndDistances(), and snapshotting the result. This gives a geometrically-coherent starting point that avoids the hint-angle mismatch that trapped the optimizer in the wrong basin. - Only use wide perturbation (30%, floor 15) for the 3rd start, and only when initialAvgConsistent meaningfully differs from initialArr (> 5 units); when starts are identical, skip the wide-perturb run to avoid finding unexpected local minima in other tests. - Switch MathNet path to best-tracking objective that captures the lowest-error point seen during any optimization run (guards against iteration-limit fallback returning a non-best point). - fmin path: nonZeroDelta=1.3, zeroDelta=15.0 (unchanged from earlier in session). - New test BracketX_MultiStartConvergesFromAllHints: all 5 hints (l, cl, c, cr, r) converge to the same x within 30 units. Observed spread after fix: ~11.7 units. All 76 existing .NET unit tests pass. https://claude.ai/code/session_013FzUDrEU4omtyJJtnSaqrP
terryspitz
changed the title
Previously all solver sections paid the 3-start cost regardless of whether bracket syntax was used. For ordinary glyphs this caused a large slowdown because: - scaledIter = N * max(maxIter, 200) inflated iterations 10x - 3 optimizer runs instead of 1 Now: check whether the hint-based initialisation differs from the neighbour-average by > 5 units. When it does not (no bracket syntax, or hint ≈ avg), a single run is used — identical to main. When it does, the 3-run strategy fires: hint-start, avg-start, and the geometry-consistent avg start with wide perturbation. The reinitAnglesAndDistances / save-restore is also skipped for the common (no-diff) case. https://claude.ai/code/session_013FzUDrEU4omtyJJtnSaqrP
…ng points Tests 5 starting heuristics (designer hint, midpoint, 7-point grid scan, perpendicular-arc offset, tangent intersection) across 11 bracket-using glyphs, 5 axis configurations, and width tweens for 'c' and 'S'. Key finding: H4perp (arc perpendicular) wins 83% of 170 cases with mean rank 1.38, outperforming the legacy designer hint (worst, mean rank 1.71). For open-arc glyphs (c, b, 3top, 3bot) the improvement is 31–82%. The Recommendation section in the plan file proposes replacing bracket hints with geometry-driven H4perp in production. The new test is [<Explicit>] and does not run in normal CI. https://claude.ai/code/session_013FzUDrEU4omtyJJtnSaqrP
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
2 participants
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.
Add FontTests.O_And_o_Outline_IsHorizontallyAndVerticallySymmetric which
verifies that every outline knot of 'O' and 'o' has a corresponding mirror
point within 1 unit, both horizontally (about the bounding-box cx) and
vertically (about cy).
The test revealed that the DactylSpline solver's Nelder-Mead optimiser was
perturbing the initial tangent angles and converging to a slightly asymmetric
solution for these four-point oval glyphs. Fixed by adding explicit cardinal
tangents (N/E/S/W) to the 'O' and 'o' backbone definitions so the solver
treats the tangent directions as fixed and only optimises handle lengths.
https://claude.ai/code/session_013obEC5m12xpz7Rcoz1NKH3