From 2e74a49797a1f83db7075d12ec16f203c95feb94 Mon Sep 17 00:00:00 2001 From: pullin Date: Sat, 25 Jul 2026 14:56:59 +0000 Subject: [PATCH] fix: maxInnerIterationsPerGrowthAttempt was silently ignored GrowShrinkHighDensityIntraNodeSolver bounds a single growth attempt by assigning `activeSubSolver.MAX_ITERATIONS`. PortfolioSingleIntraNodeSolver then recomputes MAX_ITERATIONS in refreshDynamicIterationLimit() from the remaining candidate budgets - on its first step, and again on every portfolio expansion - so the cap was discarded immediately. Observed on the added fixture: a cap of 25 becomes 45128. The parameter has therefore never had any effect on this path, and a growth attempt always ran to the portfolio's own dynamic budget before the node was grown and retried. Fix: an explicit `externalMaxIterations` ceiling on the portfolio that the dynamic limit respects, set by GrowShrink alongside the existing assignment. Tests: tests/features/never-fail-growth-high-density/caps-inner-iterations.test.ts covers both the wiring (a busy 4-connection node whose inner solver is still running after one step) and the portfolio ceiling itself. Both fail before this change and pass after. Suite: 419 pass / 55 skip / 2 fail; the two failures (bugreport36-d4c6c2 and "dip16 crossing traces 1206x4") reproduce identically on an unmodified checkout of this commit's parent, so they are pre-existing and unrelated. --- .../GrowShrinkHighDensityIntraNodeSolver.ts | 4 ++ .../PortfolioSingleIntraNodeSolver.ts | 14 +++++ .../caps-inner-iterations.test.ts | 63 +++++++++++++++++++ 3 files changed, 81 insertions(+) create mode 100644 tests/features/never-fail-growth-high-density/caps-inner-iterations.test.ts diff --git a/lib/solvers/HyperHighDensitySolver/GrowShrinkHighDensityIntraNodeSolver/GrowShrinkHighDensityIntraNodeSolver.ts b/lib/solvers/HyperHighDensitySolver/GrowShrinkHighDensityIntraNodeSolver/GrowShrinkHighDensityIntraNodeSolver.ts index 72d8fbc6a..5fc375727 100644 --- a/lib/solvers/HyperHighDensitySolver/GrowShrinkHighDensityIntraNodeSolver/GrowShrinkHighDensityIntraNodeSolver.ts +++ b/lib/solvers/HyperHighDensitySolver/GrowShrinkHighDensityIntraNodeSolver/GrowShrinkHighDensityIntraNodeSolver.ts @@ -148,6 +148,10 @@ export class GrowShrinkHighDensityIntraNodeSolver extends BaseSolver { ), }) if (this.constructorParams.maxInnerIterationsPerGrowthAttempt) { + // Must be an external ceiling: the portfolio recomputes MAX_ITERATIONS + // dynamically on its first step and would otherwise discard this. + this.activeSubSolver.externalMaxIterations = + this.constructorParams.maxInnerIterationsPerGrowthAttempt this.activeSubSolver.MAX_ITERATIONS = this.constructorParams.maxInnerIterationsPerGrowthAttempt } diff --git a/lib/solvers/HyperHighDensitySolver/PortfolioSingleIntraNodeSolver.ts b/lib/solvers/HyperHighDensitySolver/PortfolioSingleIntraNodeSolver.ts index ac7f4fbd6..0d8025144 100644 --- a/lib/solvers/HyperHighDensitySolver/PortfolioSingleIntraNodeSolver.ts +++ b/lib/solvers/HyperHighDensitySolver/PortfolioSingleIntraNodeSolver.ts @@ -279,6 +279,14 @@ export class PortfolioSingleIntraNodeSolver extends HyperParameterSupervisorSolv if (typeof setup === "function") setup.call(solver) } + /** + * Hard ceiling imposed by an owner, e.g. GrowShrink's + * maxInnerIterationsPerGrowthAttempt. Assigning MAX_ITERATIONS directly does + * not work: refreshDynamicIterationLimit() recomputes it from the remaining + * candidate budgets on the first step and would discard the cap. + */ + externalMaxIterations: number | null = null + private refreshDynamicIterationLimit() { const remainingSupervisorIterations = (this.supervisedSolvers ?? []).reduce( (total, { solver }) => { @@ -300,6 +308,12 @@ export class PortfolioSingleIntraNodeSolver extends HyperParameterSupervisorSolv this.iterations + 1, this.iterations + remainingSupervisorIterations, ) + if ( + this.externalMaxIterations !== null && + this.MAX_ITERATIONS > this.externalMaxIterations + ) { + this.MAX_ITERATIONS = this.externalMaxIterations + } this.stats.dynamicSupervisorIterationLimit = this.MAX_ITERATIONS } diff --git a/tests/features/never-fail-growth-high-density/caps-inner-iterations.test.ts b/tests/features/never-fail-growth-high-density/caps-inner-iterations.test.ts new file mode 100644 index 000000000..668a3bd16 --- /dev/null +++ b/tests/features/never-fail-growth-high-density/caps-inner-iterations.test.ts @@ -0,0 +1,63 @@ +import { expect, test } from "bun:test" +import { GrowShrinkHighDensityIntraNodeSolver } from "lib/solvers/HyperHighDensitySolver/GrowShrinkHighDensityIntraNodeSolver" +import { PortfolioSingleIntraNodeSolver } from "lib/solvers/HyperHighDensitySolver/PortfolioSingleIntraNodeSolver" +import type { NodeWithPortPoints } from "lib/types/high-density-types" +import { makeNode } from "./test-helpers" + +/** + * A node with four mutually crossing connections in a small area, so the inner + * portfolio is still running after one step and its budget can be inspected. + */ +const makeBusyNode = (): NodeWithPortPoints => ({ + capacityMeshNodeId: "cn_busy", + center: { x: 0, y: 0 }, + width: 1, + height: 1, + availableZ: [0, 1], + portPoints: [ + { connectionName: "a", x: -0.5, y: -0.3, z: 0 }, + { connectionName: "a", x: 0.5, y: 0.3, z: 0 }, + { connectionName: "b", x: -0.5, y: 0.3, z: 0 }, + { connectionName: "b", x: 0.5, y: -0.3, z: 0 }, + { connectionName: "c", x: -0.3, y: -0.5, z: 0 }, + { connectionName: "c", x: 0.3, y: 0.5, z: 0 }, + { connectionName: "d", x: 0.3, y: -0.5, z: 0 }, + { connectionName: "d", x: -0.3, y: 0.5, z: 0 }, + ], +}) + +/** + * `maxInnerIterationsPerGrowthAttempt` bounds how long a single growth attempt + * searches before the node is grown and retried. It was silently ineffective: + * GrowShrink assigned `activeSubSolver.MAX_ITERATIONS`, then the portfolio's + * `refreshDynamicIterationLimit()` recomputed that value from the remaining + * candidate budgets on its first step and discarded the cap (observed: a cap of + * 25 became 45128). + */ +test("maxInnerIterationsPerGrowthAttempt caps the inner solver's budget", () => { + const maxInnerIterationsPerGrowthAttempt = 25 + + const solver = new GrowShrinkHighDensityIntraNodeSolver({ + nodeWithPortPoints: makeBusyNode(), + maxInnerIterationsPerGrowthAttempt, + maxGrowthAttempts: 3, + }) + + solver.step() + + expect(solver.activeSubSolver).toBeTruthy() + expect(solver.activeSubSolver!.MAX_ITERATIONS).toBeLessThanOrEqual( + maxInnerIterationsPerGrowthAttempt, + ) +}) + +test("an external iteration ceiling survives the portfolio's dynamic limit", () => { + const solver = new PortfolioSingleIntraNodeSolver({ + nodeWithPortPoints: makeNode(), + }) + solver.externalMaxIterations = 25 + + solver.step() + + expect(solver.MAX_ITERATIONS).toBeLessThanOrEqual(25) +})