Skip to content

QA: scan PureKLUForwardDiffExt (load ForwardDiff in the QA env) - #74

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

QA: scan PureKLUForwardDiffExt (load ForwardDiff in the QA env)#74
ChrisRackauckas merged 1 commit into
SciML:mainfrom
ChrisRackauckas-Claude:qa-check-extensions

Conversation

@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member

Please ignore this PR until reviewed by @ChrisRackauckas.

Problem

SciMLTesting.run_qa runs ExplicitImports over the package's submodules. ExplicitImports does know about extensions — it reads the [extensions] table out of Project.toml — but it skips any extension whose module does not exist:

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

An extension module only exists once its trigger weakdep is loaded, and the QA environment never loaded ForwardDiff. So PureKLUForwardDiffExt was getting zero ExplicitImports coverage, while QA reported a clean pass.

Fix

  • Add ForwardDiff to test/qa/Project.toml ([deps] + [compat] = "1", mirroring the root compat).
  • using ForwardDiff in test/qa/qa.jl before run_qa, with a comment explaining why.

Proof the extension is now actually scanned

A passing QA summary is not proof — each ExplicitImports check folds every submodule into one @test, so a clean extension yields an identical summary either way. Direct check from test/qa:

=== WITH ForwardDiff loaded (this PR) ===
get_extension(PureKLU, :PureKLUForwardDiffExt) => PureKLUForwardDiffExt
modules scanned: Module[PureKLUForwardDiffExt, PureKLU, PureKLU.AMD, PureKLU.BTF]

=== WITHOUT ForwardDiff loaded (main today) ===
get_extension(PureKLU, :PureKLUForwardDiffExt) => nothing
modules scanned: Module[PureKLU, PureKLU.AMD, PureKLU.BTF]

Findings surfaced, and what was done with each

Turning the scan on produced 3 errored checks. All were addressed:

Fixed in source (not ignored):

  1. no_stale_explicit_importsAbstractKLUFactorization was imported by ext/PureKLUForwardDiffExt.jl and never used. Removed from the import list.
  2. all_explicit_imports_are_publicKLUFactorization is not public in PureKLU. It is documented API (it is what klu returns, and it has a @docs entry in docs/src/api.md), it just was never declared public. Declared it in src/PureKLU.jl, guarded for the 1.10 LTS where the public keyword does not exist. It deliberately stays unexported, since KLU.jl exports the same name and PureKLU is a drop-in for it. This adds public API, hence the 1.2.01.3.0 minor bump.

Ignored, with justification:

  1. all_explicit_imports_are_public / all_qualified_accesses_are_publicForwardDiff.Dual, ForwardDiff.Partials, ForwardDiff.value, ForwardDiff.partials. ForwardDiff v1 declares no name public and exports none of its dual-number interface (names(ForwardDiff) == [:DiffResults, :ForwardDiff]), so there is no public spelling for the types and accessors any AD extension must dispatch on and unpack. Added as a single commented FORWARDDIFF_NONPUBLIC tuple applied to both public-API checks. No check was disabled and nothing was marked @test_broken.

Coverage

  • Now scanned: PureKLUForwardDiffExt (the package's only extension).
  • Still unscanned: none.

Local results

$ GROUP=QA julia --project=. -e 'using Pkg; Pkg.test()'
Test Summary: | Pass  Total   Time
QA/qa.jl      |   20     20  39.4s
     Testing PureKLU tests passed

GROUP=Core also run, to confirm the extension edit and the public declaration break nothing:

Core/test_allocations.jl        |   16
Core/test_check_false.jl        |    8
Core/test_compare_klu.jl        |  710
Core/test_fma_on.jl             |  396
Core/test_full_factor_false.jl  |    4
Core/test_fully_preallocated.jl |   40
Core/test_generic_eltypes.jl    |  167
Core/test_interfaces.jl         |   10
Core/test_issue4.jl             |    1
Core/test_klu_wrappers.jl       |  137
Core/test_phases.jl             |  678
Core/test_singular_fastexit.jl  |   56
     Testing PureKLU tests passed

(Julia 1.12, ForwardDiff v1.4.3.)

Also gitignored test/qa/Manifest.toml, which the QA run generates.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Yb5kCpT5SRzTrhppKSh1n7

ExplicitImports only adds an extension module to the set it checks when
`Base.get_extension` returns non-`nothing`, which requires the trigger
package to be loaded. The QA environment never loaded ForwardDiff, so
`PureKLUForwardDiffExt` was not scanned at all.

Add ForwardDiff to `test/qa/Project.toml` and load it in `qa.jl`. The
newly-surfaced findings:

* `AbstractKLUFactorization` was an unused import in the extension --
  removed.
* `KLUFactorization` is documented API (`klu`'s return type) but was
  never declared `public`; declare it. It stays unexported because
  KLU.jl exports the same name.
* ForwardDiff declares nothing `public` and exports none of its
  dual-number interface, so `Dual`, `Partials`, `value` and `partials`
  have no public spelling -- ignored in `ei_kwargs`.

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
@ChrisRackauckas
ChrisRackauckas marked this pull request as ready for review August 1, 2026 08:27
@ChrisRackauckas
ChrisRackauckas merged commit 755f173 into SciML:main Aug 1, 2026
6 of 7 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