diff --git a/CLAUDE.md b/CLAUDE.md index f5e9574..10b8fb0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -468,10 +468,13 @@ used to, passing at repulsion 1,200,000 and 600,000, failing at 800,000, and pas `tests/ForceDirectedLayout.Tests/Bench/` is the harness that replaces that: -- **`GraphCorpus`** — four graphs that break a layout differently. `Counter` is a real twenty-node - document with sizes running from a 60-wide literal to a 118x180 function; `Chain` is the shape that - most wants to be a horizontal row; `FanIn` is eight sources arriving at eight pins on one target, - which is where crossings come from; `MixedSizes` alternates 400-wide slabs with 50-wide literals. +- **`GraphCorpus`** — six graphs that break a layout differently. `Counter` is a real twenty-node + document with sizes running from a 60-wide literal to a 118x180 function; `TwoClasses` is a + thirty-four-node document with two roots and calls crossing between them, which is where a link gets + drawn through a body parked in the middle; `Chain` is the shape that most wants to be a horizontal + row; `FanIn` is eight sources arriving at eight pins on one target, which is where crossings come + from; `MixedSizes` alternates 400-wide slabs with 50-wide literals; `Disconnected` is three + components with no link between them, the only shape that measures what gravity is for. Node sizes and pin rows are not decoration — repulsion measures clear space between boxes, every angle force measures between pins, and a graph of equal-sized points exercises none of it. - **`LayoutMetrics`** — settled area, mean edge angle, links drawn across a body they are no end of, @@ -485,6 +488,18 @@ used to, passing at repulsion 1,200,000 and 600,000, failing at 800,000, and pas - **`LayoutSvg`** — writes a settled graph to SVG, links drawn first as the cubic the renderer actually draws and nodes over them, so a link hidden in the picture is a link hidden in the editor. Overlapping bodies are outlined in red. No window, no GPU, no ImGui context. +- **`LayoutScore` / `LayoutTuner`** — one weighted number per configuration, and a coordinate descent + over it. The score is a judgement call written down, and its weights are the thing to argue with + first if you disagree with a tuned default. + +**The corpus score is a random variable, and it is noisier than the gains a tuning run chases.** +Measured over independent families of starting arrangements (`BenchOptions.StartOffset`), the same +settings score with a standard deviation of 0.317 at twelve starts, 0.096 at twenty-four and 0.032 at +forty-eight. Two descents run at eight and twelve starts, keeping every improvement, reached values +that disagreed on six settings out of fifteen and scored within 0.01 of each other — both were fitting +the arrangements they were handed. Raise `Starts` until the deviation is small against the gain being +claimed, keep `LayoutTuner.DefaultMinimumGain` a few times above it, and validate the result on a +`StartOffset` family it was not chosen on. To iterate: add a scratch `[TestMethod]` that prints a sweep, run the suite, read the column that should have moved. @@ -511,12 +526,57 @@ Two things that bite: `Corpus_SettlesIntoAReadableShape_UnderTheDefaults` is the quality gate a layout change is expected to break if it makes things worse. Its per-graph thresholds are current behaviour with headroom, not -targets — and two of them are loose because of a real defect the corpus exposed: **centre gravity coils -a long chain**. A plain twelve-node chain settles at about 53 degrees mean edge angle with only two -starts in six reading left to right, and it is not a settling-time problem (4000, 12000 and 30000 -frames all land on 52.6). Sweeping `GravityStrength` over the same starts gives 0.6 degrees at 0, 1.9 -at 10, 52.9 at the default 50, and 58.4 at 200; raising `DirectionalBias` makes it worse rather than -better, because ordering pairs left-to-right says nothing about the shape of the whole. +targets. + +### Tuned defaults + +`LayoutSettings.Defaults` is measured, not inherited. A coordinate descent over the corpus +(`LayoutTuner`, scored by `LayoutScore`) moved six of the fifteen settings and left the other nine +where they were; the corpus score went from 3.25 to 1.19 and held at 1.22–1.28 on three families of +starting arrangements the values were never chosen on. + +| setting | was | is | +|---|---|---| +| `RepulsionStrength` | 600,000 | 900,000 | +| `MinRepulsionDistance` | 50 | 5 | +| `LinkSpringStrength` | 0.5 | 0.1 | +| `RestLinkLength` | 225 | 50 | +| `DirectionalBias` | 0.5 | 4 | +| `LinkFlatteningStrength` | 0.5 | 3 | + +That fixed the defect this corpus was built to expose. **Centre gravity used to coil a long chain**: a +plain twelve-node chain settled at about 53 degrees mean edge angle with two starts in six reading +left to right, and it was not a settling-time problem (4000, 12000 and 30000 frames all landed on +52.6). It now settles at 0.1 degrees, twelve starts in twelve, and `MixedSizes` — a chain too — went +from 50 degrees and 1/12 readable to 2.1 degrees and 12/12. + +**Gravity is not what fixed it.** Weakening `GravityStrength` did straighten the chain, but at the +cost of the one thing gravity is for, and the descent left it at 50 untouched. `LinkFlatteningStrength` +at six times its old value simply outcompetes the coil: a force pulling each edge towards horizontal +beats one pulling every body towards a point, and neither has to be turned off for that to be true. + +Four things to know before changing any of it: + +- **Flattening hides links, monotonically.** Links drawn across a body they are no end of go from 0.10 + to 0.40 (normalised) as the setting goes 0 → 12, and roughly double on `TwoClasses`, whose + cross-class calls are the long edges that have to cross whatever is parked between them. That is the + price of everything above. The score's minimum is a shallow basin — 2 and 3 land within half a + standard deviation of each other, 2 hiding fewer links and settling slightly less reliably — so that + particular choice is a judgement call and not a measurement. +- **`MinRepulsionDistance` does nothing below `sqrt(RepulsionStrength / MaxForce)`**, which is about + 13 under these defaults. `MaxForce` caps the total force at 5,000 while the law's own cap is 36,000, + so close-range repulsion is a constant 5,000 and the law only reappears past 13 units of clear space. + Sweeping the setting across 0, 2, 5 and 10 measures the same layout four times. What it really + controls is whether that floor is hard or soft: set it high enough that the law's cap falls below + `MaxForce` and repulsion goes soft at close range, which is what used to let bodies crowd. +- **Zero is not a valid `MinRepulsionDistance`, and the library now floors it.** It is itself the clamp + keeping the inverse-square law finite where boxes touch, so zero used to yield infinity and then NaN + positions — a layout that stops being numbers rather than one that is merely bad. +- **A mechanism test must pin the settings its mechanism depends on.** Three tests in + `ForceLayoutTests` broke on this tuning without anything being wrong: the flattening splay stopped + splaying because the pair now levels completely, and the untwist's overlap-pass exemption became + unobservable because flattening decides that geometry outright. Their fixtures now name every input + they rely on. ### Demo UI tests diff --git a/ForceDirectedLayout/LayoutCore.cs b/ForceDirectedLayout/LayoutCore.cs index 9bd5168..2ff635f 100644 --- a/ForceDirectedLayout/LayoutCore.cs +++ b/ForceDirectedLayout/LayoutCore.cs @@ -240,7 +240,13 @@ private void ResetForces() /// private void CalculateRepulsionForces() { - double minDist = Settings.MinRepulsionDistance; + // Floored above zero, not just taken as given. The clamp below is what stops an inverse-square + // force exploding when two boxes touch, and a caller who sets MinRepulsionDistance to zero + // removes it: the clear distance between touching boxes is exactly zero, so the division + // yields infinity, the integrator carries that into a NaN position, and every metric taken + // afterwards reads NaN rather than "bad". A layout that silently becomes NaN is worse than one + // that is merely crowded, so zero means "as small as this can safely be" rather than nothing. + double minDist = Math.Max(Settings.MinRepulsionDistance, MinimumRepulsionClamp); double strength = Settings.RepulsionStrength; for (int i = 0; i < bodyCount; i++) @@ -271,6 +277,16 @@ private void CalculateRepulsionForces() } } + /// + /// Smallest separation the inverse-square repulsion is ever evaluated at, whatever + /// says. + /// + /// + /// Small enough that it changes nothing for any usable setting, and positive so the division can + /// never be by zero. + /// + private const double MinimumRepulsionClamp = 0.001; + /// /// The distance between the two closest points on two bodies' bounding boxes: their gap along each /// axis they are disjoint on, and zero once they touch or overlap on both. diff --git a/ForceDirectedLayout/LayoutSettings.cs b/ForceDirectedLayout/LayoutSettings.cs index a8eb51e..79b77fc 100644 --- a/ForceDirectedLayout/LayoutSettings.cs +++ b/ForceDirectedLayout/LayoutSettings.cs @@ -89,24 +89,47 @@ public struct LayoutSettings public double MaxOverlapCorrection; /// - /// Sensible defaults matching the previous Force<float>/Length<float> values, save for - /// , which was recalibrated when repulsion moved from measuring - /// between body centres to measuring the clear space between their bounding boxes. + /// Defaults tuned against the benchmark corpus, one setting at a time. /// + /// + /// These are measured rather than inherited. Six of them were moved by a coordinate descent over + /// the corpus in tests/ForceDirectedLayout.Tests/Bench/, scored by LayoutScore, and + /// the other nine were offered the same range and declined it. The corpus score went from 3.25 to + /// 1.19, and held at 1.22-1.28 on three families of starting arrangements the values were never + /// chosen on, against a measurement deviation of 0.03. + /// + /// What moved, and what it bought: a chain used to settle at about 54 degrees off horizontal with + /// five starts in twelve reading left to right, because pulling every body towards one centre folds + /// a long chain into a coil. It now settles at 0.1 degrees, twelve in twelve. That was not fixed by + /// weakening , which the descent left exactly where it was, but by + /// at six times its old value, which simply outcompetes the + /// coil. The link spring gets weaker and much shorter to go with it, so link geometry is settled by + /// the flattening force rather than negotiated with the spring, and repulsion rises to keep the + /// tighter graph from crowding. + /// + /// + /// The cost, which is real and worth knowing before raising the flattening further: links drawn + /// across a body they are no end of get more common as the graph flattens, roughly doubling on the + /// two-class corpus graph. It rises monotonically with the setting, so 3 is where it stops buying + /// enough to be worth it; 2 hides fewer links and settles slightly less reliably, and the two score + /// within half a standard deviation of each other, so that particular choice is a judgement call + /// and not a measurement. + /// + /// public static LayoutSettings Defaults => new() { Enabled = 0, - RepulsionStrength = 600_000.0, - LinkSpringStrength = 0.5, - DirectionalBias = 0.5, - LinkFlatteningStrength = 0.5, + RepulsionStrength = 900_000.0, + LinkSpringStrength = 0.1, + DirectionalBias = 4.0, + LinkFlatteningStrength = 3.0, LinkFlatteningMargin = 0.0, LinkUntwistStrength = 0.1, GravityStrength = 50.0, OriginAnchorWeight = 1.0, DampingFactor = 0.5, - MinRepulsionDistance = 50.0, - RestLinkLength = 225.0, + MinRepulsionDistance = 5.0, + RestLinkLength = 50.0, MaxForce = 5000.0, MaxVelocity = 250.0, TargetPhysicsHz = 120.0, diff --git a/ForceDirectedLayout/PhysicsSettings.cs b/ForceDirectedLayout/PhysicsSettings.cs index 8a9fe63..d2c97d3 100644 --- a/ForceDirectedLayout/PhysicsSettings.cs +++ b/ForceDirectedLayout/PhysicsSettings.cs @@ -16,20 +16,20 @@ public sealed record PhysicsSettings /// Strength of pairwise inverse-square repulsion between bodies, measured across the clear space /// between their bounding boxes rather than between their centres. /// - public double RepulsionStrength { get; init; } = 600_000.0; + public double RepulsionStrength { get; init; } = 900_000.0; /// Dimensionless Hooke's-law spring constant for edges. - public double LinkSpringStrength { get; init; } = 0.5; + public double LinkSpringStrength { get; init; } = 0.1; /// Strength of the horizontal source-left/target-right ordering bias. 0 disables it. - public double DirectionalBias { get; init; } = 0.5; + public double DirectionalBias { get; init; } = 4.0; /// /// Strength of the preference for horizontal edges: it both levels an edge's two ends and, when the /// rendered curve would otherwise hide, splays them apart horizontally. 0 disables both. /// See for the clearance geometry. /// - public double LinkFlatteningStrength { get; init; } = 0.5; + public double LinkFlatteningStrength { get; init; } = 3.0; /// Extra horizontal clearance demanded on top of the derived bezier bound, in position units. public double LinkFlatteningMargin { get; init; } @@ -53,10 +53,10 @@ public sealed record PhysicsSettings /// Floor on the clear space used as the inverse-square repulsion denominator, so a pair that touches /// pushes hard rather than infinitely hard. /// - public double MinRepulsionDistance { get; init; } = 50.0; + public double MinRepulsionDistance { get; init; } = 5.0; /// Spring rest length for edges. - public double RestLinkLength { get; init; } = 225.0; + public double RestLinkLength { get; init; } = 50.0; /// Per-body force magnitude cap (applied before integration). public double MaxForce { get; init; } = 5000.0; diff --git a/tests/ForceDirectedLayout.Tests/Bench/LayoutBenchTests.cs b/tests/ForceDirectedLayout.Tests/Bench/LayoutBenchTests.cs index 7b2df34..3760a57 100644 --- a/tests/ForceDirectedLayout.Tests/Bench/LayoutBenchTests.cs +++ b/tests/ForceDirectedLayout.Tests/Bench/LayoutBenchTests.cs @@ -290,30 +290,33 @@ private sealed record Expectation(BenchGraph Graph, double MaxEdgeAngle, int Min /// /// /// This is the test a layout change is expected to break if it makes things worse, and the reason - /// the corpus has four shapes rather than one — a change that helps a wide fan-in can hurt a deep + /// the corpus has six shapes rather than one — a change that helps a wide fan-in can hurt a deep /// chain, and only running both says so. The thresholds are current behaviour with headroom, not /// targets: they are here to catch a regression, not to pin the numbers a particular tuning /// happens to produce. /// - /// Two of them are loose for a reason worth knowing, because it is a real defect and not a quirk - /// of the measurement. is a plain twelve-node chain, the shape that - /// most obviously wants to be a horizontal row, and it settles at about 53 degrees with only two - /// starts in six reading left to right. It is not a question of settling time — 4000, 12000 and - /// 30000 frames all land on the same 52.6 — it is gravity. Sweeping - /// over the same six starts: + /// They used to be far looser, and the reason they no longer need to be is worth recording. A plain + /// twelve-node chain — the shape that most obviously wants to be a horizontal row — used to settle + /// at about 53 degrees off horizontal with only two starts in six reading left to right, because + /// pulling every body towards one centre folds a long chain into a coil. + /// is a chain too and coiled the same way, at 50 degrees with + /// one start in twelve. Both now settle flat: 0.1 and 2.1 degrees, twelve starts in twelve. /// - /// - /// gravity=0 angle 0.6 readable 5/6 - /// gravity=10 angle 1.9 readable 5/6 - /// gravity=50 angle 52.9 readable 2/6 (the default) - /// gravity=200 angle 58.4 readable 0/6 - /// /// - /// Pulling every body towards one centre folds a long chain into a coil, and the directional bias - /// that is supposed to order it left-to-right does not undo that — raising the bias makes it worse - /// (62.7 degrees at bias 2), because ordering the pairs says nothing about the shape of the whole. - /// is a chain too and coils the same way. Fixing it is a - /// change to gravity, not to this gate, so the thresholds record where it stands. + /// What fixed it is not what the defect looked like it needed. Weakening + /// did straighten the chain, but at the cost of the + /// thing gravity is for, and a tuning run over the whole corpus left it exactly where it was at 50. + /// The coil is undone by instead, which at six + /// times its old value simply outcompetes it — a force that pulls each edge towards horizontal + /// beats one that pulls every body towards a point, and neither has to be turned off for that to be + /// true. + /// + /// + /// One threshold is still loose, and it is a real cost rather than a quirk: + /// is allowed a high count of links drawn across bodies. Links + /// hide more readily as a graph flattens, monotonically with the setting, and that graph's calls + /// between its two classes are the long edges that have to cross whatever is parked between them. + /// It is the price of everything above. /// /// [TestMethod] @@ -323,10 +326,12 @@ public void Corpus_SettlesIntoAReadableShape_UnderTheDefaults() Expectation[] expectations = [ - new(GraphCorpus.Counter, MaxEdgeAngle: 40.0, MinReadableStarts: 5), - new(GraphCorpus.Chain, MaxEdgeAngle: 60.0, MinReadableStarts: 1), - new(GraphCorpus.FanIn, MaxEdgeAngle: 45.0, MinReadableStarts: 4), - new(GraphCorpus.MixedSizes, MaxEdgeAngle: 58.0, MinReadableStarts: 1), + new(GraphCorpus.Counter, MaxEdgeAngle: 25.0, MinReadableStarts: 5), + new(GraphCorpus.TwoClasses, MaxEdgeAngle: 32.0, MinReadableStarts: 5), + new(GraphCorpus.Chain, MaxEdgeAngle: 10.0, MinReadableStarts: 5), + new(GraphCorpus.FanIn, MaxEdgeAngle: 20.0, MinReadableStarts: 4), + new(GraphCorpus.MixedSizes, MaxEdgeAngle: 15.0, MinReadableStarts: 5), + new(GraphCorpus.Disconnected, MaxEdgeAngle: 10.0, MinReadableStarts: 4), ]; List rows = []; diff --git a/tests/ForceDirectedLayout.Tests/Bench/LayoutTuner.cs b/tests/ForceDirectedLayout.Tests/Bench/LayoutTuner.cs index c53c3fa..5a7a0c1 100644 --- a/tests/ForceDirectedLayout.Tests/Bench/LayoutTuner.cs +++ b/tests/ForceDirectedLayout.Tests/Bench/LayoutTuner.cs @@ -105,7 +105,7 @@ public static class LayoutTuner s => s.LinkSpringStrength, (s, v) => s with { LinkSpringStrength = v }), new("RestLinkLength", [50, 75, 100, 150, 225, 300, 400, 500], s => s.RestLinkLength, (s, v) => s with { RestLinkLength = v }), - new("DirectionalBias", [0, 0.25, 0.5, 1.0, 2.0, 4.0], + new("DirectionalBias", [0, 0.25, 0.5, 1.0, 2.0, 4.0, 8.0, 16.0], s => s.DirectionalBias, (s, v) => s with { DirectionalBias = v }), new("LinkFlatteningStrength", [0, 0.25, 0.5, 1.0, 2.0, 3.0, 4.0, 6.0, 8.0, 12.0], s => s.LinkFlatteningStrength, (s, v) => s with { LinkFlatteningStrength = v }), diff --git a/tests/ForceDirectedLayout.Tests/Bench/LayoutTunerTests.cs b/tests/ForceDirectedLayout.Tests/Bench/LayoutTunerTests.cs index fb2ca6a..0cf7bed 100644 --- a/tests/ForceDirectedLayout.Tests/Bench/LayoutTunerTests.cs +++ b/tests/ForceDirectedLayout.Tests/Bench/LayoutTunerTests.cs @@ -103,10 +103,24 @@ public void Score_TotalIsTheWeightedSumOfItsTerms() [TestMethod] public void Score_PenalisesALayoutThatNeverRan() { - // Disabling the simulation leaves every node where it was scattered, which is the worst + // Every force zeroed, so the nodes stay exactly where they were scattered - which is the worst // arrangement available. A score that cannot tell that apart from a settled one is measuring // nothing. - LayoutSettings inert = LayoutSettings.Defaults with { LinkSpringStrength = 0.0, GravityStrength = 0.0 }; + // + // Silencing only the spring and gravity is not enough and made this test wrong once already: + // the remaining forces still organise a graph perfectly well, so "inert" has to mean all of + // them rather than the two that happen to look like the important ones. + LayoutSettings inert = LayoutSettings.Defaults with + { + RepulsionStrength = 0.0, + LinkSpringStrength = 0.0, + GravityStrength = 0.0, + DirectionalBias = 0.0, + LinkFlatteningStrength = 0.0, + LinkUntwistStrength = 0.0, + OriginAnchorWeight = 0.0, + OverlapMargin = 0.0, + }; LayoutScore settled = LayoutScore.OfCorpus(LayoutSettings.Defaults, Quick, TinyCorpus); LayoutScore scattered = LayoutScore.OfCorpus(inert, Quick, TinyCorpus); diff --git a/tests/ForceDirectedLayout.Tests/ForceLayoutTests.cs b/tests/ForceDirectedLayout.Tests/ForceLayoutTests.cs index f8a17f4..0e1e89d 100644 --- a/tests/ForceDirectedLayout.Tests/ForceLayoutTests.cs +++ b/tests/ForceDirectedLayout.Tests/ForceLayoutTests.cs @@ -320,6 +320,13 @@ public void GenericFacade_Step_WithEdge_PullsNodesCloser() /// Settings that isolate the flattening force: no repulsion, no gravity, no ordering bias, and a /// spring only strong enough to hold the pair together. /// + /// + /// The flattening strength is pinned here rather than inherited, because the splay this fixture + /// exists to test only happens while a drop remains to be cleared. Left on the default it stopped + /// testing anything the moment that default was tuned upwards: the pair levelled completely, the + /// drop went to zero, and a bound of zero is met by any arrangement at all. A mechanism test should + /// fail when the mechanism breaks, not when a default moves, so every input it depends on is named. + /// private static PhysicsSettings FlatteningOnly() => new() { Enabled = true, @@ -327,6 +334,8 @@ public void GenericFacade_Step_WithEdge_PullsNodesCloser() GravityStrength = 0, DirectionalBias = 0, LinkSpringStrength = 0.1, + LinkFlatteningStrength = 0.5, + RestLinkLength = 225.0, OverlapMargin = 0, DampingFactor = 0.1, }; @@ -556,25 +565,41 @@ public void TwistedLinks_SwapTheirFarEndsIntoPinOrder() } /// - /// Tests that a graph with no pin offsets is left alone, since without them there is no pin order to - /// be wrong about. + /// Tests that a graph with no pin offsets is left alone by the untwist force, since without them + /// there is no pin order to be wrong about. /// + /// + /// Asserted as the difference the force makes rather than as a position, which is what it names and + /// the only form of it that is actually about untwisting. Asserting instead that the two sources + /// keep the vertical order they started in tested something incidental: with no pin offsets both + /// links attach at their bodies' mid-heights, so once flattening pulls the pair to a common height + /// the order they separate into is arbitrary, and a stronger flattening default flipped it. Nothing + /// was wrong with the untwist, and the test said there was. + /// [TestMethod] public void Untwisting_DoesNothingWithoutPinOffsets() { - List bodies = [Body(1, 0, 260, 100, 60), Body(2, 0, 0, 100, 60), Body(3, 400, 100, 120, 140)]; - List edges = [new(1, 3), new(2, 3)]; - - ForceDirectedLayout layout = CreateLayout(new PhysicsSettings { Enabled = true }); - for (int i = 0; i < 600; i++) + static (double First, double Second) Settle(double untwistStrength) { - layout.Step(bodies, edges, 0.016); + List bodies = [Body(1, 0, 260, 100, 60), Body(2, 0, 0, 100, 60), Body(3, 400, 100, 120, 140)]; + List edges = [new(1, 3), new(2, 3)]; + + ForceDirectedLayout layout = CreateLayout( + new PhysicsSettings { Enabled = true, LinkUntwistStrength = untwistStrength }); + + for (int i = 0; i < 600; i++) + { + layout.Step(bodies, edges, 0.016); + } + + return (bodies[0].Position.Y, bodies[1].Position.Y); } - // Both links fall back to their bodies' mid-heights, giving the two the same pin height at the - // shared node, so neither ordering is the wrong one and the starting order survives. - Assert.IsTrue(bodies[0].Position.Y > bodies[1].Position.Y, - "with no pin offsets the two sources should keep the order they started in"); + (double firstOff, double secondOff) = Settle(0.0); + (double firstOn, double secondOn) = Settle(LayoutSettings.Defaults.LinkUntwistStrength); + + Assert.AreEqual(firstOff, firstOn, 1e-9, "with no pin offsets the untwist should not move the first source"); + Assert.AreEqual(secondOff, secondOn, 1e-9, "nor the second"); } /// @@ -651,8 +676,22 @@ public void TwistedLinks_AreNotHeldApartOnTheAxisTheySwapAlong() new(2, 3, new Vec2D(300, 25), new Vec2D(0, 100)), ]; + // Isolated rather than run on the defaults, because the claim is about the overlap pass and + // nothing else. Flattening in particular decides this geometry outright once it is strong: + // two wide slabs stacked in a column are levelled onto the target so hard that the pair + // settles identically whether the untwist runs or not, and the exemption under test becomes + // unobservable rather than absent. Measured on the tuned defaults, the held and free cases + // agree to six figures; with flattening and the ordering bias off, they separate by 193 + // vertically and 426 horizontally, which is the effect this test is named for. ForceDirectedLayout layout = CreatePinnedLayout( - new PhysicsSettings { Enabled = true, LinkUntwistStrength = untwistStrength }); + new PhysicsSettings + { + Enabled = true, + LinkUntwistStrength = untwistStrength, + RestLinkLength = 225.0, + LinkFlatteningStrength = 0, + DirectionalBias = 0, + }); for (int i = 0; i < 3000; i++) { @@ -666,9 +705,13 @@ public void TwistedLinks_AreNotHeldApartOnTheAxisTheySwapAlong() (double heldVertical, double heldHorizontal) = Settle(0.0); (double freeVertical, double freeHorizontal) = Settle(0.1); - double clearance = (50 * 0.5) + (50 * 0.5) + LayoutSettings.Defaults.OverlapMargin; - Assert.IsTrue(heldVertical >= clearance, - $"With nothing untwisting them the pair should be held at least a clearance apart vertically; it was {heldVertical:F0} against {clearance:F0}."); + // Half of each height: the pair is separated on Y rather than left stacked. The overlap margin is + // deliberately not added on top. It is a soft preference that the rest of the forces settle just + // inside, and how far inside depends on how tightly the spring and repulsion are tuned - so + // asserting the margin here measured the defaults rather than the overlap pass. + double separated = (50 * 0.5) + (50 * 0.5); + Assert.IsGreaterThanOrEqualTo(separated, heldVertical, + $"With nothing untwisting them the pair should be held apart vertically; it was {heldVertical:F0} against {separated:F0}."); Assert.IsTrue(freeVertical < heldVertical, $"A twisted pair should be allowed to close on the axis it swaps along; it settled {freeVertical:F0} apart against {heldVertical:F0}."); Assert.IsTrue(freeHorizontal > heldHorizontal, diff --git a/tests/ForceDirectedLayout.Tests/RepulsionTests.cs b/tests/ForceDirectedLayout.Tests/RepulsionTests.cs index 0d897b5..44b9a66 100644 --- a/tests/ForceDirectedLayout.Tests/RepulsionTests.cs +++ b/tests/ForceDirectedLayout.Tests/RepulsionTests.cs @@ -137,24 +137,85 @@ public void Repulsion_PushesAlongTheLineBetweenTheCentres() /// /// Tests that a pair with no clear space between them is pushed at the floor rather than infinitely - /// hard, and that the floor is reached at contact rather than at coincidence. + /// hard, and that the law takes over again once there is room between them. /// + /// + /// There are two floors, and which one binds depends on the settings. + /// caps the inverse-square law at + /// RepulsionStrength / MinRepulsionDistance², and caps + /// the total force on a body regardless. Under the tuned defaults the second is by far the lower — + /// the law's cap is 36,000 against a MaxForce of 5,000 — so close-range repulsion is a constant + /// 5,000 and the law only reappears beyond about 13 units of clear space. + /// + /// Worth knowing before tuning either: it means + /// has no effect at all below sqrt(RepulsionStrength / MaxForce), which is why sweeping it + /// across 0, 2, 5, 10 measures the same layout four times. What it does control is whether the + /// floor is hard or soft: set it high enough that the law's cap falls under MaxForce and close-range + /// repulsion goes soft, which is what used to let bodies crowd. + /// + /// [TestMethod] public void Repulsion_WithNoClearSpace_PushesAtTheFloor() { Vec2D size = new(100, 100); - double floor = LayoutSettings.Defaults.RepulsionStrength / - (LayoutSettings.Defaults.MinRepulsionDistance * LayoutSettings.Defaults.MinRepulsionDistance); + double minDist = LayoutSettings.Defaults.MinRepulsionDistance; + double strength = LayoutSettings.Defaults.RepulsionStrength; + + double floor = Math.Min(strength / (minDist * minDist), LayoutSettings.Defaults.MaxForce); + // Where the law resumes: the clear distance at which it first asks for less than the floor. + double lawResumesAt = Math.Sqrt(strength / floor); + + // Two 100-wide boxes whose centres are 100 apart are touching, so a centre offset of 100 plus a + // clear distance is that much clear space between their facing edges. double overlapping = RepulsionOn(size, size, new Vec2D(30, 0)).Length(); double touching = RepulsionOn(size, size, new Vec2D(100, 0)).Length(); - double atTheFloor = RepulsionOn(size, size, new Vec2D(150, 0)).Length(); - double beyond = RepulsionOn(size, size, new Vec2D(200, 0)).Length(); + double justInside = RepulsionOn(size, size, new Vec2D(100 + (lawResumesAt * 0.5), 0)).Length(); + double beyond = RepulsionOn(size, size, new Vec2D(100 + (lawResumesAt * 2.0), 0)).Length(); Assert.AreEqual(floor, overlapping, 0.001, "overlapping bodies should be pushed at the floor"); Assert.AreEqual(floor, touching, 0.001, "and so should touching ones"); - Assert.AreEqual(floor, atTheFloor, 0.001, "and so should a pair exactly one floor of clear space apart"); + Assert.AreEqual(floor, justInside, 0.001, "and so should a pair inside the distance the law resumes at"); Assert.IsTrue(beyond < floor, $"beyond it the law takes over again; {beyond:F1} against the floor's {floor:F1}"); + Assert.AreEqual(strength / (lawResumesAt * 2.0 * lawResumesAt * 2.0), beyond, 0.001, + "and once it has, the force is the law's value at that clear distance"); + } + + /// + /// Tests that a zero still yields a finite layout + /// rather than a NaN one. + /// + /// + /// That setting is itself the clamp keeping the inverse-square law finite where bodies touch, so + /// setting it to zero removes the only thing preventing a division by zero: touching boxes have + /// exactly no clear space between them, the force comes back infinite, and the integrator carries + /// that into positions that are NaN forever after. It surfaced from a parameter sweep that happened + /// to offer zero, where every measurement of the result read NaN rather than "bad" — which is the + /// real cost, since a layout that silently stops being a number is worse than a crowded one. + /// + [TestMethod] + public void Repulsion_WithNoMinimumDistance_StaysFinite() + { + LayoutSettings settings = RepulsionOnly() with { MinRepulsionDistance = 0.0 }; + + LayoutCore core = new() { Settings = settings }; + core.ResizeBodies(2); + + // Overlapping, which is where the clear distance between them is exactly zero. + core.Bodies[0] = new BodyState { Id = 1, Position = Vec2D.Zero, Dimensions = new Vec2D(100, 100) }; + core.Bodies[1] = new BodyState { Id = 2, Position = new Vec2D(30, 0), Dimensions = new Vec2D(100, 100) }; + + for (int frame = 0; frame < 60; frame++) + { + core.Step(1.0 / 60.0); + } + + for (int i = 0; i < core.BodyCount; i++) + { + Vec2D position = core.Bodies[i].Position; + Assert.IsTrue(double.IsFinite(position.X) && double.IsFinite(position.Y), + $"body {i} should still have a real position; it was ({position.X}, {position.Y})"); + } } ///