Add compat bounds, ignore certain piracies, fix Aqua QA sublibrary tests#1230
Merged
Conversation
| @testset "Aqua" begin | ||
| Aqua.test_all(OptimizationNLopt) | ||
| # Extending SciMLBase traits onto NLopt optimizer types is the entire purpose | ||
| # of this package, so flag those types as "our own" for Aqua's piracy check. |
Member
There was a problem hiding this comment.
No, add the SciML parts os own.
jClugstor
force-pushed
the
sublibrary_qa_fixes
branch
from
a965256 to
6c0ceb6
Compare
Re-land the sublibrary QA work on a clean master base and fix the
Downgrade Sublibraries CI failures:
- Repair duplicate TOML tables/keys from a bad merge that made
Project.toml unparseable (SimpleOptimization, OptimizationManopt,
OptimizationODE).
- Skip QuadDIRECT in the downgrade workflow: it is unregistered, so the
resolver cannot look up its versions.
- Raise compat lower bounds that fell below what the local OptimizationBase
5.1.4 (and local sibling libs) require, which broke the downgrade resolve:
* SymbolicIndexingInterface -> 0.3.46 (MadNLP, MOI, Ipopt)
* SciMLBase -> 2.122.1 (Polyalgorithms, NOMAD); -> 2.130 (NLPModels)
* Reexport -> 1.2.2 (Auglag, Polyalgorithms; OptimizationOptimisers pins 1.2.2)
* DocStringExtensions -> 0.9.5 in OptimizationBase (LBFGSB test dep needs it)
OptimizationManopt src edits remain excluded (split into a separate PR).
Known remaining downgrade failures for OptimizationBBO and
OptimizationEvolutionary are a transitive registry incompatibility
(LogExpFunctions 1.0.1 vs SparseConnectivityTracer vs DifferentiationInterface),
tracked separately.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jClugstor
force-pushed
the
sublibrary_qa_fixes
branch
from
6c0ceb6 to
ee53c71
Compare
Under the downgrade resolve, MathOptInterface was pinned to its floor (effectively 1.43.0), whose transitive dependency closure caps SparseMatrixColorings at <= 0.4.22. But OptimizationBase floors DifferentiationInterface at 0.7.13, and DifferentiationInterface >= 0.7.12 requires SparseMatrixColorings >= 0.4.23 (via its extension), leaving no satisfiable SparseMatrixColorings version. Bisecting against the registry, MathOptInterface 1.45.0 is the lowest version that resolves alongside SparseMatrixColorings >= 0.4.23, so raise the floor to 1.45. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
More Downgrade Sublibraries fixes. The downgrade resolver pins each dep to its compat floor, so any sublibrary floor below what the local OptimizationBase 5.1.4 requires makes the resolve unsatisfiable: - OptimizationMadNLP: ADTypes 1 -> 1.14, DifferentiationInterface 0.7 -> 0.7.13, SciMLBase 2.122.1 -> 2.130 (its OptimizationNLPModels dep needs it). - OptimizationManopt: DifferentiationInterface 0.7 -> 0.7.13. - SimpleOptimization: ADTypes 1 -> 1.14. Also: - OptimizationQuadDIRECT: add git [sources] for the unregistered QuadDIRECT so the resolver can find it (the skip list alone can't). - Skip LogExpFunctions in the downgrade: it gets pinned to its latest (1.0.1), but SparseConnectivityTracer (transitive via OptimizationBase, pulled in by BlackBoxOptim/Evolutionary/NOMAD) only supports LogExpFunctions 0.3 via its extension, making the resolve unsatisfiable. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address the JET/Aqua static-analysis failures across sublibraries: JET "possibly undefined local" (conditional definition / closure boxing): - GCMAES: define the gradient closure `g` unconditionally (the QA pass's `local g` + conditional assignment is what JET flagged). - NLopt: drop the theta-memoization and write constraints/Jacobian into the preallocated caches in place, so no captured variable is reassigned/boxed. - OptimizationBase (DISparseExt): hoist `cons_oop`/`lagrangian` out of the `if f.cons === nothing` else-branch so they are defined on all paths. JET "no matching method ... ::Nothing": - PRIMA: `get_solve_func` now errors in an else branch, so its return type (and `optfunc`) no longer includes Nothing. - NOMAD: merge the `bb`/`bbcons` definition and the NomadProblem construction into a single if/else so the callback isn't seen as possibly-undefined. Real bugs surfaced by JET: - SciPy: `pytypeof` -> `pytype` (pytypeof does not exist in PythonCall; the fallback branch would have errored). - SimpleOptimization: guard `open_brkt_ind` against `nothing` before subtracting, in the AD-package-not-loaded error path. Aqua / syntax: - Manopt: remove the stray second `;` in the `Aqua.test_all` call (syntax error). - QuadDIRECT: add `Test` compat (Aqua check_extras) and raise SciMLBase floor to 2.122.1 to match OptimizationBase. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- OptimizationManopt: add [compat] for the Aqua-check_extras extras (Aqua,
QuadraticModels, Random, ReverseDiff, RipQP, Test, Zygote); its Aqua run was
previously masked by the qa.jl syntax error.
- OptimizationBase: add Mooncake weakdep compat ("0.4.138, 0.5", matching the
root project) for Aqua check_weakdeps.
- OptimizationBase (function.jl): guard `open_brkt_ind` against `nothing`
before subtracting in the AD-package-not-loaded error path. JET could not
prove the elseif branches return an Int, so it flagged `-(::Nothing, ::Int64)`
(same fix as SimpleOptimization, which copied this code).
With this, the earlier DISparseExt hoist cleared OptimizationBase's 6
cons_oop/lagrangian JET findings, leaving only this one, now fixed.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Aqua now reports "Unexpected Pass" for OptimizationManopt's undefined_exports check: the `solve!` reexport clash that motivated `broken = true` no longer produces an undefined export, so the broken marker errors the test. Remove it and let the check run normally. Verified the other libs with the same marker are still genuinely broken and keep theirs: OptimizationNOMAD (`OptimizationNOMAD.solve` is undefined), plus OptimizationMadNLP/OptimizationMetaheuristics (their QA jobs pass with the marker in place). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Per review feedback: these wrapper packages don't own the third-party solver types, so claiming them for Aqua's piracy check is wrong (and would mask real piracy on those types). Instead mark the SciML interface functions the package *implements* as own — that's what the methods actually extend. For each package, treat_as_own now lists the SciMLBase / OptimizationBase functions it defines methods for (traits like allowsbounds/requiresgradient, plus __init/__solve, supports_sense, etc.) instead of the solver types: NLopt, Optimisers, OptimJL, Metaheuristics, Evolutionary, MultistartOptimization, MOI, NLPModels. Two packages genuinely extend a *foreign* function on a foreign type (no SciML function to attribute it to), so those keep a targeted entry: - NLopt: `NLopt.Algorithm` for the `(::NLopt.Algorithm)()` normalization method. - Evolutionary: `Evolutionary.trace!` (hooks Evolutionary's tracing into callbacks). Verified locally with Aqua.Piracy.hunt (0 piracies) for NLopt, Evolutionary, and NLPModels — covering the functor, foreign-function, and constructor cases. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Now that JET passes, OptimizationBase's Aqua run surfaces two issues: - Piracy: OptimizationBase defines the Optimization solve interface, extending SciMLBase.__init/__solve and CommonSolve.init/solve/solve! (the latter reexported via SciMLBase) on SciMLBase.OptimizationProblem / AbstractOptimizationCache. These are our own interface functions, so mark them as own for the piracy check (verified 0 piracies locally with Aqua.Piracy.hunt). - Stale dependency: PDMats was declared in [deps]/[compat] but is not referenced anywhere in the package (src or extensions); remove it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Keep the workflow YAML identical to master (skip: "SymbolicAnalysis" only). The QuadDIRECT/LogExpFunctions downgrade handling is dropped from the workflow per request; the compat-floor bumps and QuadDIRECT's git [sources] entry in its own Project.toml remain. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The Evolutionary.trace! override dispatches Evolutionary's own function on Evolutionary.AbstractOptimizer, changing Evolutionary's tracing globally — that is genuine type piracy, not a SciML-interface extension, so it shouldn't be suppressed via treat_as_own. Keep treat_as_own for the actual SciML trait methods and mark the piracy test `broken = true` so the trace! piracy is tracked rather than hidden. It should be replaced with a non-pirating callback hook. Verified with Aqua.test_piracies: recorded as Broken (not Fail/Error). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Checklist
contributor guidelines, in particular the SciML Style Guide and
COLPRAC.
Additional context
Add any other context about the problem here.