Skip to content

OptimizationIpopt: make imports explicit - #1413

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

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

Conversation

@ChrisRackauckas-Claude

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

Copy link
Copy Markdown
Member

OptimizationIpopt now imports the Ipopt, OptimizationBase, SciMLBase, and SparseArrays module bindings and the individual names its implementation uses (OptimizationCache, ReturnCode, SparseMatrixCSC, findnz, nonzeros). Unused using Reexport / LinearAlgebra / SymbolicIndexingInterface lines were dropped (and the unused Reexport dependency removed so Aqua stale-deps stays green). The QA suite no longer marks no_implicit_imports as broken. In-repo qualified uses are only OptimizationIpopt.IpoptOptimizer and OptimizationIpopt.IpoptEvaluator (docs); both remain defined here.

Verification

Group env var confirmed in lib/OptimizationIpopt/test/runtests.jl as OPTIMIZATION_TEST_GROUP. Commands ran from lib/OptimizationIpopt with job-local TMPDIR / JULIA_DEPOT_PATH, JULIA_CONDAPKG_BACKEND=Null, JULIA_PYTHONCALL_EXE=/usr/bin/python3, and ulimit -n 65536.

  • Baseline QA, before changes:
    OPTIMIZATION_TEST_GROUP=QA julia --project -e 'using Pkg; Pkg.test()'
    Output tail: Quality Assurance | 20 1 21 1m05.3s; Testing OptimizationIpopt tests passed.
  • QA after change: same command — output tail: Quality Assurance | 21 21 1m03.4s; Testing OptimizationIpopt tests passed.
  • Core after change:
    OPTIMIZATION_TEST_GROUP=Core julia --project -e 'using Pkg; Pkg.test()'
    Output tail: Core | 131 1 132 6m09.9s; Testing OptimizationIpopt tests passed (the one broken is pre-existing, unrelated to imports).
  • From the repository root, julia --project=@runic -e 'using Runic; exit(Runic.main(ARGS))' -- --inplace lib/OptimizationIpopt/src/OptimizationIpopt.jl lib/OptimizationIpopt/test/qa/qa.jl — exit 0.
  • From the repository root, typos lib/OptimizationIpopt/src/OptimizationIpopt.jl lib/OptimizationIpopt/test/qa/qa.jl — exit 0.
  • ExplicitImports after the change: check_no_implicit_imports and check_no_stale_explicit_imports return ok.
  • Scanned lib/, test/, and docs/ for qualified OptimizationIpopt.x references; only docs cite IpoptOptimizer / IpoptEvaluator.

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 OptimizationIpopt. Exports and solver behavior are unchanged. Names that were only incidentally reachable as OptimizationIpopt.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 / 0 broken; Core passes 131 with 1 pre-existing broken; Runic and typos exit 0; ExplicitImports reports no implicit or stale imports.
  • Independent review: pending
  • Merge: needs review

Cursor Agent CLI 2026.10.01-14929f9 (model: auto); transcript: /home/crackauc/sandbox/goals/qa-hygiene/jobs/ipopt-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: #1413 (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/ipopt-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: #1413 (OptimizationIpopt: make imports explicit). No linked issue. It is one of a series of "make imports explicit" PRs (merged siblings #1409 and #1398), and it follows their pattern: module binding plus named imports, and drop the ei_broken = (:no_implicit_imports,) marker.

Blocking findings

None.

Non-blocking findings

  1. lib/OptimizationIpopt/Project.toml:8,13: after this PR, LinearAlgebra and SymbolicIndexingInterface stay in [deps]/[compat], but nothing in src/ uses them. The PR removes their using lines and leaves the deps. Confirmed by running: a grep of src/ finds no LinearAlgebra/SII names or qualified accesses, and Aqua.test_stale_deps(OptimizationIpopt) still passes. It passes only because Aqua counts any package that gets loaded, even transitively, and OptimizationBase depends on both. After using OptimizationIpopt both modules show up in Base.loaded_modules, but Reexport does not. So the PR's reason for dropping Reexport ("so Aqua stale-deps stays green") holds. The same logic says these two deps should go as well, either here or in a follow-up. CI does not depend on this.
  2. src/OptimizationIpopt.jl:7: using SparseArrays: SparseArrays, ... brings in the SparseArrays module binding, but src never uses SparseArrays.x. This is harmless, and ExplicitImports does not flag it.
  3. Breaking-change exposure, suspected from reading only: names that came in through the old blanket using SciMLBase/OptimizationBase/LinearAlgebra/SymbolicIndexingInterface/Reexport could be reached as OptimizationIpopt.<name>. They no longer can. The PR body says this. The only in-repo uses are OptimizationIpopt.IpoptOptimizer/IpoptEvaluator in docs/src/optimization_packages/ipopt.md, and both are still defined. This matches how the merged siblings handled it, so I am not asking for a version bump.

What I ran

All runs used lib/OptimizationIpopt, Julia 1.12.7, and the PR head 5ec45d4.

  • OPTIMIZATION_TEST_GROUP=QA julia --project -e 'using Pkg; Pkg.test()' → Quality Assurance | 21 21, tests passed. This matches the PR body's 21/21. Confirmed by running.
  • OPTIMIZATION_TEST_GROUP=Core julia --project -e 'using Pkg; Pkg.test()' → Core | 131 1 132, tests passed. The one Broken is @test_broken at test/core_tests.jl:197, and that line is identical on origin/master. The PR does not touch test files apart from qa.jl. Confirmed by running.
  • ExplicitImports directly on the PR head: check_no_implicit_imports and check_no_stale_explicit_imports both return nothing, and improper_explicit_imports and stale_explicit_imports are both empty. That justifies removing ei_broken. The non-public accesses that remain are all on the all_qualified_accesses_are_public ignore list that already exists. The PR leaves that list unchanged.
  • Aqua stale-deps probe:
    • PR head: passes.
    • A copy with Reexport added back to [deps]/[compat]: fails at Aqua/src/stale_deps.jl:32 and names Reexport. So removing it was required.
    • I checked by loading the package that LinearAlgebra and SII are loaded transitively (finding 1).
  • julia +1.12 --project=@runic -m Runic --check on the two changed .jl files → exit 0. typos lib/OptimizationIpopt → exit 0.
  • I grepped the repo for OptimizationIpopt.<name> and found only docs and README hits for IpoptOptimizer/IpoptEvaluator.
  • gh pr checks 1413:
    • All OptimizationIpopt jobs pass: Core on 1, lts and pre; QA; and downgrade.
    • Two jobs fail, and both are unrelated:
      • ModelingToolkit.jl/Optimization downstream fails with an Unsatisfiable requirements ... TimerOutputs resolver error.
      • downgrade-sublibraries (lib/OptimizationReactant) fails with test failures inside OptimizationReactant.
    • The same workflows (IntegrationTest, Downgrade Sublibraries) also fail on master at ce2b3a0.
  • When I finished, git status --porcelain in the checkout was empty. All scratch output is in ../scratch (qa.log, core.log, probe_pr_head.log, probe_reexport.log).

What I did not verify

  • The Julia 1.10 LTS and 1.11 runs locally (only 1.12.7). CI's lts and pre Core jobs pass.
  • Downstream packages outside this repo that might spell an incidentally reachable name as OptimizationIpopt.<name>.
  • The PR body's baseline QA result of 20 pass / 1 broken from before the change. I did not rerun master.
  • The repo root has no AGENTS.md or CLAUDE.md, so I judged against the global rules only. The PR adds no dependency, no non-public API use, and no narrating comments, and it covers a single concern.

🤖 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 11:02
@ChrisRackauckas
ChrisRackauckas merged commit f67663d 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