Skip to content

QA: run_qa v1.6 form + ExplicitImports#232

Merged
ChrisRackauckas merged 2 commits into
SciML:masterfrom
ChrisRackauckas-Claude:qa-run_qa-v1.6
Jul 3, 2026
Merged

QA: run_qa v1.6 form + ExplicitImports#232
ChrisRackauckas merged 2 commits into
SciML:masterfrom
ChrisRackauckas-Claude:qa-run_qa-v1.6

Conversation

@ChrisRackauckas-Claude

Copy link
Copy Markdown
Contributor

Please ignore until reviewed by @ChrisRackauckas.

Converts the hand-rolled test/qa/qa.jl (raw Aqua.test_* + per-function JET.report_call) to the SciMLTesting 1.6 run_qa form and enables the ExplicitImports checks.

ExplicitImports findings (run locally vs released SciMLTesting 1.6.0)

Check Outcome
no_stale_explicit_imports fixed — removed stale ArrayInterface.allowed_getindex (imported, never used)
no_implicit_imports broken (#231) — ~31 implicit names from using LinearAlgebra, SparseArrays, Printf, PrecompileTools; explicit refactor deferred
all_explicit_imports_via_owners pass
all_qualified_accesses_via_owners ignoreBlasFloat/chkstride1/libblastrampoline reached via LinearAlgebra.BLAS (owners: LinearAlgebra / libblastrampoline_jll)
all_qualified_accesses_are_public ignore — Base / LinearAlgebra(.BLAS/.LAPACK) non-public names (@aliasscope, @assume_effects, @blasfunc, @propagate_inbounds, chkfinite, chklapackerror, gebal!, gesv!, rcswap!, stegr!, ...)
all_explicit_imports_are_public ignore — non-public names imported from LinearAlgebra / ArrayInterface / Base (BlasInt, checksquare, ismutable, allowed_setindex!, typename) + Stegr submodule's @blasfunc/stegr!

All ignored names are other packages' internals that will become public as the base libs declare them; only own-package surface is otherwise touched.

Analyzability fix

ExplicitImports could not analyze the module because of the dynamic for i in 1:13; include("exp_generated/exp_$i.jl"); end in src/exp_noalloc.jl (UnanalyzableModuleException). Replaced it with 13 literal includes. Verified the Higham2005 matrix exponential still matches Base exp to ~6.7e-16 and expv is unchanged. This unblocked no_implicit_imports and no_stale_explicit_imports.

Deps

  • test/qa/Project.toml: SciMLTesting compat → "1.6". Aqua + ExplicitImports are transitive via SciMLTesting; Aqua is kept a direct dep so the test_ambiguities child process can resolve it. JET kept for the JET check.
  • Root Project.toml: SciMLTesting compat → "1.6".

Verification

QA group on Julia 1.10 (lts), released SciMLTesting 1.6.0 (Pkg.test, GROUP=QA, folder model):

Test Summary: | Pass  Broken  Total
QA/qa.jl      |   17       1     18
Testing ExponentialUtilities tests passed

(the one Broken is no_implicit_imports, tracked in #231).

Pre-existing JET-on-1.12 caveat

On Julia 1.12 the JET typo check reports pre-existing "local variable may be undefined" findings (kiops order/kest, Higham2005 ilo/ihi/scale/bal). master is already red on the QA(1)/1.12 lane today, and the source-level fixes live in the still-open draft #229. This PR does not bundle that numerical-source fix (out of scope, belongs in #229) and does not use jet_broken (it would turn the clean lts JET lane into an Unexpected-Pass error). The lts QA lane is fully green; the 1.12 JET red is pre-existing and tracked by #229.

🤖 Generated with Claude Code

ChrisRackauckas and others added 2 commits June 29, 2026 06:03
Convert the hand-rolled test/qa/qa.jl (raw Aqua.test_* + per-function
JET.report_call) to the SciMLTesting 1.6 `run_qa` form and enable the
ExplicitImports checks.

ExplicitImports findings (run vs released SciMLTesting 1.6.0):
  * no_stale_explicit_imports: removed the genuinely stale
    `ArrayInterface.allowed_getindex` import (never referenced; only
    `ismutable`/`allowed_setindex!` are used).
  * Made the `for i in 1:13 include("exp_generated/exp_$i.jl")` dynamic
    include in exp_noalloc.jl static (13 literal includes) so the module is
    analyzable — this unblocked no_implicit_imports and
    no_stale_explicit_imports (previously UnanalyzableModuleException).
    Verified Higham2005 matrix-exp still matches Base `exp` to ~6.7e-16.
  * all_qualified_accesses_via_owners / all_qualified_accesses_are_public /
    all_explicit_imports_are_public: ignore-listed other packages' non-public
    names (Base / LinearAlgebra(.BLAS/.LAPACK, incl. Stegr submodule) /
    ArrayInterface / libblastrampoline_jll); they go public as the base libs
    declare them.
  * no_implicit_imports: ~31 implicit names from `using LinearAlgebra,
    SparseArrays, Printf, PrecompileTools`. Making them explicit is a large
    refactor; marked ei_broken and tracked in SciML#231 (auto-flags when fixed).

Deps: test/qa/Project.toml SciMLTesting compat -> "1.6" (Aqua + ExplicitImports
are transitive via SciMLTesting; Aqua kept a direct dep so the ambiguities
sub-check's child process can resolve it; JET kept for the JET check). Root
Project.toml SciMLTesting compat -> "1.6".

QA group on Julia 1.10 (lts), released SciMLTesting 1.6.0:
  Quality Assurance | 17 Pass, 1 Broken, 0 Fail, 0 Error (no_implicit_imports
  broken per SciML#231). On Julia 1.12 the JET typo check reports pre-existing
  "may be undefined" findings (kiops order/kest, Higham2005 ilo/ihi/scale/bal);
  master is already red there and the source fixes live in draft PR SciML#229.

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…Higham2005)

The run_qa v1.6 conversion runs JET in report_package typo mode, which analyzes
each method signature in isolation. The hand-rolled qa.jl this replaced used
JET.report_call(exponential!, (Matrix{Float64},)), where ExpMethodHigham2005(A)
sets do_balancing = (A isa StridedMatrix) as a constant that JET could
constant-propagate, so both `if method.do_balancing` blocks folded to true and
ilo/ihi/scale/bal were seen as always defined. In report_package the method is
analyzed with an abstract ExpMethodHigham2005, so do_balancing is a runtime
Bool, the two balancing blocks are not provably correlated, and the undo block
reads ilo/ihi/scale/bal as possibly-undefined locals (20 JET typo reports on
Julia 1.12; 1.10 abstract-interp did not reach them).

Seed ilo=1/ihi=n/scale=_scale as no-op defaults and lift the GenericSchur
row/col permutations into prow/pcol locals (nothing on the BLAS path, which
never reads them), so every local read in the symmetric undo block is
unconditionally defined. Behavior is unchanged: the seeds are only live when
do_balancing is false (where the undo block does not run), and the BLAS vs
GenericSchur branches use exactly the values they used before.

Verified Julia 1.12.6 (released SciMLTesting 1.7.0, JET 0.11.5): report_package
typo mode goes from 20 reports to 0. Verified Julia 1.10.11 numerics unchanged:
strided-BLAS balancing relerr 3.3e-16, GenericSchur (BigFloat) balancing relerr
1.1e-16, no-balancing relerr 1.6e-16 vs reference exp.

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
@ChrisRackauckas-Claude

Copy link
Copy Markdown
Contributor Author

QA(julia 1) red diagnosis + fix

Failing lane: tests / QA (julia 1, ubuntu-latest) — JET typo-mode reported 20 "local variable may be undefined" errors (JET.report_package(ExponentialUtilities; target_modules=(ExponentialUtilities,), mode=:typo)). Findings in src/kiops.jl (order, kest), src/exp_baseexp.jl (si), and src/exp_noalloc.jl (ilo/ihi/scale/bal).

Classification: conversion bug (run_qa's broader JET check surfaces genuine latent type-instabilities the old hand-rolled qa.jl never reached).

Two root causes:

  1. Stale base. This branch was cut from v1.30.1 (pre-Fix master CI: expv zero-input NaN, JET-on-1.12 QA, GPU-in-All #229). Master Fix master CI: expv zero-input NaN, JET-on-1.12 QA, GPU-in-All #229 already fixed the kiops order/kest and exp_baseexp si undefined locals, and Release v1.30.2 #233 released v1.30.2. Rebased onto current master to pick those up (the only conflict was qa.jl, resolved in favor of the run_qa form).

  2. exp_noalloc.jl ilo/ihi/scale/bal (not fixed by Fix master CI: expv zero-input NaN, JET-on-1.12 QA, GPU-in-All #229). The old qa.jl ran JET.report_call(exponential!, (Matrix{Float64},)). ExpMethodHigham2005(A::Matrix) sets do_balancing = (A isa StridedMatrix) as a constant, so JET constant-propagated do_balancing == true, both if method.do_balancing blocks folded to a single branch, and the balancing locals were always-defined — clean. run_qa's report_package analyzes exponential!(A, ::ExpMethodHigham2005, _cache) in isolation with an abstract ExpMethodHigham2005, so do_balancing is a runtime Bool, the two balancing blocks aren't provably correlated, and the undo block reads possibly-undefined locals. This was latent on master (its report_call never reached it) and is a real type-instability. Fixed by seeding ilo=1/ihi=n/scale=_scale and lifting the GenericSchur permutations into prow/pcol locals — behavior unchanged (seeds only live when do_balancing==false, where the undo block does not run).

Verified locally (released SciMLTesting 1.7.0):

  • Julia 1.12.6: report_package typo mode 20 → 0 reports ("No errors detected").
  • Julia 1.10.11 numerics unchanged: strided-BLAS balancing relerr 3.3e-16, GenericSchur/BigFloat balancing 1.1e-16, no-balancing 1.6e-16 vs reference exp.
  • Runic check passes on exp_noalloc.jl.

Note — Julia 1.10 EI (not a CI lane): with explicit_imports=true, the no_stale_explicit_imports check throws UnanalyzableModuleException on Julia 1.10 because ExplicitImports' static parser won't recurse into the generated-code includes in exp_noalloc.jl ("Dynamic include found … not recursing"). This reproduces byte-identically on master's for i in 1:13 include(...) loop, so it's a pre-existing ExplicitImports-on-1.10 limitation, not introduced here, and it does not affect CI: the centralized grouped-tests workflow schedules QA only on julia-1 (≥1.11), where the module is analyzable and the full QA group passes (Aqua + EI 5 pass/1 broken + JET).

Ignore until reviewed by @ChrisRackauckas.

@ChrisRackauckas-Claude ChrisRackauckas-Claude marked this pull request as ready for review July 3, 2026 00:36
@ChrisRackauckas ChrisRackauckas merged commit 9e519ec into SciML:master Jul 3, 2026
35 checks passed
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