Remove Core.Box captures from the unconstrained OptimizationLBFGSB __solve path

[ChrisRackauckas-Claude]

Summary

SciMLBase.__solve for LBFGSB declared local x and assigned iter_count / _loss in both the constrained and unconstrained branches, so Julia boxed those captures. Every objective evaluation then went through Core.Box, which showed up as dynamic dispatch (JET: 23 reports in the module) and extra allocations. This drops local x (so x is local to each closure) and renames the unconstrained-branch iter_count / _loss to ub_iter_count / ub_loss so the dual-branch assignment no longer forces boxing. Behavior is unchanged: u, objective, iteration/feval/geval counts, and callback sequences are bit-identical before vs after on Rosenbrock n=2/10/100 (Julia 1.12.4 and 1.10.12).

Regression test (fail before / pass after)

Unconstrained __solve typed IR must contain zero Core.Box SSA values (lib/OptimizationLBFGSB/test/core_tests.jl).

Before (src stashed; nboxes = 2):

=== Julia 1.12.4 (unfixed src, regression test) ===
has local x: true
nboxes = 2
Test Failed
  Expression: nboxes == 0
   Evaluated: 2 == 0

=== Julia 1.10.12 (unfixed src, regression test) ===
has local x: true
nboxes = 2
Test Failed
  Expression: nboxes == 0
   Evaluated: 2 == 0

After (nboxes = 0):

=== Julia 1.12.4 (fixed) ===
nboxes = 0
Test Summary:                               | Pass  Total
no Core.Box in unconstrained LBFGSB __solve |    1      1

=== Julia 1.10.12 (fixed) ===
nboxes = 0
Test Summary:                               | Pass  Total
no Core.Box in unconstrained LBFGSB __solve |    1      1

Before / after performance

Same machine (amdci2), JULIA_NUM_THREADS=1 taskset -c <core>, BenchmarkTools minima, Rosenbrock with AutoForwardDiff.

Julia	n	before (min µs / allocs)	after (min µs / allocs)
1.12.4	2	274.6 / 481	224.1 / 206
1.12.4	10	870.0 / 1275	721.4 / 458
1.12.4	100	18021.9 / 9842	16924.5 / 3256
1.10.12	2	448.4 / 563	401.4 / 263
1.10.12	10	1393.2 / 1595	1243.8 / 701
1.10.12	100	22715.5 / 12431	21539.4 / 5244

JET reports targeting OptimizationLBFGSB: 23 → 11 on both 1.12.4 and 1.10.12.

Correctness fingerprint (u, objective, iters/fevals/gevals, full callback sequence) is byte-identical before vs after on both Julias for n=2/10/100.

Test-group tails

OPTIMIZATION_TEST_GROUP=Core via Pkg.test on lib/OptimizationLBFGSB (Julia 1.12.4):

Test Summary: | Pass  Total  Time
Core          |    9      9  46.5s
Testing OptimizationLBFGSB tests passed

OPTIMIZATION_TEST_GROUP=QA on the same sublibrary:

Test Summary:     | Pass  Broken  Total  Time
Quality Assurance |   20       1     21  56.4s
Testing OptimizationLBFGSB tests passed

Runic --check and typos clean on the two touched files (already Runic-clean on master; only changed lines reformatted as needed).

Not verified

• Constrained (cache.f.cons !== nothing) path still reassigns λ/μ/ρ/θ captured by closures; remaining JET reports (11) were not chased.
• No version bump (no new public API).
• Root GROUP=QA / GROUP=All not run; only OptimizationLBFGSB Core + QA.
• Did not re-benchmark against raw LBFGSB.lbfgsb in this job (audit already had parity after the same patch).

Reviewer pushback

• Whether renaming only the unconstrained branch is enough long-term, vs extracting each branch behind a function barrier (would also unbox the constrained AugLag path).
• Whether the regression should assert an allocation bound instead of / in addition to Core.Box SSA count (SSA shape can change across Julia versions; it held on 1.10 and 1.12 here).
• Constrained-path boxes left in place intentionally for a minimal diff.

Please ignore until reviewed by @ChrisRackauckas.

Risk assessment

Independent review: Claude Code (claude-opus-5-5) MERGE, risk low (confidence high): u/objective/retcode/iteration counts/callback-sequence hash byte-identical to master across unconstrained, bounded, equality and mixed-constraint cases; n=2 275.1 -> 224.0 us (481 -> 206 allocs), n=10 870 -> 718 us, constrained 90.4 -> 75.7 ms; JET 23 -> 11 (unconstrained). Scope note: the boxes are removed on the unconstrained path; the augmented-Lagrangian branch still captures reassigned λ/μ/ρ (21 Core.Box, from 23).

🤖 Generated with Cursor Agent 2026.10.01-14929f9 (model: unknown (Cursor auto)), transcript /home/crackauc/sandbox/goals/performance/jobs/opt/opt-lbfgsb/log.txt on amdci2; orchestrated by Claude Code (claude-opus-5-5[1m]) https://claude.ai/code/session_01LPHREnnonfLg1VcE1EJovv

Made with Cursor

Independent review: Devin Fusion (fusion-claude-opus-5-5-high-sidekick-swe-2-medium) rated it low, verdict MERGE: #1402 (comment)

[ChrisRackauckas]

🤖 Automated comment from an AI agent running as @ChrisRackauckas — not written or reviewed by Chris.

Independent review (Devin CLI 3000.11.3, model fusion-claude-opus-5-5-high-sidekick-swe-2-medium): MERGE, risk low.

Full review

VERDICT: MERGE
RISK: low

PR: #1402 (head c41cafa, 2 files, +15/−6). The PR links no issue and has no parent tracking issue.

Blocking findings

None.

Non-blocking findings

1. The _loss → ub_loss rename does nothing; the PR body and commit message say it is needed (confirmed by running). I built a variant from master with only local x dropped and iter_count renamed in the unconstrained branch, with _loss left alone (scratch/var). On 1.12 and on 1.10 it matches the PR exactly: the same Core.Box counts (0 unconstrained/bounded; 21 on 1.12, 5 on 1.10 constrained), the same allocations (208 / 460 for n=2 / n=10 on 1.12) and the same callback hashes. _loss is never captured by a closure, so assigning it in both branches never caused a box. Only x and iter_count did, which is why the count goes 2 → 0.

   • Suggested change, lib/OptimizationLBFGSB/src/OptimizationLBFGSB.jl:221-226: in the unconstrained branch the counter is only ever incremented and never read, so the cleanest fix is to delete iter_count = Ref(0) and iter_count[] += 1 there instead of renaming them. Keep _loss as it was.
   • The ub_ prefix is also misleading in this file. ub already means "upper bound" here (cache.ub, solver_kwargs.ub), and this branch handles bounded problems too.

2. The test comment names variables that no longer exist (lib/OptimizationLBFGSB/test/core_tests.jl:99-100). The comment talks about "dual-branch iter_count/_loss", but after the rename neither name exists in that branch, so the comment describes the old code. A one-line comment would do, e.g. "closures in the unconstrained __solve path must not capture boxed variables".

3. The regression test only covers the unconstrained path without bounds (core_tests.jl:101-107). It passes typeof(cache) with f.cons === nothing, so the constrained branch is compiled away. That branch still has 21 boxes on 1.12 (5 on 1.10), and the PR body says so openly. Checking the unboxed path this way is reasonable. Counting SSA values typed Core.Box depends on what the compiler emits, but CI passed on lts, 1 and pre, and the test failed against master src here, so the assertion does work.

4. Problems that already exist on master, not caused by this PR (confirmed by running; identical on master and the PR, on both Julia versions):

   • zeros(Float32, 2) as u0 throws MethodError: Cannot convert Vector{Float32} to Float64.
   • The mixed eq/ineq augmented-Lagrangian case diverges to u ≈ [-1.1e140, 1.1e140], obj = Inf, MaxIters.
   • The constrained-branch stats report iterations = fevals = maxiters regardless of what actually ran.

   These are worth separate issues, not changes to this PR.

5. The benchmark numbers in the PR body are from amdci2 and are not reproducible here, but the trend reproduces (confirmed by running on an M2 Max, 1 thread, BenchmarkTools minimum):

   Julia	n	master (min / allocs)	PR (min / allocs)
   1.12.7	2	125.3 µs / 483	104.0 µs / 208
   1.12.7	10	417.8 µs / 1277	354.6 µs / 460
   1.10.12	2	155.1 µs / 563	128.1 µs / 263
   1.10.12	10	516.4 µs / 1595	434.3 µs / 701

   The allocation counts are within 2 of the PR body's figures.

6. The two red CI checks are not caused by this PR. ModelingToolkit.jl/Optimization downstream and downgrade-sublibraries (lib/OptimizationReactant) fail the same way on master, e.g. https://github.com/SciML/Optimization.jl/actions/runs/37929754135 and https://github.com/SciML/Optimization.jl/actions/runs/37929754213. Every OptimizationLBFGSB sublibrary job is green (Core on lts/1/pre, QA, downgrade).

7. No conflict with the one other open PR on this sublibrary. OptimizationLBFGSB: make imports explicit #1411 (explicit imports) only touches the import block (lines 1-17), Project.toml and qa.jl, and __solve uses no names it removes.

8. Rule checks pass. There are no new dependencies, no new exports and no public-API change, and the PR has one concern. The internal LBFGSBJL._opt_bounds call was already on master and is not introduced here. Without an API change, a patch release without a version bump in this PR is fine.

What I ran

All runs used TMPDIR and scratch under .../SciML_Optimization.jl-1402/. I made copies of lib/{OptimizationLBFGSB,OptimizationBase} in three variants: pr (PR head), base (master src plus the PR's new test file) and var (master minus local x, with only iter_count renamed). The checkout itself was not touched.

• scratch/probe.jl on each variant × Julia 1.12.7 and 1.10.12. For each case it records the Core.Box count in typed IR, u, objective, retcode, iterations/fevals, the number of callback calls and a hash of the full callback (u, loss) sequence. The cases are: unconstrained n=2 and n=10, bounded, equality-constrained, mixed eq/ineq, and Float32. It also checks that a callback halt still raises, and benchmarks n=2/10.
  • Every case gives identical u, objective, retcode, iterations, fevals and callback hash across master, PR and variant on both Julia versions.
  • Boxes, master → PR: 2 → 0 for unconstrained/bounded; 23 → 21 (1.12) and 7 → 5 (1.10) for constrained.
• OPTIMIZATION_TEST_GROUP=Core Pkg.test() on lib/OptimizationLBFGSB:
  • PR, 1.12: 9/9 pass.
  • PR, 1.10: 9/9 pass.
  • master src with the new test, 1.12: 8 pass, 1 fail. The failure is no Core.Box in unconstrained LBFGSB __solve at core_tests.jl:107, so the test fails without the fix.
• OPTIMIZATION_TEST_GROUP=QA Pkg.test() on the PR, 1.12: 19 pass, 1 broken (the no_implicit_imports that qa.jl declares broken), 1 fail. The failure is "public API is rendered in docs: [:LBFGSB]". It looks like a side effect of running QA from a scratch copy that has the root test/ (symlinked in) but not the root docs/. The real docs page (docs/src/optimization_packages/lbfgsb.md) exists, and the PR's CI QA job for this sublibrary is green. I am treating it as not a finding, but I only inferred that; I did not confirm it.
• gh reads: the PR's checks, master's failing runs, and open PRs touching lib/OptimizationLBFGSB.

What I did not verify

• The JET report counts (23 → 11) in the PR body. I did not run JET outside the QA group.
• The exact amdci2 timings in the PR body (different machine).
• Runic and typos locally. I relied on CI's green Runic and Typos checks.
• Root-package test groups.

---

🤖 Posted by an AI agent — harness: Devin CLI 3000.11.3 (review), Claude Code 2.1.285 (posting) · model: fusion-claude-opus-5-5-high-sidekick-swe-2-medium
Conversation: local Claude session 3c311569-dda6-4687-bc9f-65febb212c33 (fleet master)