Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 70 additions & 10 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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.
Expand All @@ -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

Expand Down
18 changes: 17 additions & 1 deletion ForceDirectedLayout/LayoutCore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,13 @@
/// </remarks>
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++)
Expand Down Expand Up @@ -271,6 +277,16 @@
}
}

/// <summary>
/// Smallest separation the inverse-square repulsion is ever evaluated at, whatever
/// <see cref="LayoutSettings.MinRepulsionDistance"/> says.
/// </summary>
/// <remarks>
/// Small enough that it changes nothing for any usable setting, and positive so the division can
/// never be by zero.
/// </remarks>
private const double MinimumRepulsionClamp = 0.001;

/// <summary>
/// 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.
Expand Down Expand Up @@ -488,7 +504,7 @@
/// <param name="next">The chain to follow, indexed by edge.</param>
/// <param name="sharedAtTarget">True when the list is of edges arriving, false when leaving.</param>
/// <param name="strength">Force per unit of vertical swap still to be made.</param>
private void UntwistSharedEnds(int first, int[] next, bool sharedAtTarget, double strength)

Check warning on line 507 in ForceDirectedLayout/LayoutCore.cs

View workflow job for this annotation

GitHub Actions / Analyze & Release

Refactor this method to reduce its Cognitive Complexity from 24 to the 15 allowed.

Check warning on line 507 in ForceDirectedLayout/LayoutCore.cs

View workflow job for this annotation

GitHub Actions / Analyze & Release

Refactor this method to reduce its Cognitive Complexity from 24 to the 15 allowed.

Check warning on line 507 in ForceDirectedLayout/LayoutCore.cs

View workflow job for this annotation

GitHub Actions / Analyze & Release

Refactor this method to reduce its Cognitive Complexity from 24 to the 15 allowed.

Check warning on line 507 in ForceDirectedLayout/LayoutCore.cs

View workflow job for this annotation

GitHub Actions / Analyze & Release

Refactor this method to reduce its Cognitive Complexity from 24 to the 15 allowed.
{
for (int a = first; a >= 0; a = next[a])
{
Expand Down Expand Up @@ -688,7 +704,7 @@
/// and the pair comes to rest still overlapping, just less.
/// </para>
/// </remarks>
private void SeparateOverlaps()

Check warning on line 707 in ForceDirectedLayout/LayoutCore.cs

View workflow job for this annotation

GitHub Actions / Analyze & Release

Refactor this method to reduce its Cognitive Complexity from 47 to the 15 allowed.

Check warning on line 707 in ForceDirectedLayout/LayoutCore.cs

View workflow job for this annotation

GitHub Actions / Analyze & Release

Refactor this method to reduce its Cognitive Complexity from 47 to the 15 allowed.

Check warning on line 707 in ForceDirectedLayout/LayoutCore.cs

View workflow job for this annotation

GitHub Actions / Analyze & Release

Refactor this method to reduce its Cognitive Complexity from 47 to the 15 allowed.

Check warning on line 707 in ForceDirectedLayout/LayoutCore.cs

View workflow job for this annotation

GitHub Actions / Analyze & Release

Refactor this method to reduce its Cognitive Complexity from 47 to the 15 allowed.
{
double margin = Settings.OverlapMargin;
if (margin <= 0)
Expand Down
41 changes: 32 additions & 9 deletions ForceDirectedLayout/LayoutSettings.cs
Original file line number Diff line number Diff line change
Expand Up @@ -89,24 +89,47 @@ public struct LayoutSettings
public double MaxOverlapCorrection;

/// <summary>
/// Sensible defaults matching the previous Force&lt;float&gt;/Length&lt;float&gt; values, save for
/// <see cref="RepulsionStrength"/>, 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.
/// </summary>
/// <remarks>
/// These are measured rather than inherited. Six of them were moved by a coordinate descent over
/// the corpus in <c>tests/ForceDirectedLayout.Tests/Bench/</c>, scored by <c>LayoutScore</c>, 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.
/// <para>
/// 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 <see cref="GravityStrength"/>, which the descent left exactly where it was, but by
/// <see cref="LinkFlatteningStrength"/> 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.
/// </para>
/// <para>
/// 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.
/// </para>
/// </remarks>
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,
Expand Down
12 changes: 6 additions & 6 deletions ForceDirectedLayout/PhysicsSettings.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
/// </summary>
public double RepulsionStrength { get; init; } = 600_000.0;
public double RepulsionStrength { get; init; } = 900_000.0;

/// <summary>Dimensionless Hooke's-law spring constant for edges.</summary>
public double LinkSpringStrength { get; init; } = 0.5;
public double LinkSpringStrength { get; init; } = 0.1;

/// <summary>Strength of the horizontal source-left/target-right ordering bias. 0 disables it.</summary>
public double DirectionalBias { get; init; } = 0.5;
public double DirectionalBias { get; init; } = 4.0;

/// <summary>
/// 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 <see cref="LayoutCore.BezierClearanceRatio"/> for the clearance geometry.
/// </summary>
public double LinkFlatteningStrength { get; init; } = 0.5;
public double LinkFlatteningStrength { get; init; } = 3.0;

/// <summary>Extra horizontal clearance demanded on top of the derived bezier bound, in position units.</summary>
public double LinkFlatteningMargin { get; init; }
Expand All @@ -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.
/// </summary>
public double MinRepulsionDistance { get; init; } = 50.0;
public double MinRepulsionDistance { get; init; } = 5.0;

/// <summary>Spring rest length for edges.</summary>
public double RestLinkLength { get; init; } = 225.0;
public double RestLinkLength { get; init; } = 50.0;

/// <summary>Per-body force magnitude cap (applied before integration).</summary>
public double MaxForce { get; init; } = 5000.0;
Expand Down
49 changes: 27 additions & 22 deletions tests/ForceDirectedLayout.Tests/Bench/LayoutBenchTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -290,30 +290,33 @@ private sealed record Expectation(BenchGraph Graph, double MaxEdgeAngle, int Min
/// </summary>
/// <remarks>
/// 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.
/// <para>
/// Two of them are loose for a reason worth knowing, because it is a real defect and not a quirk
/// of the measurement. <see cref="GraphCorpus.Chain"/> 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
/// <see cref="LayoutSettings.GravityStrength"/> 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.
/// <see cref="GraphCorpus.MixedSizes"/> 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.
/// </para>
/// <code>
/// 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
/// </code>
/// <para>
/// 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.
/// <see cref="GraphCorpus.MixedSizes"/> 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
/// <see cref="LayoutSettings.GravityStrength"/> 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 <see cref="LayoutSettings.LinkFlatteningStrength"/> 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.
/// </para>
/// <para>
/// One threshold is still loose, and it is a real cost rather than a quirk:
/// <see cref="GraphCorpus.TwoClasses"/> 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.
/// </para>
/// </remarks>
[TestMethod]
Expand All @@ -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<BenchResult> rows = [];
Expand Down
2 changes: 1 addition & 1 deletion tests/ForceDirectedLayout.Tests/Bench/LayoutTuner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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 }),
Expand Down
18 changes: 16 additions & 2 deletions tests/ForceDirectedLayout.Tests/Bench/LayoutTunerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Loading