Skip to content

OptimizationPolyalgorithms: make imports explicit - #1415

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

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

Conversation

@ChrisRackauckas-Claude

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

Copy link
Copy Markdown
Member

OptimizationPolyalgorithms now imports the OptimizationBase, SciMLBase, OptimizationOptimJL, and OptimizationOptimisers module bindings and the individual names its implementation uses (OptimizationProblem, remake, Optimisers). The QA suite no longer marks no_implicit_imports as broken, and OptimizationPolyalgorithms.PolyOpt remains available.

Verification

Group env var confirmed in lib/OptimizationPolyalgorithms/test/runtests.jl as OPTIMIZATION_TEST_GROUP. Commands below ran from lib/OptimizationPolyalgorithms, 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/polyalg-imports/tmp
    export JULIA_DEPOT_PATH=/home/crackauc/sandbox/goals/qa-hygiene/jobs/polyalg-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 1m05.0s (Pass/Broken/Total); Testing OptimizationPolyalgorithms tests passed.

  • QA after change: same commands — output tail: Quality Assurance | 21 21 1m03.3s; Testing OptimizationPolyalgorithms tests passed.

  • Core:

    export TMPDIR=/home/crackauc/sandbox/goals/qa-hygiene/jobs/polyalg-imports/tmp
    export JULIA_DEPOT_PATH=/home/crackauc/sandbox/goals/qa-hygiene/jobs/polyalg-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 | 1 1 14.8s; Testing OptimizationPolyalgorithms tests passed.

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

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

  • Scanned lib/, test/, and docs/ for qualified OptimizationPolyalgorithms.x references. The only code use is OptimizationPolyalgorithms.PolyOpt (exported from this package); no other qualified bindings were required. ExplicitImports check_no_implicit_imports, check_no_stale_explicit_imports, and check_all_explicit_imports_via_owners all return ok on the changed source.

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 and QA status for OptimizationPolyalgorithms. Exports and solver behavior are unchanged. In-repo qualified use OptimizationPolyalgorithms.PolyOpt still resolves. Names that were only incidentally reachable as OptimizationPolyalgorithms.x through the old blanket using 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 1/1; Runic and typos exit 0; ExplicitImports no_implicit/no_stale/via_owners ok.
  • Independent review: pending
  • Merge: needs review

Cursor Agent CLI 2026.10.01-14929f9 (model: auto); transcript at /home/crackauc/sandbox/goals/qa-hygiene/jobs/polyalg-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: #1415 (comment)

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/polyalg-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

Scope: PR #1415 (head 5ba67b2, base ce2b3a0). It touches two files with +4/−6 lines: blanket using becomes explicit imports in lib/OptimizationPolyalgorithms/src/OptimizationPolyalgorithms.jl, and ei_broken = (:no_implicit_imports,) plus its explanatory comment are removed from lib/OptimizationPolyalgorithms/test/qa/qa.jl. It adds no dependency, removes no export, and makes no change to solver logic. The PR links no issue (closingIssuesReferences is empty). The repo has no AGENTS.md or CLAUDE.md at its root.

Blocking findings

None.

Non-blocking findings

  1. Some names are no longer reachable through the module (confirmed by running). On master, OptimizationPolyalgorithms.solve, .Optim, .OptimizationFunction, .Adam, .BFGS and .AutoForwardDiff resolved via the blanket using. With this PR they do not. My probe printed reachable on master but not PR: [:solve, :Optim, :OptimizationFunction, :Adam, :BFGS, :AutoForwardDiff]. None of these names were exported or documented, so this is not a SemVer break, and the PR body says so. I searched GitHub code for OptimizationPolyalgorithms. (gh search code) and fetched the hits: every external use is OptimizationPolyalgorithms.PolyOpt (claeys01/Thesis, sosiristseng/jl-ude, AdriaCoding/ADM-Notebook, nichomueller/Opal.jl), and that still resolves.
  2. Existing dead branch, not introduced here (confirmed by running). SciMLBase.__solve has a branch for a problem without bounds (BFGS-only), but a bounded problem never reaches the solver. OptimizationBase.solve(prob_with_lb_ub, PolyOpt(), maxiters=1000) throws The algorithm PolyOpt does not support box constraints because PolyOpt defines no allowsbounds. The same happens on master's source, which I loaded as module OldPoly. This is out of scope for an import-hygiene PR, but worth a separate issue.
  3. Thin test coverage, not introduced here. test/core_tests.jl exercises only the deterministic, no-bounds, maxiters path (Adam then BFGS). To compensate, I exercised the maxiters === nothing path, the non-deterministic Adam(0.1) path and the "requires maxiters" error path against the PR source myself (see below). All of them use the newly explicit bindings (Optimisers, OptimizationOptimJL.Optim, remake, OptimizationBase.solve) and pass.
  4. Attribution and process (from reading). The PR body has no 🚧 UNREVIEWED banner as its first line and no 🤖 Posted by an AI agent footer in the form Chris's global CLAUDE.md asks for. It has a "Please ignore this draft" line and a Cursor footer instead. The commit trailer says Agent-Model: auto, which is not an exact model ID. Neither affects the code.
  5. Two red CI jobs, both already red on master. ModelingToolkit.jl/Optimization / Downstream Tests - Optimization and downgrade-sublibraries / test (lib/OptimizationReactant) fail on this PR. The same two jobs also fail on the master base commit ce2b3a0 (IntegrationTest run 38076978987, Downgrade Sublibraries run 38076978819) and on sibling PRs OptimizationIpopt: make imports explicit #1413, SimpleOptimization: make imports explicit #1416 and OptimizationMadNLP: make imports explicit #1417. All OptimizationPolyalgorithms jobs (Core lts/1/pre, QA, downgrade) are green.

What I ran

All runs used TMPDIR=.../SciML_Optimization.jl-1415/tmp, Julia 1.12.7 (the juliaup default on this host) and the default depot.

  • OPTIMIZATION_TEST_GROUP=QA julia --project -e 'using Pkg; Pkg.test()' in lib/OptimizationPolyalgorithms gave Quality Assurance | 21 21, tests passed, exit 0. This matches the PR's 21/21. The PR's run was Julia 1.11 on amdci2, and I did not rerun its 20+1-broken baseline through Pkg.test. The direct check of master's source below shows the same thing.
  • OPTIMIZATION_TEST_GROUP=Core ... Pkg.test() gave Core | 1 1, exit 0.
  • An ExplicitImports probe (scratch/probe.jl, in an env with the lib/ packages dev'd plus ExplicitImports 1.15 and ForwardDiff):
    • PR source: check_no_implicit_imports, check_no_stale_explicit_imports, check_all_explicit_imports_via_owners and check_all_explicit_imports_are_public all pass.
    • Master source, loaded as OldPoly: check_no_implicit_imports throws ImplicitImportsException on OptimizationBase, OptimizationOptimJL, OptimizationOptimisers, Optimisers, SciMLBase, OptimizationProblem, remake, exactly the set the PR now imports. So the newly enforced QA check is not vacuous: it fails on the old code and passes on the new.
    • Behaviour of the PR source on Rosenbrock with p=[1,100], u0=0:
      • PolyOpt() without maxiters gives u ≈ [1,1] within atol 1e-4, the analytic minimum.
      • A noisy loss with maxiters=2000 takes the Adam(0.1) branch and gives an objective below l0/10.
      • A noisy loss without maxiters throws ErrorException.
      • Result: 3/3 pass.
    • Bounded problem: identical box-constraint error on both PR and master (finding 2).
  • I confirmed OptimizationOptimisers does export Optimisers and OptimizationOptimJL does export Optim. The sibling PRs QA: make OptimizationOptimisers imports explicit #1397, /1392 and /1391 (all still open) do not remove those exports, so using OptimizationOptimisers: Optimisers and OptimizationOptimJL.Optim stay valid if those PRs land.
  • julia +1.12 --project=@runic -m Runic --check --diff on both changed files: exit 0. typos on both files: exit 0.
  • gh pr view 1415 --json statusCheckRollup, plus the master runs listed above, for the CI comparison.
  • git status after the test runs is clean. The Manifest.toml that Pkg.test writes is gitignored.

What I did not verify

  • Julia 1.10 (lts) locally. CI's Tests - Core (Julia lts) for this sublibrary is green, but I did not read its log.
  • Downstream packages beyond the GitHub code search and the existing CI downstream jobs (DiffEqFlux and NeuralPDE are green; MTK's failure predates this PR, and I did not inspect its log).
  • The PR's 20-pass/1-broken QA baseline through Pkg.test on master. I instead checked master's source directly with ExplicitImports.

Links


🤖 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 12:08
@ChrisRackauckas
ChrisRackauckas merged commit 357c9bd 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