QA: run_qa v1.6 form + ExplicitImports#34
Draft
ChrisRackauckas-Claude wants to merge 4 commits into
Draft
Conversation
Convert all four monorepo QA groups (umbrella + OUQBase/CanonicalMoments/ DiscreteMeasures) from hand-rolled `Aqua.test_all` + `JET.test_package` to `SciMLTesting.run_qa` (v1.6) with `explicit_imports = true`. Add SciMLTesting (compat "1.6") to each test/qa/Project.toml; Aqua + JET stay direct deps (ambiguities child-proc needs Aqua; JET runs); ExplicitImports is transitive via SciMLTesting. ExplicitImports findings, by package: - OptimalUncertaintyQuantification (umbrella): only `OUQBase` implicit, brought in by `@reexport using OUQBase`; ignored in `no_implicit_imports`. `using Reexport` narrowed to `using Reexport: @reexport`. QA 18/18. - DiscreteMeasures: zero findings. QA 18/18. - CanonicalMoments: FIX removed stale `import Base: inv` and the redundant `import Polynomials: denominator, numerator` (both owned by Base, already imported there), and made the implicit `using Polynomials/LinearAlgebra/ RecurrenceRelationships/Reexport` explicit. Remaining implicit `DiscreteMeasures`/`DiscreteMeasure` come from `@reexport using DiscreteMeasures`; ignored. QA 18/18. - OUQBase: FIX removed stale dep `PolynomialRoots` (never imported). `no_implicit_imports` (~40 names from heavy ModelingToolkit/Symbolics/ Optimization/JuMP `using`) is a large refactor, tracked as ei_broken (SciML#32). Pre-existing type piracy `CanonicalMoments.RawMomentSequence(::Symbolics.Num, ...)` tracked as aqua_broken=(:piracies,) (SciML#33). Upstream non-public/non-owner names ignored via ei_kwargs (each documented with its source package). QA 16 pass + 2 broken, 0 fail (was 2 fail on main: stale_deps + piracies). Verified locally on Julia 1.10 (lts) via the canonical `Pkg.test` GROUP=QA flow against released SciMLTesting 1.6.0. Runic-clean. Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The run_qa v1.6 conversion turns on JET's `mode = :typo` check (the previous QA used `JET.test_package(...; target_defined_modules = true)`, i.e. `:basic` mode, which does not flag undefined-name typos). Typo mode found two genuine, pre-existing bugs in `OUQBase` that the old config let through: - `winkler_extremal_measures.jl`: the `construct_optimization_problem` method for `WinklerExtremalMeasures` referenced the local `objective` in a `@debug` string *before* it was bound on the next line (`local variable `objective` is not defined`). Assign `objective` first, and interpolate the real field (`objective._obj`) to match the `canonical_moments.jl` sibling method. - `canonical_moments.jl`: the `ExpectationObjective` branch of the `StengerCanonicalMoments` method called bare `simplify(...)` (`OUQBase.simplify is not defined`); every other call site in the file uses `Symbolics.simplify`. Qualify it. Verified on Julia 1.11 against released SciMLTesting 1.6.0: `JET.report_package(OUQBase; target_modules = (OUQBase,), mode = :typo)` now returns 0 reports (was 2). Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
SciMLTesting 1.7.0 gates check_all_explicit_imports_are_public and
check_all_qualified_accesses_are_public to Julia >= 1.11, so the public-API
ignore-lists only matter on >= 1.11. Swept the two lists against the registered
releases on Julia 1.12 and removed the entries that are now genuinely public in
the versions OUQBase actually resolves:
- :wrap (now public in Symbolics 6.58.0) -- removed from both
all_explicit_imports_are_public and all_qualified_accesses_are_public
- :remove_linenums! (public in Base) -- removed from
all_qualified_accesses_are_public
Every other entry was re-flagged by the empty-list run and restored: OUQBase's
[compat] pins SciMLBase 2.x / Symbolics 6.x / SymbolicUtils 3.x /
ModelingToolkit 9.x, where NullParameters / value / NoAD / etc. are still
non-public (the public declarations landed in the later majors OUQBase's compat
excludes), and AbstractSupportAlg / AbstractWeightAlg are non-public siblings in
CanonicalMoments.
Verified via Pkg.test (GROUP=QA) on Julia 1.12 against registered releases
(SciMLTesting 1.7.0): Quality Assurance 16 Pass / 2 Broken / 0 Fail/Error.
Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…API survivors SciMLTesting 1.7.0 gates check_all_qualified_accesses_are_public and check_all_explicit_imports_are_public to Julia >= 1.11 (they silently skip on the LTS), so the qa envs should require >=1.7 for the gated behavior. Bumped the SciMLTesting [compat] floor "1.6" -> "1.7" in all four qa environments (root + CanonicalMoments + DiscreteMeasures + OUQBase). Re-swept OUQBase's two public-API EI ignore-lists against the registered releases on Julia 1.12 (SciMLBase 3.27 / Symbolics 7.29 / SymbolicUtils 4.36 / ModelingToolkit 11.29 all live, SciMLTesting 1.7.0). An empty-ignore-list run re-flagged every existing entry: OUQBase's [compat] caps SciMLBase 2.x / Symbolics 6.x / SymbolicUtils 3.x / ModelingToolkit 9.x, so the resolution lands on SciMLBase 2.153.1 / Symbolics 6.58.0 / SymbolicUtils 3.32.0 / ModelingToolkit 9.84.0, where NullParameters / NoAD / value / Operator / Term / symtype / BasicSymbolic / evaluate / geq / leq / getdefault / isbinop / promote_symtype are still not `public`-declared (the public declarations only landed in the later majors that [compat] excludes), and AbstractSupportAlg / AbstractWeightAlg are non-public siblings in CanonicalMoments. No name became public under the resolved versions, so the lists stay as-is; only their comment was updated to record the versions verified against. There are no DiffEqBase qualified accesses anywhere in src/ (NullParameters/NoAD are already accessed as SciMLBase.*), so there was nothing to migrate. Verified via the QA group on Julia 1.12 against the registered releases (SciMLTesting 1.7.0): Quality Assurance 16 Pass / 2 Broken / 0 Fail/Error. The 2 Broken are the pre-existing aqua piracies (SciML#33) and no_implicit_imports (SciML#32). On Julia 1.10 the two public-API checks skip (SciMLTesting 1.7 gating), so the ignore-lists are irrelevant there. CI runs QA only on Julia 1, where all four QA lanes (root + 3 sublibraries) pass. Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com> 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
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.
Please ignore until reviewed by @ChrisRackauckas.
Converts all four monorepo QA groups (umbrella
OptimalUncertaintyQuantification+ sublibrariesOUQBase,CanonicalMoments,DiscreteMeasures) from hand-rolledAqua.test_all+JET.test_packagetoSciMLTesting.run_qa(v1.6) withexplicit_imports = true.Each
test/qa/Project.tomlgainsSciMLTesting(compat"1.6");AquaandJETstay direct deps (Aqua's ambiguities/persistent-tasks child-proc needs Aqua; JET runs),ExplicitImportsis transitive through SciMLTesting.ExplicitImports findings
OUQBaseimplicit, from@reexport using OUQBase-> ignored inno_implicit_imports. Narrowedusing Reexporttousing Reexport: @reexport. QA 18/18.import Base: invand the redundantimport Polynomials: denominator, numerator(both owned byBase, already imported there); made the implicitusing Polynomials/LinearAlgebra/RecurrenceRelationships/Reexportexplicit. RemainingDiscreteMeasures/DiscreteMeasurecome from@reexport using DiscreteMeasures-> ignored. QA 18/18.PolynomialRoots(never imported).no_implicit_imports(~40 names from heavy ModelingToolkit/Symbolics/Optimization/JuMPusing) is a large refactor ->ei_broken(OUQBase: make implicit imports explicit (ExplicitImports no_implicit_imports) #32). Pre-existing type piracyCanonicalMoments.RawMomentSequence(::Symbolics.Num, ...)->aqua_broken = (:piracies,)(OUQBase: type piracy in RawMomentSequence(::Symbolics.Num, ...) constructor (Aqua piracies) #33). Upstream non-public / non-owner names ignored viaei_kwargs(each documented with its source package). QA 16 pass + 2 broken, 0 fail (was 2 fail onmain: stale_deps + piracies).Notes
main; the two Aqua failures (stalePolynomialRoots, piracy) are pre-existing. This PR fixes the stale dep and tracks the piracy as a documented@test_broken(OUQBase: type piracy in RawMomentSequence(::Symbolics.Num, ...) constructor (Aqua piracies) #33). The OUQBase Core failure is a separate pre-existing issue, out of scope here.Pkg.testGROUP=QA flow against released SciMLTesting 1.6.0 (no dev-from-branch). Runic-clean.🤖 Generated with Claude Code