Skip to content

Make the QA group scan all four package extensions - #193

Merged
ChrisRackauckas merged 1 commit into
SciML:masterfrom
ChrisRackauckas-Claude:qa-check-extensions
Aug 1, 2026
Merged

ChrisRackauckas merged 1 commit into
SciML:masterfrom
ChrisRackauckas-Claude:qa-check-extensions

Conversation

@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member

Please ignore this PR until it has been reviewed by @ChrisRackauckas.

Why

SciMLTesting.run_qa runs ExplicitImports' checks on the package module.
ExplicitImports does know about extensions — it reads the [extensions] table out
of Project.toml — but it only checks an extension module that actually exists:

ext_mod = Base.get_extension(mod, Symbol(ext))
ext_mod === nothing && continue

An extension module only comes into existence once its trigger weakdep is loaded, and
the QA environment loaded none of them. Today no PreallocationTools extension is
scanned by QA at all.
The fix is to put the weakdeps in test/qa/Project.toml and
using them in test/qa/qa.jl before run_qa.

Coverage

All four weakdeps are pure Julia and resolve cleanly, so all four extensions are now
scanned
— nothing is left out:

Extension Trigger Now scanned
PreallocationToolsEnzymeCoreExt EnzymeCore yes
PreallocationToolsForwardDiffExt ForwardDiff yes
PreallocationToolsReverseDiffExt ReverseDiff yes
PreallocationToolsSparseConnectivityTracerExt SparseConnectivityTracer yes

[compat] entries in test/qa/Project.toml mirror the root package's bounds
(EnzymeCore = "0.8", ForwardDiff = "0.10.38, 1.0.1", ReverseDiff = "1.16",
SparseConnectivityTracer = "1").

Fixed in the extension source (not ignored)

Two findings had a public spelling, so the source was fixed rather than ignored:

  • no_implicit_imports — PreallocationToolsForwardDiffExt did bare
    using ForwardDiff / using ArrayInterface / using PrecompileTools and then
    referred to ForwardDiff.Dual, ArrayInterface.restructure, @setup_workload and
    @compile_workload. These are now using ForwardDiff: ForwardDiff,
    using ArrayInterface: ArrayInterface and
    using PrecompileTools: @setup_workload, @compile_workload.
    using Adapt is dropped: the extension never referenced Adapt (it is used only
    in src/PreallocationTools.jl), so keeping it as an explicit import would have
    tripped no_stale_explicit_imports.
  • all_qualified_accesses_are_public: Core.apply_type — replaced with
    wrapper{new_parameters...}, which is exactly the same operation spelled with public
    syntax (Core.apply_type is the intrinsic behind T{...}).

Ignore entries added, with justification

Every remaining finding is a name that is genuinely non-public at its owner and has no
public equivalent. Each is grouped under a comment naming the owner in qa.jl.

all_explicit_imports_are_public:

Symbol Owner Why unavoidable
:EnzymeRules EnzymeCore EnzymeCore neither exports nor declares the submodule public, yet it is the only entry point for defining Enzyme custom rules.
:AbstractTracer, :Dual SparseConnectivityTracer SCT exports only TracerSparsityDetector, TracerLocalSparsityDetector, jacobian_sparsity, hessian_sparsity, jacobian_eltype, hessian_eltype, jacobian_buffer, hessian_buffer. These two types are what a get_tmp method must dispatch on for sparsity detection to reach the cache.

all_qualified_accesses_are_public:

Symbol Owner Why unavoidable
:forward, :augmented_primal, :reverse EnzymeCore.EnzymeRules Declared as bare function ... end stubs and not exported. Adding methods to them is the documented way to write a custom rule.
:Dual, :pickchunksize ForwardDiff ForwardDiff exports only DiffResults. Dual is the type every dual-cache method dispatches on; pickchunksize is the chunk heuristic the cache sizing mirrors.
:TrackedArray ReverseDiff ReverseDiff also exports only DiffResults; TrackedArray is the type the LazyBufferCache method keys on.
:typename Base The only way to recover a DataType's UnionAll wrapper so its type parameters can be substituted. Base offers no public equivalent.

No check was disabled, and no @test_broken / explicit_imports = false escape hatch
was used.

Local verification

GROUP=QA julia --project=. -e 'using Pkg; Pkg.test()' on this branch (Julia 1.12):

Test Summary: | Pass  Total     Time
QA            |   27     27  1m28.9s
     Testing PreallocationTools tests passed

(For reference, the same command with the weakdeps added but before the fixes reported
24 passed, 3 errored, which is the previously-invisible extension debt this PR pays
off.)

The Core.apply_type → wrapper{...} rewrite was additionally checked directly against
replace_type_parameter for Float64, Complex{Float64}, Vector{Complex{Float64}}
and String, all unchanged.

test/qa/Manifest.toml is a local build artifact and is not committed (Manifest.toml
is already in .gitignore). Runic was run on both touched files.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Yb5kCpT5SRzTrhppKSh1n7

ExplicitImports reads the `[extensions]` table out of `Project.toml`, but it
only checks an extension module that actually exists -- which requires the
extension's trigger package to be loaded. The QA environment loaded no
weakdeps, so none of the four extensions were being checked at all.

Add EnzymeCore, ForwardDiff, ReverseDiff and SparseConnectivityTracer to
`test/qa/Project.toml` and load them in `qa.jl`, which brings all four
extensions under the ExplicitImports checks.

Two source fixes for findings that had a public spelling:

  * `PreallocationToolsForwardDiffExt` relied on implicit imports for
    `ForwardDiff`, `ArrayInterface`, `@setup_workload` and
    `@compile_workload`; these are now explicit. `using Adapt` is dropped
    because the extension never used it (Adapt is only used in `src/`).
  * `Core.apply_type(wrapper, params...)` is spelled `wrapper{params...}`,
    which is the same operation via public syntax.

The remaining findings are names that are genuinely non-public at their owner
with no public equivalent (Enzyme's custom-rule interface, the AD backends'
dual/tracer/tracked-array types, `Base.typename`), so they are added to the
per-check `ignore` lists with a comment naming the owner in each case.

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
@ChrisRackauckas
ChrisRackauckas marked this pull request as ready for review August 1, 2026 08:51
@ChrisRackauckas
ChrisRackauckas merged commit 4ca018b into SciML:master Aug 1, 2026
9 of 17 checks passed
@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member Author

Core group status (informational)

I also ran GROUP=Core locally on this branch, since it touches extension source. It fails, but the identical failure reproduces on unmodified master (ac5aeb9), so it is not caused by this PR:

DiffCache Nested Duals: Error During Test
  LoadError: UndefVarError: `issquare` not defined in `LinearSolveForwardDiffExt`
  Hint: a global variable of this name also exists in SciMLOperators.
ERROR: LoadError: Some tests did not pass: 2 passed, 0 failed, 1 errored, 0 broken.
Test Summary:          | Pass  Error  Total     Time
DiffCache Nested Duals |    2      1      3  2m18.7s

Both runs (master and this branch) produce byte-identical failure output. The root cause is upstream in LinearSolveForwardDiffExt, with LinearSolve v5.3.0 / SciMLOperators v1.25.2 in the resolved manifest. A separate investigation is tracking it; it is out of scope here.

Every other Core testset passed on this branch: Developer Interface (16), DiffCache Dispatch (82 pass / 10 broken), DiffCache ODE tests (7), DiffCache Resizing (64).

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