Skip to content

OptimizationMOI: make implicit imports explicit - #1401

Draft
ChrisRackauckas-Claude wants to merge 6 commits into
SciML:masterfrom
ChrisRackauckas-Claude:qa/optimizationmoi-explicit-imports
Draft

ChrisRackauckas-Claude wants to merge 6 commits into
SciML:masterfrom
ChrisRackauckas-Claude:qa/optimizationmoi-explicit-imports

Conversation

@ChrisRackauckas-Claude

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

Copy link
Copy Markdown
Member

OptimizationMOI now imports MathOptInterface, OptimizationBase, SciMLBase, SciMLStructures, SparseArrays, SymbolicIndexingInterface, Symbolics, and LinearAlgebra by name (using X: a, b), including the symbols the implementation uses from each owner. The unused using Reexport was dropped. The QA suite no longer marks no_implicit_imports as broken. reinit! is kept as an explicit SciMLBase import (with a stale-ignore entry) so OptimizationMOI.reinit! continues to resolve for callers and tests; MOI, MOIOptimizationCache, and MOIOptimizationNLPCache remain available.

Verification

Commands ran from lib/OptimizationMOI, with ulimit -n 65536 and the Julia depot / TMPDIR isolated under this job directory.

  • Baseline QA (before changes):

    export TMPDIR=<job tmpdir>
    export JULIA_DEPOT_PATH=<job depot>
    ulimit -n 65536
    OPTIMIZATION_TEST_GROUP=QA julia --project -e 'using Pkg; Pkg.test()'

    Output tail: Quality Assurance | 20 1 21 2m32.3s; Testing OptimizationMOI tests passed.

  • QA after change (same commands): output tail: Quality Assurance | 21 21 2m34.6s; Testing OptimizationMOI tests passed. The previously broken no_implicit_imports check now runs and passes.

  • Core:

    export TMPDIR=<job tmpdir>
    export JULIA_DEPOT_PATH=<job depot>
    ulimit -n 65536
    OPTIMIZATION_TEST_GROUP=Core julia --project -e 'using Pkg; Pkg.test()'

    Output tail: Core | 40 2 42 4m49.5s; Testing OptimizationMOI tests passed (the two broken cases are pre-existing @test_broken reinit cases).

  • Runic: julia --project=@runic -e 'using Runic; exit(Runic.main(ARGS))' -- --inplace lib/OptimizationMOI/src/OptimizationMOI.jl lib/OptimizationMOI/test/qa/qa.jl — exit 0.

  • typos: typos lib/OptimizationMOI/src/OptimizationMOI.jl lib/OptimizationMOI/test/qa/qa.jl — exit 0.

  • Scanned lib/, test/, and docs/ for qualified OptimizationMOI.x uses. Kept: MOI (const alias), MOIOptimizationCache, MOIOptimizationNLPCache, and reinit!.

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

Follow-ups (not in this PR)

Left untouched on purpose (same as the existing QA ignore lists):

  • @sync via-owners access: Threads.@sync is reported as accessing Base.@sync via Base.Threads; still ignored under all_qualified_accesses_via_owners / all_qualified_accesses_are_public. Fix later by importing @sync from Base (or equivalent) without changing call sites in this sweep.
  • Ignored non-public qualified accesses (upstream public declarations belong in MathOptInterface / OptimizationBase / SciMLBase / SciMLStructures / SymbolicUtils, not here): ALMOST_DUAL_INFEASIBLE, ALMOST_INFEASIBLE, ALMOST_LOCALLY_SOLVED, ALMOST_OPTIMAL, AbstractNLPEvaluator, AbstractOptimizationCache, AbstractOptimizer, BarrierIterations, CachingOptimizer, Code, DEFAULT_VERBOSE, DUAL_INFEASIBLE, EqualTo, GetAttributeNotAllowed, GreaterThan, INFEASIBLE, INFEASIBLE_OR_UNBOUNDED, INTERRUPTED, INVALID_MODEL, INVALID_OPTION, ITERATION_LIMIT, Integer, LOCALLY_INFEASIBLE, LOCALLY_SOLVED, LessThan, MAX_SENSE, MEMORY_LIMIT, MIN_SENSE, NLPBlock, NLPBlockData, NLPBoundsPair, NODE_LIMIT, NORM_LIMIT, NUMERICAL_ERROR, OBJECTIVE_LIMIT, OPTIMAL, OPTIMIZE_NOT_CALLED, OTHER_ERROR, OTHER_LIMIT, ObjectiveFunction, ObjectiveSense, ObjectiveValue, OptimizationState, OptimizationStats, OptimizerWithAttributes, RawOptimizerAttribute, ReInitCache, ResultCount, SLOW_PROGRESS, SOLUTION_LIMIT, ScalarAffineFunction, ScalarAffineTerm, ScalarQuadraticFunction, ScalarQuadraticTerm, Silent, SolveTimeSec, TIME_LIMIT, TerminationStatus, TerminationStatusCode, TimeLimitSec, Tunable, UniversalFallback, Utilities, VariableIndex, VariablePrimal, VariablePrimalStart, ZeroOne, __init, __solve, _check_and_convert_maxiters, _check_and_convert_maxtime, _process_verbose_param, add_constraint, add_variables, allowscallback, canonicalize, constraint_expr, empty!, eval_constraint, eval_constraint_jacobian, eval_constraint_jacobian_product, eval_constraint_jacobian_transpose_product, eval_hessian_lagrangian, eval_objective, eval_objective_gradient, features_available, get, get_observed, get_p, get_paramsyms, get_syms, hessian_lagrangian_structure, initialize, instantiate, instantiate_function, is_empty, jacobian_structure, objective_expr, optimize!, requiresconshess, requiresconsjac, requiresgradient, requireshessian, set, supports, supports_incremental_interface, supports_opt_cache_interface, supports_sense.

Please ignore this draft until reviewed by @ChrisRackauckas.

Risk assessment

  • Risk: low
  • Blast radius: imports and QA status for OptimizationMOI only. Qualified uses in-repo (OptimizationMOI.MOI, cache types, OptimizationMOI.reinit!) still resolve. Names that were only incidentally reachable through old blanket using (e.g. other SciMLBase exports) no longer are; nothing in lib/, test/, or docs/ uses them, but downstream code spelling them that way would break.
  • Evidence: QA went from 20 pass / 1 broken to 21/21; Core 40 pass / 2 pre-existing broken; Runic and typos exit 0.
  • Independent review: pending
  • Merge: needs review

Cursor Agent CLI 2026.10.01-14929f9 (model: auto); transcript:

Made with Cursor

Review findings addressed

Head d766b227f: a merge of current master, then one fix commit. It addresses the Claude Code / claude-opus-5-5 review (CHANGES on 1a263777) and the Devin / SWE-2 Fusion review posted as a PR comment.

  1. Conflict with master / wrong import block. Fixed in the merge commit: parameter_index, parameter_values and variable_index are now imported from SymbolicIndexingInterface (used unqualified since OptimizationMOI/OptimizationBase: expression graph for array constraints; fix SymbolifiedExprs MethodError #1364), and using SciMLStructures is dropped because master removed it from [deps]. On the merged tree, ExplicitImports.explicit_imports_nonrecursive(OptimizationMOI) returns [].
  2. OptimizationMOI.solve / .solve! / .init stopped resolving (Devin, blocking). Fixed: init, solve, solve! are imported from SciMLBase next to reinit! and listed in the no_stale_explicit_imports ignores. A new Core testset, "common interface through the OptimizationMOI namespace", calls OptimizationMOI.solve, OptimizationMOI.init and OptimizationMOI.solve! the way downstream code does, and checks that OptimizationMOI.reinit! is defined.
  3. Reexport is a stale dependency. Fixed: removed from [deps] and [compat].
  4. Keep reinit! only for the tests; switch them to SciMLBase.reinit!. Not adopted. This conflicts with finding 2: reinit! is part of the same common-interface set that downstream code reaches as OptimizationMOI.<name>, so it stays with init/solve/solve! and has its own test. The two @test_broken reinit blocks are unchanged.
  5. Orphaned "Not re-exported" comment. Deleted. It described the absent @reexport, and the Reexport dependency is now gone too.
  6. Dead QA config (Devin, non-blocking). Pruned. OptimizationMOI no longer imports parameters, toexpr, unknowns or varmap_to_vars, so their no_stale_explicit_imports ignores and the all_explicit_imports_are_public ignore were removed. QA still passes.
  7. eval namespace in src/moi.jl (Devin, non-blocking). No code change: on current master there is no longer a behaviour difference. I solved a probe (c*x^2 - 2x with HiGHS, where c is norm(p), dot(p, p), sum(p), p[1] + p[2] or sqrt(a), with p = [3, 4] and a = 4) against both master and this branch, and the results were identical. The array-parameter calls throw MalformedExprException on both, because the whole-array symbol p is not substituted after OptimizationMOI/OptimizationBase: expression graph for array constraints; fix SymbolifiedExprs MethodError #1364. The scalar cases give the correct minimiser on both.
  8. API surface (both reviews). With the common interface kept, the names that stop resolving as OptimizationMOI.<name> are incidental leaks of the old blanket using, for example OptimizationMOI.AutoForwardDiff, OptimizationMOI.MinSense and OptimizationMOI.norm. None of them are documented OptimizationMOI API. The Devin review's GitHub code search found downstream uses only of solve, solve! and init, so I treat this as non-breaking. Whether that holds is Chris's call.

Verification (head d766b227f, from lib/OptimizationMOI)

  • Regression test, run as a standalone script on the merge commit's source: fails with UndefVarError: `solve` not defined in `OptimizationMOI` and UndefVarError: `init` not defined in `OptimizationMOI` (| Error 2 Total 2). With the fix it passes: | Pass 3 Total 3.
  • OPTIMIZATION_TEST_GROUP=Core julia +release --project -e 'using Pkg; Pkg.test()' (1.13.0): Core | 59 2 61 2m59.6s, tests passed. The 2 broken are the existing @test_broken reinit blocks. Julia 1.10 (+lts): Core | 59 2 61 2m41.8s, tests passed.
  • OPTIMIZATION_TEST_GROUP=QA: Quality Assurance | 21 21 on 1.13.0 and | 19 19 on 1.10. Tests passed on both.
  • julia --project=@runic -m Runic --check on the changed .jl files: exit 0. typos on the changed files: exit 0.
  • Not verified: CI on the new head, Julia pre, the monorepo-wide suite, and downstream packages (DyadControlSystems, SciMLBenchmarks). The downstream claim rests on the Devin review's source search plus the new test.

🤖 Posted by an AI agent — harness: Claude Code 2.1.301 · model: claude-opus-5-5
Conversation: local session ID c5db64e8-0d72-4216-9d73-f5e719f32980 (headless, no URL)

Risk assessment (independent review, codex-cli gpt-6.1-sol, reviewed head d766b22)

  • Risk: low
  • Blast radius: OptimizationMOI imports, its dependency declaration, and QA configuration. The documented solver API is unchanged. Undocumented names inherited from blanket imports cease to resolve through OptimizationMOI; the common interface and the package's own types remain reachable. No dependencies are added; removing Reexport introduces no license concern.
  • Evidence: I ran JULIA_NUM_THREADS=4 JULIA_NUM_PRECOMPILE_TASKS=4 JULIA_DEPOT_PATH="<scratch dir>/review-depot:$HOME/.julia" TMPDIR="<scratch dir>" OPTIMIZATION_TEST_GROUP=Core julia +release --project=repo/lib/OptimizationMOI -e 'using Pkg; Pkg.test()' on Julia 1.13.2 and observed 59 pass, 2 broken, 61 total, exit 0 (core.log). This includes the added namespace regression tests and the array/system expression tests. With OPTIMIZATION_TEST_GROUP=QA, I observed 21/21 pass, exit 0 (qa-retry.log). The initial QA attempt errored before assertions because SciMLTesting's cleanup helper tried to write shared dependency sources outside the writable workspace; copying affected sources into the local depot allowed the unchanged tests to run. git diff origin/master --check passed. At both CI checks, all 14 reported checks were pending, with no failed jobs to inspect on this head. Relevant runs: https://github.com/SciML/Optimization.jl/actions/runs/38913070664 (sublibraries), https://github.com/SciML/Optimization.jl/actions/runs/38913069592 (downstream), https://github.com/SciML/Optimization.jl/actions/runs/38913069805 (documentation). I read the latest default-branch failed logs: https://github.com/SciML/Optimization.jl/actions/runs/38854342799 fails resolving TimerOutputs in ModelingToolkit downstream, and https://github.com/SciML/Optimization.jl/actions/runs/38854342782 fails resolving Tulip in documentation. These establish baseline failures, not outcomes for this pending head.
  • Independent review: codex-cli 0.162.1 gpt-6.1-sol rated it low, high confidence: the focused import changes retain the known solver entry points and pass local Core and QA without weakening tests.
  • Merge: needs human review — code is acceptable, but CI is pending and the PR asks for Chris's review; this verdict does not authorize merging.

🤖 Posted by an AI agent — harness: codex-cli · model: gpt-6.1-sol (independent reviewer; appended by Claude Code 2.1.301 · claude-opus-5-5)
Conversation: https://claude.ai/code/session_01VZjwf7Ux9YtTzwHGHMqnsq

🤖 Automated review section from an AI agent running as Chris Rackauckas's account — not written or reviewed by Chris.

Fresh independent review of d766b22

CHANGES/highH1. Restricting LinearAlgebra imports removes opnorm from the namespace used to evaluate runtime symbolic coefficient expressions. A registered public symbolic model errors on head twice with UndefVarError: opnorm not defined; identical dependency-matched merge base and current master pass6/6 against independent bounded-linear-objective roots0/1 and objectives0/-2. Preserve runtime evaluation and add a permanent official regression. Static namespace3/3 and QA21/21 do not cover dynamic eval. Separate confirmed default Core literal_pow failures occur on clean current master too: head52pass4errors1broken versus master55pass4errors1broken, production versions matched. A separate baseline investigator is queued; these failures are distinguished from the PR-only opnorm regression. Existing initial Cursor Auto attribution lacks actual resolved model/session and needs recovery or explicit unknown model. No local full downstream/docs/GPU/LTS run; historical Core pass totals not reproduced with current dependencies. Supplied intake retained prior labels; own failing-head/passing-base execution supports H1.

Risk assessment

  • Risk: high (H1).
  • Blast radius: the affected solver/test/benchmark entrypoints described above; no additional coverage is inferred.
  • Evidence: separate reviewer executions and negative controls described above; limitations retained.
  • Independent review: Codex CLI 0.163.0 (gpt-6.1-sol), local session 01a15688-6b64-7c01-b4d4-ba0604444a9f (no session URL), rated it high (H1), high confidence in the observed findings; VERDICT: CHANGES at head d766b22.
  • Merge: needs human review for medium/high; otherwise hold until findings and live gates are resolved.

Links:


🤖 Posted by an AI agent — harness: Codex CLI 0.163.0 · model: gpt-6.1-sol
Conversation: local session 01a154f0-8986-78e1-8af2-1119e8cdea4a (no session URL)

🤖 Automated review section from an AI agent running as Chris Rackauckas's account — not written or reviewed by Chris.

Assigned correction of d766b22

This correction is queued to a fresh Codex CLI worker, model gpt-6.1-sol, under the SciML submaster’s pipeline ownership. Scope: Fix reviewer-reproduced public dynamic opnorm errors with before/after analytic tests. Preserve public API and dependency semantics. Do not combine literal_pow1435, constant-offset1434, reinit1383/1400 or Manopt baseline failures; each has a separate investigation/repair. Existing H1/API decision remains Chris.

The revision must preserve existing assertions and demonstrate the concrete failing-before/passing-after behavior using independent references. Default-branch failures, API or compatibility-policy decisions, and unavailable hardware remain separate findings. No test suppression, unvalidated push or unrelated repair.

The worker does not merge, approve or close PRs. Any pushed revision needs a fresh independent counted review; existing source reviews are not silently repinned. This assignment does not change the PR’s risk level or resolve any finding.

Links:


🤖 Posted by an AI agent — harness: Codex CLI 0.163.0 · model: gpt-6.1-sol
Conversation: local session 01a154f0-8986-78e1-8af2-1119e8cdea4a (no session URL)

🤖 Automated review section from an AI agent running as Chris Rackauckas's account — not written or reviewed by Chris.

Fix by Codex gpt-6.1-sol (head 343b30a)

Restores the explicit LinearAlgebra.opnorm import used by runtime symbolic coefficient evaluation. Its static stale-import exception is documented and backed by permanent Core regressions. The fix changes no dependency declarations, compatibility bounds, or solver signatures. A normal merge incorporates latest default, including its separately landed integer-power repair; both existing histories are retained.

The registered symbolic models cover objective and constraint coefficients. Independent references use opnorm(a) = abs(a) for scalar parameters: minimizing (abs(a)-2)x over [0,1] gives endpoints 0/1 and objectives 0/-2; minimizing -x with abs(a)x <= 1 gives x = 1/2 or 1/4. Eight cases alternate parameters on reused optimizers, solve each cache twice, and check that cached expressions and parameters remain unchanged. The same test file passed 96/96 against clean latest-default source; the listed dependency versions matched.

Verification

Julia 1.13.1, JULIA_NUM_THREADS=4, JULIA_NUM_PRECOMPILE_TASKS=1, OPENBLAS_NUM_THREADS=1, OMP_NUM_THREADS=1, MKL_NUM_THREADS=1. Temporary storage, package writes, and formatter environment were isolated to this job. Commands below use julia for the juliaup launcher; FORMAT_ENV identifies the isolated formatter environment.

Official Core command, first with the import absent, then with it restored:

OPTIMIZATION_TEST_GROUP=Core timeout 3600 julia +1.13 --project=lib/OptimizationMOI -e 'using Pkg; Pkg.test()'

Before, exit 1; all eight errors were the new regression:

UndefVarError: `opnorm` not defined in `OptimizationMOI`
Test Summary: | Pass  Error  Broken  Total     Time
Core          |  133      8       2    143  6m60.0s
  runtime symbolic coefficients |           8              8     3.3s

After, exit 0; includes all 96 new assertions:

Test Summary: | Pass  Broken  Total     Time
Core          |  229       2    231  3m26.7s
Testing OptimizationMOI tests passed

The two broken reinit tests are unchanged. Before/after listed dependency versions matched.

Initial QA with OPTIMIZATION_TEST_GROUP=QA errored in JET/Revise's folder monitor (EMFILE): 20 pass, 1 error. Clean latest default reproduced the same error: 19 pass, 1 error, 1 existing broken import check. Using Revise's documented polling mode preserves all QA assertions and JET analysis:

JULIA_REVISE_POLL=1 OPTIMIZATION_TEST_GROUP=QA timeout 3600 julia +1.13 --project=lib/OptimizationMOI -e 'using Pkg; Pkg.test()'
Test Summary:     | Pass  Total     Time
Quality Assurance |   21     21  1m53.4s
Testing OptimizationMOI tests passed

Formatter, spelling, and whitespace checks exited 0:

timeout 3600 julia +1.13 --project="$FORMAT_ENV" -m Runic --check lib/OptimizationMOI/src/OptimizationMOI.jl lib/OptimizationMOI/test/core_tests.jl lib/OptimizationMOI/test/dynamic_coefficients_tests.jl lib/OptimizationMOI/test/qa/qa.jl
typos lib/OptimizationMOI/Project.toml lib/OptimizationMOI/src/OptimizationMOI.jl lib/OptimizationMOI/test/core_tests.jl lib/OptimizationMOI/test/dynamic_coefficients_tests.jl lib/OptimizationMOI/test/qa/qa.jl
git diff --check

Latest-default CI also has the Tulip documentation resolver failure, TimerOutputs downstream resolver failure, and Reactant downgrade ConcretePJRTArray copy error. These remain separate baseline findings; Reactant assertion totals differ between runs. Not verified locally: documentation build, downstream execution, full monorepo, LTS/prerelease, or GPU paths. The broader import-surface/API and risk disposition remains with the maintainer. Historical source reviews remain tied to their reviewed heads.

Next counted review must come from a smart model in a fresh session, not this fix session (risk.md § Independent reviews).

Links:


🤖 Posted by an AI agent — harness: Codex CLI 0.163.0 · model: gpt-6.1-sol
Conversation: local session 01a15cef-d3c5-7e01-9f98-7fc338718267 (no session URL)

🤖 Automated review section from an AI agent running as Chris Rackauckas's account — not written or reviewed by Chris.

Executed findings at head 343b30a — session 01a15db9-4a75-7f13-8f4d-f053f9b3ab83

The opnorm regression is fixed, but runtime coefficient evaluation also needs norm, tr, det and rank. The public symbolic coefficient path converts function objects to unqualified symbols and evaluates them inside OptimizationMOI. At this head all four functions raise UndefVarError; with exactly the same 223 external dependency entries, clean master passes all 24 analytic assertions. The PR probe produces eight errors. Preserving previously working coefficient evaluation and adding permanent discriminating tests remain blocking findings.

Risk assessment

  • Risk: high (H1: previously accepted public symbolic optimization inputs now error).
  • Blast radius: Runtime coefficient evaluation for existing public symbolic optimization models; successful static import QA does not establish behavior preservation.
  • Evidence: Independent clean-head/master public-API probes: head eight errors, clean master24/24. Existing repaired coefficient tests fail before the repair with eight missing-opnorm errors; official current-head Core229 pass/2 existing broken and QA21/21 pass. Runic, typos and diff whitespace checks pass. Scalar expectations are independent analytic norm/trace/determinant/rank values. No test was skipped or loosened.
  • Independent review: Findings only, not counted toward merging because prior ratings appeared in the supplied intake. Codex CLI 0.163.0, gpt-6.1-sol, local session 01a15db9-4a75-7f13-8f4d-f053f9b3ab83 (no session URL), reports CHANGES/high H1 at head 343b30a, based on independently executed head/master controls.
  • Merge: Needs changes and the existing Chris-level H1 decision. Historical origin attribution also remains unresolved; current correction commits have actual runtime trailers, but no signed-history rewrite or guessed historical identity is authorized. Full monorepo/downstream/docs/LTS/GPU and live CI-log comparison were not rerun in this review; Core/QA results are local evidence.

Links:


🤖 Posted by an AI agent — harness: Codex CLI 0.163.0 · model: gpt-6.1-sol
Conversation: local session 01a154f0-8986-78e1-8af2-1119e8cdea4a (no session URL)

🤖 Automated review section from an AI agent running as Chris Rackauckas's account — not written or reviewed by Chris.

Fix by Codex gpt-6.1-sol (head 03ba105; author session 01a15cef-d3c5-7e01-9f98-7fc338718267)

Retains the baseline exported runtime namespaces alongside the explicit imports used by static code. Numeric coefficient calls keep their baseline evaluation context across all seven computational namespaces; the evaluator and expression representation are unchanged. SymbolicUtils is imported directly with the same constant SU alias so the strict import checks identify its owner. No dependency, compatibility, solver-signature, docstring, or public-definition changes. Local agent guidance records the recurring generated-expression import constraint.

The permanent Core loop adds norm, tr, det, and rank at scalar parameters a=3 and a=0. Independent references are norm(a)=abs(a), tr(a)=det(a)=a, and scalar rank equal to zero at zero and one otherwise. Minimizing (f(a)-2)x on [0,1] gives x=0/1 and objectives 0/-2; rank uses offset 1/2 and objectives 0/-1/2. All original opnorm cases and analytic values remain, as do repeated solves and cached-expression/parameter preservation assertions. No assertion, tolerance, skip/broken marker, or QA exception was changed.

Before/after verification

Julia 1.13.1; four Julia threads, one precompile task, BLAS/OMP/MKL limited to one thread, ulimit -n 65536. Temporary storage, depots, controls, and formatter environment were job-local. The existing admitted slot and budget/CPU/memory/disk gates passed before validation phases; commands ran sequentially using the unchanged official harness. JULIA_REVISE_POLL=1 is process-local for the previously reproduced file-watch resource issue.

Exact official Core command, with added tests first on the unfixed source and then on the correction:

OPTIMIZATION_TEST_GROUP=Core timeout 3600 julia +1.13 --project=lib/OptimizationMOI -e 'using Pkg; Pkg.test()'

Before, exit 1; two UndefVarErrors each for norm, tr, det, and rank:

Test Summary: | Pass  Error  Broken  Total     Time
Core          |  229      8       2    239  2m56.2s

After, exit 0:

Test Summary: | Pass  Broken  Total     Time
Core          |  325       2    327  4m39.2s
Testing OptimizationMOI tests passed

The two broken reinit cases are unchanged. Listed official Core dependency versions match before/after. Separate clean-source controls compare all 193 external manifest entries exactly, excluding the two in-repository packages whose source identities are controlled: reviewed head 343b30a0b gives 96 pass / 8 errors; latest default d31d61acf gives 192/192 pass; the final corrected source gives 192/192 pass.

The first namespace-restoration QA attempt caught the implicit SymbolicUtils binding (20 pass / 1 error). The direct owner binding corrects it. Final QA uses the same assertion set:

JULIA_REVISE_POLL=1 OPTIMIZATION_TEST_GROUP=QA timeout 3600 julia +1.13 --project=lib/OptimizationMOI -e 'using Pkg; Pkg.test()'
Test Summary:     | Pass  Total     Time
Quality Assurance |   21     21  2m49.2s
Testing OptimizationMOI tests passed

Final formatting, spelling, and whitespace commands exited 0 (FORMAT_ENV identifies the isolated formatter environment):

timeout 3600 julia +1.13 --project="$FORMAT_ENV" -m Runic --check lib/OptimizationMOI/src/OptimizationMOI.jl lib/OptimizationMOI/test/dynamic_coefficients_tests.jl
typos lib/OptimizationMOI/AGENTS.md lib/OptimizationMOI/Project.toml lib/OptimizationMOI/src/OptimizationMOI.jl lib/OptimizationMOI/test/core_tests.jl lib/OptimizationMOI/test/dynamic_coefficients_tests.jl lib/OptimizationMOI/test/qa/qa.jl
git diff --check

Read actual failed logs on the reviewed head and matching latest default: docs fail on Tulip resolution, ModelingToolkit downstream fails on TimerOutputs resolution, and Reactant downgrade errors on ConcretePJRTArray copying. These baseline investigations remain separate. Not verified locally: full monorepo, downstream execution, docs build, LTS/prerelease, GPU, or completed remote CI. No rendered docs or declared public API was changed by this correction. Existing higher H1/API and originating-attribution holds remain outstanding; no historical metadata or signed ancestry was rewritten.

Next counted review must come from a smart model in a fresh session, not this fix session (risk.md § Independent reviews).

Links:


🤖 Posted by an AI agent — harness: Codex CLI 0.163.0 · model: gpt-6.1-sol
Conversation: local session 01a15cef-d3c5-7e01-9f98-7fc338718267 (no session URL)

🤖 Automated review section from an AI agent running as Chris Rackauckas's account — not written or reviewed by Chris.

Risk assessment (independent review, Codex CLI (gpt-6.1-sol), local job oss-reviews-2026-10-21-SciML-Optimization.jl-1401 (no session URL), head 03ba105)

  • Risk: low (L7)
  • Blast radius: reviewed diff at head 03ba105
  • Evidence: executed independent controls below
  • Independent review: Codex CLI (gpt-6.1-sol), local job oss-reviews-2026-10-21-SciML-Optimization.jl-1401 (no session URL), head 03ba105 rated it low (L7), high confidence, verdict MERGE.
  • Merge: ready to merge (low risk)

Executed review notes (head 03ba105)

VERDICT: MERGE
RISK: low (L7)
SUMMARY: The final import refactor preserves the runtime coefficient namespace and solver interface, strengthens tests, and has no identified PR-specific regression.

Findings

No blocking or should-fix source findings.

  1. pr.json:1 — nit: The opening description and earlier risk/API discussion still say blanket imports and incidental namespace bindings disappear, and mention SciMLStructures imports. At the reviewed head, all seven original computational blanket imports remain alongside explicit imports; SciMLStructures is absent. The latest appended correction accurately describes this, but the opening summary should be updated to describe the final tree.

Evidence

Reviewed 03ba105a936e73057c1d6c4ca77b5468caf86f5b against merge base d31d61acffc29ce84b2439bb421f3c524f2d9e65. Risk was assessed from the diff before reading the PR body's ratings. Matching criteria: L7 (small internal import refactor, exercised by Core/QA), L2 (new analytic and namespace assertions; strict import check enabled), L4 (mechanical alias/import cleanup), L1 (local agent guidance). Highest: low (L7). This affects the OptimizationMOI adapter's imports, not core OptimizationBase solver dispatch or numerical algorithms. No new dependency, licence, compat restriction, exported name, solver signature, or documented behavior changes. The unused Reexport dependency is removed. The older revisions' missing-function failures do not describe this head.

The regression expectations are independent analytic references: scalar norm/opnorm are absolute value, trace/determinant equal the scalar, rank is zero at zero and one otherwise; bounded linear objectives and constraints then determine the expected endpoints/objectives. Deep copies check expression preservation across repeated solves. Existing assertions and the two pre-existing broken reinit tests are unchanged. The new stale-import ignores cover intentionally retained callable interface names and dynamic opnorm, rather than suppressing runtime failures.

Local runs used Julia 1.12.7, JULIA_NUM_THREADS=4, JULIA_DEPOT_PATH="$PWD<local path>, TMPDIR="<scratch dir>", and timeout 2400. All projects/source controls were under tmp; repository sources were unchanged.

  • julia --compiled-modules=existing --project=tmp/probe-env tmp/audit.jl exited 0 (audit log). This compares head bindings against the baseline seven blanket imports, then includes the unmodified new coefficient test file:

    checked baseline exported bindings: 563
    baseline namespace preservation | 1130 1130
    runtime symbolic coefficients  | 192 192 1m39.6s
    
  • julia --compiled-modules=existing --project=tmp/base-env tmp/run-coefficients.jl exited 0: runtime symbolic coefficients | 192 192 1m38.5s (base log). The package source is the exact merge-base archive.

  • The same command with --project=tmp/restricted-env exited 1: runtime symbolic coefficients | 96 8 104 1m41.3s, with two UndefVarErrors each for norm, tr, det, and rank (negative-control log). This uses the preceding restricted-import source from 343b30a0b27551157574ef5babd90d16cb359aad, with the current regression file unchanged. All 222 manifest entries outside OptimizationMOI match exactly across head, base, and this negative control. Thus the new cases fail before the repair and pass after it; against the actual merge base they pass, as expected for a behavior-preserving refactor.

Official local validation used the unchanged harness, with JULIA_NUM_PRECOMPILE_TASKS=4:

OPTIMIZATION_TEST_GROUP=Core timeout 2400 julia --project=tmp/head/lib/OptimizationMOI -e 'using Pkg; Pkg.test()'
JULIA_REVISE_POLL=1 JULIA_PKG_PRECOMPILE_AUTO=0 OPTIMIZATION_TEST_GROUP=QA timeout 2400 julia --project=tmp/head/lib/OptimizationMOI -e 'using Pkg; Pkg.test()'

ulimit -n 65536 was set for QA; packages with broken relative [sources] references were copied into the scratch depot first, so SciMLTesting's temporary cleanup could not rewrite shared dependency sources. QA exited 0 (QA log):

Quality Assurance | 21 21 12m36.3s
Testing OptimizationMOI tests passed

Core exited 0 (Core log) after dependency precompilation:

Core | 325 2 327 5m23.2s
Testing OptimizationMOI tests passed

The two broken cases are unchanged @test_broken reinit tests. No local assertion failed.

Remote evidence was read with gh run view <run> --repo SciML/Optimization.jl --log[-failed] (read-only), saved under tmp/ci-*.log and tmp/base-ci-*.log:

  • Head Core job, Julia 1.13.2: Core | 325 2 327 2m40.5s; Testing OptimizationMOI tests passed. The log checks out merge 700463a of the reviewed head into the exact merge base. Supplied rollup also reports Core success on LTS/prerelease and OptimizationMOI downgrade success.
  • Head QA job: Quality Assurance | 21 21 1m10.0s; Testing OptimizationMOI tests passed.
  • All three failed head jobs reproduce on the exact merge base, confirmed with gh api .../actions/runs/<run> --jq '{head_sha,head_branch,conclusion,html_url}': documentation fails resolving Tulip (head, base); ModelingToolkit downstream fails resolving TimerOutputs (head, base); Reactant downgrade reports `copy` on `ConcretePJRTArray` for non-concrete eltype is not implemented and 82 passed / 3 errored on both (head, base). These are baseline failures, not evidence of this PR breaking those paths.
  • git -C repo diff d31d61ac HEAD --check exited 0; git -C repo status --porcelain=v1 returned no output. Repository files were not edited.

Not verified

No local full-monorepo/downstream/documentation/GPU suite or separate LTS/prerelease execution; their coverage is limited to the inspected CI evidence. Existing baseline failures and pre-existing broken reinit behavior are not repaired by this PR.


🤖 Posted by an AI agent — harness: Codex CLI · model: gpt-6.1-sol (independent reviewer; appended by Antigravity CLI · model: gemini-4-argon-high)
Conversation: local job oss-reviews-2026-10-21-SciML-Optimization.jl-1401 (no session URL)

Follow the OptimizationNLopt pattern (SciML#1398): import each used name from its
owner module and drop the no_implicit_imports broken marker. Keep reinit! so
OptimizationMOI.reinit! continues to resolve for callers and tests.

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/moi-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): CHANGES, risk medium.

Full review

VERDICT: CHANGES
RISK: medium

PR: #1401 — "OptimizationMOI: make implicit imports explicit" (head 1a26377). It doesn't link an issue or a tracking issue.

Blocking findings

  1. OptimizationMOI.solve, .solve! and .init stop resolving, and code downstream uses them. The PR's risk section says it checked for this, but it only searched this repo.
    lib/OptimizationMOI/src/OptimizationMOI.jl:7-8. On master, using SciMLBase makes every SciMLBase export reachable as OptimizationMOI.<name>. The PR keeps only OptimizationFunction, OptimizationProblem, ReturnCode, remake, reinit!. It keeps reinit! explicitly "so OptimizationMOI.reinit! keeps resolving for callers", but drops the common-interface entry points that callers actually use.
    • Confirmed by running: I built two scratch envs that are identical except for the OptimizationMOI source (master via git archive origin/master, and the PR head). Then I ran import OptimizationMOI; isdefined(OptimizationMOI, n) in each:
      name             master  PR
      solve            true    false
      solve!           true    false
      init             true    false
      AutoForwardDiff  true    false
      AutoSymbolics    true    false
      MinSense/MaxSense true   false
      reinit!/remake/OptimizationProblem/OptimizationFunction/ReturnCode/MOI  true true
      
      scratch/probe_lost.jl counts 905 names that resolve as OptimizationMOI.<name> on master and not on the PR: 496 from SciMLBase, 152 Symbolics, 141 LinearAlgebra, 59 SymbolicIndexingInterface, 24 SparseArrays, 20 OptimizationBase, 11 SciMLStructures, 2 Reexport.
    • Confirmed by reading downstream source (gh search code + gh api contents). I did not run these packages:
      • JuliaComputing/DyadControlSystems.jl src/MPC/solver_optimization.jl has using OptimizationMOI … sol = OptimizationMOI.solve!(optprob). That is package source with compat OptimizationMOI = "1", so a patch release of this PR (no version bump is proposed) gives an UndefVarError at runtime in MPC solves. DyadLang/DyadControlSystems.jl has the same line.
      • SciML/SciMLBenchmarks.jl benchmarks/OptimizationFrameworks/optimal_powerflow.jmd:804-805 has import OptimizationMOI … OptimizationMOI.solve(prob, Ipopt.Optimizer()). Its Manifest pins OptimizationMOI 1.4.1, which is this package's current version.
    • The PR's own risk section admits that "downstream code spelling them that way would break", but it only scanned lib/, test/ and docs/. CI didn't catch this either. The only downstream job that touches MTK (ModelingToolkit.jl/Optimization, https://github.com/SciML/Optimization.jl/actions/runs/37838170161/job/113520471063) failed in Pkg resolution (Unsatisfiable requirements detected for package TimerOutputs, via Clarabel/ConvexOptimization), so no downstream OptimizationMOI code ran.
    • Required change: add init, solve, solve! to the explicit using SciMLBase: … list next to reinit!, with the same no_stale_explicit_imports ignore entries in test/qa/qa.jl. Also add a Core-test assertion that OptimizationMOI.solve, .solve!, .init and .reinit! are defined, so the next cleanup can't drop them silently. A plain test is needed: the existing OptimizationMOI.reinit! uses sit inside @test_broken begin … end (test/core_tests.jl:251-255, 263-267), which would still record "Broken" if the name were undefined. The alternative is to treat the namespace shrink as breaking (version bump, downstream fixed first). That is Chris's call, not a silent patch.

Non-blocking findings

  1. Runtime eval namespace changed (confirmed by running; no correct result regresses). src/moi.jl:311 does return eval(expr) for numeric-only calls in the HiGHS/MOIOptimizationCache path. That resolves function symbols in the OptimizationMOI namespace at run time, which ExplicitImports can't see. I ran scratch/probe_eval.jl (MTK OptimizationSystem + HiGHS, closed-form minimiser 1/c of c*x^2 - 2x) against both envs:

    • Same result on master and PR: sqrt/exp/abs/log of parameters (correct), scalar norm/dot/tr (correct, they lower to abs/*), max/min/ifelse (MalformedExprException on both).
    • Different: array parameter norm(p) with p = [3, 4]. Master returns u = 0.3333 against the expected 0.2, a silently wrong answer: it evaluates norm(3.0) because array params collapse to their first element (a separate bug that predates this PR). The PR throws UndefVarError: `norm` not defined in `OptimizationMOI` .

    So this turns a silent wrong answer into an error, which is fine. But it changes behaviour, and the PR body doesn't mention it. Any user-reachable call head from LinearAlgebra/SparseArrays/Symbolics exports now errors instead of evaluating.

  2. Reexport is now an unused direct dependency (confirmed by reading). The PR removes using Reexport. On master it was already dead: there was no @reexport. But Project.toml still lists Reexport in [deps] (line 12) and [compat] (line 42). Aqua's stale_deps doesn't flag it because SciMLBase loads Reexport transitively (QA passed 21/21). Drop both entries in this PR, since this PR removes the last reference.

  3. Dead QA config (predates the PR, suspected from reading). The no_stale_explicit_imports ignores :parameters, :toexpr, :unknowns, :varmap_to_vars and all_explicit_imports_are_public = (; ignore = (:varmap_to_vars,)) name things that OptimizationMOI no longer imports at all. The PR adds to this list rather than pruning it. The qa.jl:12 comment ("ignored stale imports are part of the intentionally re-surfaced API") is now only true for reinit!. Not required for this PR.

  4. The orphaned comment at src/OptimizationMOI.jl:3-4 ("Not re-exported: …") is still accurate, since nothing is re-exported. Fine as is.

  5. Consistency with sibling work. The style matches merged OptimizationNLopt: make imports explicit #1398 (OptimizationNLopt) and the open drafts OptimizationBase: make implicit imports explicit, un-break no_implicit_imports QA check #1391, /1392, /1397, /1409, /1411. OptimizationNLopt: make imports explicit #1398 shrank the OptimizationNLopt.* namespace the same way, so whatever policy is chosen for solve/solve!/init here should be applied to those too.

What I ran

All runs used Julia 1.12.7 with TMPDIR set to the review tmp dir.

  • OPTIMIZATION_TEST_GROUP=Core julia --project -e 'using Pkg; Pkg.test()' in lib/OptimizationMOI → Core | 40 2 42 3m00.8s, tests passed. This matches the PR's 40/2/42; the 2 broken are the @test_broken reinit blocks that predate the PR. Log: scratch/core.log.
  • OPTIMIZATION_TEST_GROUP=QA julia --project -e 'using Pkg; Pkg.test()' → Quality Assurance | 21 21 1m22.7s, tests passed. This matches the PR (no_implicit_imports now passes, with no broken entries). Log: scratch/qa.log.
  • julia +1.12 --project=@runic -m Runic --check on both changed files → exit 0. typos on both → exit 0.
  • scratch/probe_globals.jl: walks the lowered code of all 86 methods defined in the PR's OptimizationMOI (found by scanning every loaded module's functions and types) and checks each GlobalRef into the module with isdefined. Result: no undefined globals, so nothing inside the package body lost a binding.
  • scratch/probe_lost.jl: lists the names that master's blanket using exposes and the PR drops (905).
  • scratch/probe_eval.jl against scratch/env_master and scratch/env_pr (both dev the matching OptimizationBase + OptimizationMOI; pathof checked for each) → the results in non-blocking finding 1.
  • isdefined(OptimizationMOI, n) side-by-side for master vs PR → the table in blocking finding 1.
  • gh search code for OptimizationMOI.<name> across GitHub, plus gh api reads of SciMLBenchmarks optimal_powerflow.jmd/Manifest and DyadControlSystems Project.toml/src/MPC/solver_optimization.jl.
  • Read the CI rollup and the logs of both failing jobs. Both are Pkg resolver failures unrelated to this diff: MTK downstream, and downgrade-sublibraries / test (lib/OptimizationReactant).

What I did not verify

  • I didn't run DyadControlSystems or SciMLBenchmarks against the PR. The breakage claim rests on their source text plus the isdefined result above.
  • Julia LTS (1.10) and 1.11. I only ran 1.12.7. CI reports Core passing on lts/1/pre and QA passing on Julia 1.
  • Other downstream packages, which may use dropped names other than solve/solve!/init. The code search found none, but it is not exhaustive and doesn't see private code outside the orgs I can read.
  • The repo has no AGENTS.md or CLAUDE.md at its root, so only the global CLAUDE.md rules applied.
  • Side effect of my verification: Pkg.instantiate created lib/OptimizationMOI/Manifest.toml in the checkout. It is gitignored (git status --ignored shows !!); nothing tracked was modified.

🤖 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 and others added 2 commits October 16, 2026 09:40
Resolve the OptimizationMOI import-block conflict: import
`parameter_index`, `parameter_values` and `variable_index` from
SymbolicIndexingInterface (used unqualified since SciML#1364) and drop the
`SciMLStructures` import, which master removed from `[deps]`.

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Harness: Claude Code 2.1.301
Agent-Model: claude-opus-5-5
Agent-Session: local session ID c5db64e8-0d72-4216-9d73-f5e719f32980 (headless claude -p, no URL)
Import `init`, `solve` and `solve!` from SciMLBase next to `reinit!` so
`OptimizationMOI.solve`/`.solve!`/`.init` keep resolving: downstream
code calls them through the OptimizationMOI namespace. A Core test
exercises all four through that namespace.

Remove the now-unused `Reexport` from `[deps]` and `[compat]`, delete
the comment that described the absent re-export, and prune the QA
ignore entries for names OptimizationMOI no longer imports
(`parameters`, `toexpr`, `unknowns`, `varmap_to_vars`).

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Harness: Claude Code 2.1.301
Agent-Model: claude-opus-5-5
Agent-Session: local session ID c5db64e8-0d72-4216-9d73-f5e719f32980 (headless claude -p, no URL)
ChrisRackauckas and others added 3 commits October 21, 2026 02:02
Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
Co-Authored-By: Codex <noreply@openai.com>
Agent-Harness: Codex CLI 0.163.0
Agent-Model: gpt-6.1-sol
Agent-Session: local session 01a15cef-d3c5-7e01-9f98-7fc338718267 (no session URL)
Import opnorm for runtime symbolic coefficient evaluation and cover analytic
objective and constraint solutions with repeated solves and cache preservation.

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
Co-Authored-By: Codex <noreply@openai.com>
Agent-Harness: Codex CLI 0.163.0
Agent-Model: gpt-6.1-sol
Agent-Session: local session 01a15cef-d3c5-7e01-9f98-7fc338718267 (no session URL)
Retain computational export namespaces alongside explicit static imports.
Bind SymbolicUtils directly and cover registered norm, tr, det, and rank
coefficients with analytic solutions, repeated solves, and cache assertions.
Record the generated-expression import constraint in local agent guidance.

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
Co-Authored-By: Codex <noreply@openai.com>
Agent-Harness: Codex CLI 0.163.0
Agent-Model: gpt-6.1-sol
Agent-Session: local session 01a15cef-d3c5-7e01-9f98-7fc338718267 (no session URL)

This branch has not been deployed

No deployments
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