Fix sublibrary QA (Aqua/JET) findings in OUQBase and CanonicalMoments#27
Draft
ChrisRackauckas-Claude wants to merge 1 commit into
Draft
Conversation
The `sublibraries / [QA]` checks (added in SciML#17) fail on `main` with real Aqua/JET findings. Fix them at the source: OUQBase - Aqua stale dep: drop the unused `PolynomialRoots` dependency (and its compat) from `lib/OUQBase/Project.toml`; it is not referenced anywhere in src or test. - Aqua piracy: rename the pirate `CanonicalMoments.RawMomentSequence(::Num, ::Vector{...})` method (a constructor on a foreign type with foreign arg types) to an internal `build_raw_moment_sequence`; it was only ever called inside OUQBase, so behavior is unchanged. - JET `OUQBase.simplify is not defined`: qualify the bare `simplify` call as `Symbolics.simplify` (the bare binding is ambiguous between the ModelingToolkit and Symbolics `using` imports). - JET `local variable objective is not defined`: assign `objective = ouq_sys.objective` before the `@debug` that interpolates it. - JET `getindex(::Nothing, ::Any)`: make `raw_moments_map`/`p_free_map` dispatch on the concretely typed `reduction_data` instead of a runtime `isa` ternary, and read the maps off the concrete `StengerCanonicalMoments` arg inside `discrete_measure_map`, so the accessors stay type stable. - JET `convert(...)` union split in the `OUQSystem` constructor: route the `process_objective` result through a `_OUQSystem` function barrier so the struct's objective field is built with a concrete type parameter. CanonicalMoments - JET `no matching method found kwcall(...)`: `DEFAULT_ROOT_SOLVER` forwarded `args...; kwargs...` to the closed-form solvers (`quadratic_eq_sridhare`, `simple_real_cubic_eq`, `simple_real_quartic_eq`), which take only the coefficient vector. Only the generic `Polynomials.roots` fallback consumes those options, so pass them only there. Verified locally: - OUQBase QA on Julia 1.12: Aqua 11/11, JET 1/1 (was 9/3 + 7 JET errors). - CanonicalMoments JET on Julia 1.10 (lts): 1/1 (was 3 errors). - CanonicalMoments analytic root-solver branches still return correct roots. - OUQBase Core `Pkg.test()` on Julia 1.12 passes (canonical-moments path). Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Status update (CI triage 2026-06-20): the four
|
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.
Problem
The
sublibraries / lib/OUQBase [QA]andsublibraries / lib/CanonicalMoments [QA]checks (added in #17) fail onmainwith real Aqua/JET findings — they are not harness bugs. This PR fixes them at the source.OUQBase QA findings (Aqua 9 pass / 3 fail + 7 JET errors)
PolynomialRootsis declared in[deps]but never used anywhere insrcortest. Removed it (and its compat).CanonicalMoments.RawMomentSequence(::Num, ::Vector{Union{Equation,Inequality}})defined a constructor on a foreign type with foreign argument types. Renamed to an internalbuild_raw_moment_sequence; it was only ever called inside OUQBase, so behavior is unchanged.OUQBase.simplify is not defined: the baresimplifybinding is ambiguous between theusing ModelingToolkitandusing Symbolicsimports. Qualified it asSymbolics.simplify(matching every othersimplifycall in the file).local variable objective is not defined: a@debuginterpolatedobjectiveone line beforeobjective = ouq_sys.objective. Reordered.getindex(::Nothing, ::Any):raw_moments_map/p_free_mapreturnedUnion{Nothing, OrderedDict}via a runtimeisaternary. Made them dispatch on the concretely typedreduction_data, and read the maps off the concreteStengerCanonicalMomentsargument insidediscrete_measure_map, so they stay type stable.convert(...)union split in theOUQSystemconstructor:process_objectivereturns aUnion, so routed it through a_OUQSystemfunction barrier to build the struct with a concrete objective type parameter.CanonicalMoments QA finding (3 JET errors)
no matching method found kwcall(...):DEFAULT_ROOT_SOLVERforwardedargs...; kwargs...to the closed-form solvers (quadratic_eq_sridhare,simple_real_cubic_eq,simple_real_quartic_eq), which accept only the coefficient vector. Only the genericPolynomials.rootsfallback consumes those options, so they are now passed only there.Local verification
orthopoly_roots.jlCore tests pass 11/11).Pkg.test()on Julia 1.12 passes, including the canonical-moments objective test that exercises the refactored accessor / constructor path.Scope / remaining
mainreds (NOT addressed here)These have different root causes and need cross-repo coordination, so they are out of scope for this focused PR:
tests / Core (julia lts)andtests / QA (julia lts)fail atjulia-buildpkgwithexpected package CanonicalMoments ... to be registered. The centralizedSciML/.githubtests.ymlskips developing in-repo[sources]path deps whenproject == '.', and on Julia 1.10[sources]is not auto-resolved — aSciML/.githubworkflow issue.Downgrade/Downgrade Sublibrariesfail becausejulia-actions/julia-downgrade-compat@v2errors withunknown package UUIDon the unregistered in-repo[sources]package — an upstream action limitation.tests / QA (julia 1)GROUP=QA dispatch is fixed by the separate Route umbrella QA group through theqakeyword (fixes GROUP=QA dispatch) #22.Please ignore until reviewed by @ChrisRackauckas.