Skip to content

OptimizationLBFGSB: make imports explicit - #1411

Merged
ChrisRackauckas merged 1 commit into
SciML:masterfrom
ChrisRackauckas-Claude:qa/lbfgsb-explicit-imports
Oct 11, 2026
Merged

ChrisRackauckas merged 1 commit into
SciML:masterfrom
ChrisRackauckas-Claude:qa/lbfgsb-explicit-imports

Conversation

@ChrisRackauckas-Claude

@ChrisRackauckas-Claude ChrisRackauckas-Claude commented Oct 10, 2026 •

Copy link
Copy Markdown
Member

OptimizationLBFGSB now imports the OptimizationBase, SciMLBase, and DocStringExtensions names its implementation uses (plus the module bindings needed for qualified calls). Unused Reexport was dropped from the package deps because nothing called @reexport. The QA suite no longer marks no_implicit_imports as broken, and OptimizationLBFGSB.LBFGSB remains available.

Verification

Group env var confirmed in lib/OptimizationLBFGSB/test/runtests.jl as OPTIMIZATION_TEST_GROUP. Commands below ran from lib/OptimizationLBFGSB, with the Julia depot and temporary directory isolated under this job directory.

  • Baseline QA, before changes:

    export TMPDIR=/home/crackauc/sandbox/goals/qa-hygiene/jobs/lbfgsb-imports/tmp
    export JULIA_DEPOT_PATH=/home/crackauc/sandbox/goals/qa-hygiene/jobs/lbfgsb-imports/scratch/julia-depot
    export JULIA_CONDAPKG_BACKEND=Null JULIA_PYTHONCALL_EXE=/usr/bin/python3
    ulimit -n 65536
    OPTIMIZATION_TEST_GROUP=QA julia --project -e 'using Pkg; Pkg.test()'

    Output tail: Quality Assurance | 20 1 21 1m03.5s; Testing OptimizationLBFGSB tests passed.

  • QA after change: same commands — output tail: Quality Assurance | 21 21 1m02.9s; Testing OptimizationLBFGSB tests passed.

  • Core:

    export TMPDIR=/home/crackauc/sandbox/goals/qa-hygiene/jobs/lbfgsb-imports/tmp
    export JULIA_DEPOT_PATH=/home/crackauc/sandbox/goals/qa-hygiene/jobs/lbfgsb-imports/scratch/julia-depot
    export JULIA_CONDAPKG_BACKEND=Null JULIA_PYTHONCALL_EXE=/usr/bin/python3
    ulimit -n 65536
    OPTIMIZATION_TEST_GROUP=Core julia --project -e 'using Pkg; Pkg.test()'

    Output tail: Core | 8 8 58.2s; Testing OptimizationLBFGSB tests passed.

  • From the repository root, julia --project=@runic -e 'using Runic; exit(Runic.main(ARGS))' -- --inplace lib/OptimizationLBFGSB/src/OptimizationLBFGSB.jl lib/OptimizationLBFGSB/test/qa/qa.jl — exit 0.

  • From the repository root, typos lib/OptimizationLBFGSB/src/OptimizationLBFGSB.jl lib/OptimizationLBFGSB/test/qa/qa.jl — exit 0.

  • Scanned lib/, test/, and docs/ for qualified OptimizationLBFGSB.x references. The only code use is OptimizationLBFGSB.LBFGSB (exported from this package); no other qualified bindings were required.

Not verified: CI, the full Optimization.jl monorepo suite, and downstream packages.

Please ignore this draft until reviewed by @ChrisRackauckas.

Risk assessment

  • Risk: low
  • Blast radius: imports, unused Reexport dep removal, and QA status for OptimizationLBFGSB. Exports and solver behavior are unchanged. In-repo qualified use OptimizationLBFGSB.LBFGSB still resolves. Names that were only incidentally reachable as OptimizationLBFGSB.x through the old blanket using (and unused Reexport) no longer are; nothing in lib/, test/ or docs/ uses them, but downstream code spelling them that way would break.
  • Evidence: QA changed from 20 pass / 1 broken to 21 pass; Core passes 8/8; Runic and typos exit 0.
  • Independent review: pending
  • Merge: needs review

Cursor Agent CLI 2026.10.01-14929f9 (model: auto); transcript at /home/crackauc/sandbox/goals/qa-hygiene/jobs/lbfgsb-imports/log.txt on amdci2

Made with Cursor

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

Drop unused Reexport and un-break no_implicit_imports so QA enforces explicit imports.

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
Co-Authored-By: Cursor Agent <noreply@cursor.com>
Agent-Harness: Cursor Agent CLI 2026.10.01-14929f9
Agent-Model: auto
Agent-Session: local session, transcript at /home/crackauc/sandbox/goals/qa-hygiene/jobs/lbfgsb-imports/log.txt on amdci2
@ChrisRackauckas

Copy link
Copy Markdown
Member

🤖 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: #1411 (head e8f7389, base origin/master ce2b3a0). No linked issue and no parent/tracking issue (closingIssuesReferences is empty, and the body has no Fixes #). The repo root has no AGENTS.md or CLAUDE.md.

Blocking findings

None.

Non-blocking findings

  1. The lts QA failure exists on master too; this PR did not cause it (confirmed by running). lib/OptimizationLBFGSB/test/test_groups.toml puts QA on ["lts", "1"]. On Julia 1.10.12 the QA group fails on both master and PR head with the same two failures:

    • JET typo-mode reports "9 toplevel errors". It treats LBFGSB as the module that import LBFGSB as LBFGSBJL imports, which gives "invalid redefinition of constant OptimizationLBFGSB.LBFGSB" and "TypeVar upper bound expected Type, got Module".
    • "public API is rendered in docs" fails. This second one may come from my archive copy (see "What I did not verify").

    master gives 16 pass / 2 fail / 1 broken; head gives 17 pass / 2 fail / 0 broken. So on lts, too, the only change is that no_implicit_imports goes from broken to passing. The PR body only reports QA on one Julia version, without naming it. This is worth a separate issue, not a change to this PR.

  2. Mixed qualified and unqualified names (from reading). src/OptimizationLBFGSB.jl:3 now imports OptimizationCache, but __map_optimizer_args (line ~45) still writes OptimizationBase.OptimizationCache, while __solve uses the bare name. Similarly, OptimizationStats is imported from SciMLBase but OptimizationBase.OptimizationStats is still used near the end of the file (that one predates this PR). It's cosmetic and ExplicitImports accepts it.

  3. Names reachable through the old blanket using OptimizationBase are gone (confirmed by running). After the PR, isdefined(OptimizationLBFGSB, :OptimizationFunction/:OptimizationProblem/:solve/:Reexport) are all false, and names(OptimizationLBFGSB) == [:LBFGSB, :OptimizationLBFGSB] (the exports haven't changed). These were never public, and in this repo the only qualified access is OptimizationLBFGSB.LBFGSB (test/AD, lib/OptimizationNLPModels tests, docs). The PR body states this risk. Out-of-repo users were not checked.

  4. Attribution format (from reading). Under the machine-level agent rules, the PR body is missing the leading "UNREVIEWED" banner. It does carry a harness/model/transcript line, and the commit has the Agent-* trailers. The PR is a draft.

  5. Sibling fit (confirmed by reading the PR diffs). The pattern matches the merged OptimizationNLPModels: make imports explicit #1409 and OptimizationNLopt: make imports explicit #1398: replace blanket using with using X: X, names..., remove ei_broken = (:no_implicit_imports,) and its comment, no version bump. The open Remove Core.Box captures from the unconstrained OptimizationLBFGSB __solve path #1402 edits the same file but only in hunks at line 90 and later, and adds no new unqualified OptimizationBase/SciMLBase names. I expect no textual conflict and no new implicit-import failure, whichever merges first.

What I ran

All from the PR head checkout or from git archive copies under scratch/, with TMPDIR set to the review tmp dir.

  • OPTIMIZATION_TEST_GROUP=QA julia +1.12 --project -e 'using Pkg; Pkg.test()' in lib/OptimizationLBFGSB: Quality Assurance | 21 21, tests passed. This reproduces the PR body's "21 pass". The log shows OptimizationLBFGSB and OptimizationBase dev'd from the checkout paths, with ExplicitImports v1.15.0 and SciMLTesting v2.13.2.
  • Baseline, on a git archive origin/master copy (lib/OptimizationBase, lib/OptimizationLBFGSB, test/qa, docs), QA on 1.12: 1 broken, which is the no_implicit_imports marker. So the check that the PR un-breaks really was failing before. The run also showed 1 failure in the rendered-docs check; I attribute it to the copy, not to master (see "What I did not verify").
  • OPTIMIZATION_TEST_GROUP=Core julia +1.12 ... in the checkout: Core | 8 8, passed. That matches the PR body.
  • OPTIMIZATION_TEST_GROUP=Core julia +lts ... on a PR-head archive copy: Core | 8 8, passed.
  • OPTIMIZATION_TEST_GROUP=QA julia +lts ... on the master and PR-head archive copies (each git inited): master 16/2 fail/1 broken, head 17/2 fail/0 broken. The failures are identical: JET toplevel and rendered docs (finding 1).
  • scratch/ei_probe.jl on the PR head with Julia 1.12 runs ExplicitImports directly. check_no_implicit_imports, check_no_stale_explicit_imports and check_all_explicit_imports_via_owners all pass. explicit_imports_nonrecursive is empty and there are no stale imports. The isdefined/names results are in finding 3.
  • julia +1.12 --project=@runic -m Runic --check --diff on both changed .jl files: exit 0. git diff origin/master...HEAD | typos -: exit 0.
  • Repo grep for OptimizationLBFGSB\. and Reexport: the only qualified use of this package is OptimizationLBFGSB.LBFGSB, and nothing in lib/OptimizationLBFGSB references Reexport any more. lib/OptimizationLBFGSB/test/qa/Project.toml never listed Reexport.
  • gh pr list for open PRs touching LBFGSB or "explicit imports", and gh pr diff 1402 / gh pr diff 1403 --name-only.

The diff makes no numerical or algorithmic change. Only the import block, the [deps]/[compat] entries for Reexport, and the QA ei_broken marker change, so the review checklist's numerics and test-integrity items don't apply. No test was weakened: the only test-file change removes a broken marker, which makes the check stricter.

What I did not verify

  • QA on lts in the real git checkout. My lts QA runs used git archive copies. In those, the "public API is rendered in docs" check fails on both master and head, while in the real checkout on 1.12 it passes. That suggests the failure comes from the copy, but I didn't confirm it. The JET lts failure doesn't depend on docs layout and shows up identically on master.
  • Core on pre (listed in test_groups.toml for Core); CI; the monorepo-wide test/AD and NLPModels suites that call OptimizationLBFGSB.LBFGSB (that name is still exported and resolvable, as confirmed above).
  • Downstream packages outside this repo that might use the now-unreachable OptimizationLBFGSB.<OptimizationBase name> paths.
  • The author's own transcript on amdci2, or that their baseline was run on the same Julia version as mine.

🤖 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)

@ChrisRackauckas
ChrisRackauckas marked this pull request as ready for review October 11, 2026 10:31
@ChrisRackauckas
ChrisRackauckas merged commit a5f6c27 into SciML:master Oct 11, 2026
54 of 56 checks passed
ChrisRackauckas referenced this pull request Oct 14, 2026
…ationLBFGSB, OptimizationMOI, OptimizationNLopt, OptimizationPolyalgorithms, and SimpleOptimization for release (#1423)

Agent-Harness: Devin CLI 3000.11.3
Agent-Model: Fusion (Claude Opus 5.5 Medium + SWE-2 Medium)
Agent-Session: local transcript ~/.local/share/devin/cli/transcripts/flash-hour.json

Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants