Add QA tests with Aqua, ExplicitImports and JET

.github/workflows/Downgrade.yml

@@ -35,7 +35,7 @@ jobs:
       - uses: julia-actions/julia-downgrade-compat@v2
         with:
           projects: ".,test"
-          skip: LinearAlgebra,Random,Statistics,Test
+          skip: JET,LinearAlgebra,Random,Statistics,Test
         name: Downgrade dependencies to oldest supported versions
       - uses: julia-actions/julia-buildpkg@v1
         with:

README.md

@@ -4,6 +4,8 @@
 [![Dev](https://img.shields.io/badge/docs-dev-blue.svg)](https://juliadiff.org/ReverseDiff.jl/dev)
 [![Build status](https://github.com/JuliaDiff/ReverseDiff.jl/workflows/CI/badge.svg)](https://github.com/JuliaDiff/ReverseDiff.jl/actions)
 [![codecov.io](https://codecov.io/github/JuliaDiff/ReverseDiff.jl/coverage.svg?branch=master)](https://codecov.io/github/JuliaDiff/ReverseDiff.jl?branch=master)
+[![Aqua QA](https://raw.githubusercontent.com/JuliaTesting/Aqua.jl/master/badge.svg)](https://github.com/JuliaTesting/Aqua.jl)
+[![JET QA](https://img.shields.io/badge/%F0%9F%9B%A9%EF%B8%8F_tested_with-JET.jl-233f9a)](https://github.com/aviatesk/JET.jl)
 
 [**See ReverseDiff Usage Examples**](https://github.com/JuliaDiff/ReverseDiff.jl/tree/master/examples)
 

src/ReverseDiff.jl

@@ -1,24 +1,18 @@
 module ReverseDiff
 
 using Base: RefValue
-using Random
-using LinearAlgebra
-
+using ChainRulesCore: ChainRulesCore, AbstractThunk, unthunk
+using DiffResults: DiffResults, DiffResult
+using DiffRules: DiffRules
+using ForwardDiff: ForwardDiff, Dual
 using FunctionWrappers: FunctionWrapper
-
-using DiffResults
-using DiffResults: DiffResult
-using DiffRules, SpecialFunctions, NaNMath
-
-using ForwardDiff
-using ForwardDiff: Dual, Partials
-using StaticArrays
-
+using LinearAlgebra: LinearAlgebra, Adjoint, Transpose, det, dot, lmul!, mul!
 using LogExpFunctions: LogExpFunctions
-
-using MacroTools
-
-using ChainRulesCore
+using MacroTools: MacroTools, @capture
+using NaNMath: NaNMath
+using Random: Random
+using SpecialFunctions: SpecialFunctions
+using StaticArrays: StaticArrays, SVector
 
 # Not all operations will be valid over all of these types, but that's okay; such cases
 # will simply error when they hit the original operation in the overloaded definition.

src/derivatives/broadcast.jl

@@ -2,7 +2,7 @@
 ## Broadcasting ##
 ##################
 
-using Base.Broadcast: BroadcastStyle, ArrayStyle, Broadcasted, broadcasted
+using Base.Broadcast: BroadcastStyle, Broadcasted, broadcasted
 using ForwardDiff: ForwardDiff, Dual
 import Base.Broadcast: materialize
 const RDBroadcasted{F, T} = Broadcasted{<:Any, <:Any, F, T}

src/derivatives/linalg/arithmetic.jl

@@ -340,4 +340,4 @@ end
 
 ## zero
 
-Base.zero(x::ReverseDiff.TrackedArray) = track(zero(x.value))
+Base.zero(x::TrackedArray) = track(zero(x.value))

src/derivatives/linalg/special.jl

@@ -52,7 +52,7 @@ end
 # inv #
 #######
 
-function LinearAlgebra.inv(x::TrackedArray{V,D}) where {V,D}
+function Base.inv(x::TrackedArray{V,D}) where {V,D}
     tp = tape(x)
     out_value = inv(value(x))
     out = track(out_value, D, tp)

src/macros.jl

@@ -331,16 +331,16 @@ macro grad_from_chainrules(fcall)
     end
     args_l, args_r, args_track, args_fixed, arg_types, kwargs = _make_fwd_args(f, xs)
     return quote
-        $f($(args_l...)) = ReverseDiff.track($(args_r...))
-        function ReverseDiff.track($(args_track...))
+        $f($(args_l...)) = $ReverseDiff.track($(args_r...))
+        function $ReverseDiff.track($(args_track...))
             args = ($(args_fixed...),)
-            tp = ReverseDiff.tape(args...)
-            output_value, back = ChainRulesCore.rrule($f, map(ReverseDiff.value, args)...; $kwargs...)
-            output = ReverseDiff.track(output_value, tp)
-            closure(cls_args...; cls_kwargs...) = ChainRulesCore.rrule($f, map(ReverseDiff.value, cls_args)...; cls_kwargs...)
-            ReverseDiff.record!(
+            tp = $ReverseDiff.tape(args...)
+            output_value, back = $ChainRulesCore.rrule($f, map($ReverseDiff.value, args)...; $kwargs...)
+            output = $ReverseDiff.track(output_value, tp)
+            closure(cls_args...; cls_kwargs...) = $ChainRulesCore.rrule($f, map($ReverseDiff.value, cls_args)...; cls_kwargs...)
+            $ReverseDiff.record!(
                 tp,
-                ReverseDiff.SpecialInstruction,
+                $ReverseDiff.SpecialInstruction,
                 $f,
                 args,
                 output,
@@ -349,25 +349,25 @@ macro grad_from_chainrules(fcall)
             return output
         end
 
-        @noinline function ReverseDiff.special_reverse_exec!(instruction::ReverseDiff.SpecialInstruction{typeof($f), <:Tuple{$(arg_types...)}})
+        @noinline function $ReverseDiff.special_reverse_exec!(instruction::$ReverseDiff.SpecialInstruction{typeof($f), <:Tuple{$(arg_types...)}})
             output = instruction.output
             input = instruction.input
             back = instruction.cache[1]
-            back_output = back(ReverseDiff.deriv(output))
+            back_output = back($ReverseDiff.deriv(output))
             input_derivs = back_output[2:end]
             @assert input_derivs isa Tuple
-            ReverseDiff._add_to_deriv!.(input, input_derivs)
-            ReverseDiff.unseed!(output)
+            $ReverseDiff._add_to_deriv!.(input, input_derivs)
+            $ReverseDiff.unseed!(output)
             return nothing
         end
 
-        @noinline function ReverseDiff.special_forward_exec!(instruction::ReverseDiff.SpecialInstruction{typeof($f), <:Tuple{$(arg_types...)}})
+        @noinline function $ReverseDiff.special_forward_exec!(instruction::$ReverseDiff.SpecialInstruction{typeof($f), <:Tuple{$(arg_types...)}})
             output, input = instruction.output, instruction.input
-            ReverseDiff.pull_value!.(input)
+            $ReverseDiff.pull_value!.(input)
             pullback = instruction.cache[2]
             kwargs = instruction.cache[3]
             out_value = pullback(input...; kwargs...)[1]
-            ReverseDiff.value!(output, out_value)
+            $ReverseDiff.value!(output, out_value)
             return nothing
         end
     end

test/Project.toml

@@ -1,35 +1,43 @@
 [deps]
+Aqua = "4c88cf16-eb10-579e-8560-4a9242c79595"
 ChainRulesCore = "d360d2e6-b24c-11e9-a2a3-2a2ae2dbcce4"
 DiffResults = "163ba53b-c6d8-5494-b064-1a9d43ac40c5"
 DiffRules = "b552c78f-8df3-52c6-915a-8e097449b14b"
 DiffTests = "de460e47-3fe3-5279-bb4a-814414816d5d"
+ExplicitImports = "7d51a73a-1435-4ff3-83d9-f097790105c7"
 FillArrays = "1a297f60-69ca-5386-bcde-b61e274b549b"
 ForwardDiff = "f6369f11-7733-5829-9624-2563aa707210"
 IrrationalConstants = "92d709cd-6900-40b7-9082-c6be49f344b6"
+JET = "c3a54625-cd67-489e-a8e7-0a5a0ff4e31b"
 LinearAlgebra = "37e2e46d-f89d-539d-b4ee-838fcccc9c8e"
 LogExpFunctions = "2ab3a3ac-af41-5b50-aa03-7779005ae688"
 NaNMath = "77ba4419-2d1f-58cd-9bb1-8ffee604a2e3"
 Random = "9a3f8284-a2c9-5f02-9a11-845980a1fd5c"
 ReverseDiff = "37e2e3b7-166d-5795-8a7a-e32c996b4267"
 SpecialFunctions = "276daf66-3868-5448-9aa4-cd146d93841b"
 StaticArrays = "90137ffa-7385-5640-81b9-e52037218182"
+StaticArraysCore = "1e83bf80-4336-4d27-bf5d-d5a4f845583c"
 Statistics = "10745b16-79ce-11e8-11f9-7d13ad32a3b2"
 Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"
 
 [compat]
+Aqua = "0.8.16"
 ChainRulesCore = "1"
 DiffResults = "1"
 DiffRules = "1.4"
 DiffTests = "< 0.1.2"
+ExplicitImports = "1.15"
 FillArrays = "1"
 ForwardDiff = "1"
 IrrationalConstants = "0.2"
+JET = "0.9.18, 0.12"
 LinearAlgebra = "1"
 LogExpFunctions = "1"
 NaNMath = "1"
 Random = "1"
 SpecialFunctions = "2"
 StaticArrays = "1.6.4"
+StaticArraysCore = "1"
 Statistics = "1"
 Test = "1"
 

test/QATests.jl

@@ -0,0 +1,140 @@
+module QATests
+
+using ReverseDiff, Test
+using Aqua: Aqua
+using ExplicitImports: ExplicitImports
+using JET: JET
+using StaticArrays: StaticArrays
+using StaticArraysCore: StaticArraysCore
+
+# Activates `StatisticsExt`: `runtests.jl` includes this file after `LinAlgTests`
+using Statistics
+
+const STATISTICS_EXT = Base.get_extension(ReverseDiff, :StatisticsExt)
+
+# JET >= 0.11.4 requires Julia >= 1.12, so e.g. the LTS resolves JET 0.9.18 which does not
+# define `JET_AVAILABLE`. A version check instead of `isdefined` errors, rather than skips
+# silently, if JET removes it again.
+const JET_AVAILABLE = pkgversion(JET) >= v"0.12" && JET.JET_AVAILABLE
+
+@test STATISTICS_EXT !== nothing
+
+@testset "Aqua" begin
+    # `ambiguities`: methods generated with `@eval` clash with `Base` and `LinearAlgebra`
+    Aqua.test_all(ReverseDiff; ambiguities=(; broken=true))
+end
+
+@testset "ExplicitImports" begin
+    # Using ReverseDiff's internals inside its own extension is fine, but ExplicitImports cannot
+    # be told so: a package extension is a top-level module, so `Base.moduleroot(STATISTICS_EXT)`
+    # is `StatisticsExt` rather than `ReverseDiff` and `allow_internal_imports`/
+    # `allow_internal_accesses` never apply, while `ignore=(STATISTICS_EXT,)` is rejected because
+    # an extension is not a submodule. Listing the names is the only way to express it -- and it
+    # keeps the extension's dependence on internals visible and reviewable.
+    RD_INTERNALS = (
+        :SpecialInstruction, :TrackedArray, :deriv, :increment_deriv!, :istracked, :record!,
+        :special_forward_exec!, :special_reverse_exec!, :tape, :track, :unseed!, :value, :value!,
+    )
+    # Non-public names in `Base` and in dependencies that predate the `public` keyword.
+    # ReverseDiff itself cannot use `public` either: it requires Julia >= 1.11 and the LTS is 1.10.
+    UPSTREAM_INTERNALS = (
+        # `Base` and `Base.Broadcast`
+        :Broadcasted, :LogicalIndex, :RefValue, :broadcasted, :flatten, :materialize, :rtoldefault,
+        # `Core.Compiler`
+        :Compiler, :return_type,
+        # `DiffResults`
+        :DiffResult, :GradientResult, :ImmutableDiffResult, :derivative, :gradient, :gradient!,
+        :hessian, :jacobian,
+        # `DiffRules`, `ForwardDiff` and `FunctionWrappers`
+        :diffrules, :Dual, :derivative!, :FunctionWrapper,
+    )
+    NONPUBLIC = (RD_INTERNALS..., UPSTREAM_INTERNALS...)
+
+    for m in (ReverseDiff, STATISTICS_EXT)
+        @testset "$(nameof(m))" begin
+            @test ExplicitImports.check_no_implicit_imports(m) === nothing
+            @test ExplicitImports.check_no_stale_explicit_imports(m) === nothing
+            @test ExplicitImports.check_no_self_qualified_accesses(m) === nothing
+            @test ExplicitImports.check_all_qualified_accesses_via_owners(m) === nothing
+            # `SVector` is exported by `StaticArrays` but owned by `StaticArraysCore`
+            @test ExplicitImports.check_all_explicit_imports_via_owners(
+                m; skip=(Base => Core, StaticArrays => StaticArraysCore)
+            ) === nothing
+            @test ExplicitImports.check_all_explicit_imports_are_public(m; ignore=NONPUBLIC) ===
+                nothing
+            @test ExplicitImports.check_all_qualified_accesses_are_public(m; ignore=NONPUBLIC) ===
+                nothing
+        end
+    end
+end
+
+@testset "JET" begin
+    if JET_AVAILABLE
+        target_modules = (ReverseDiff, STATISTICS_EXT)
+
+        f(x) = sum(abs2, x) + prod(x)
+        g(x) = x .^ 2 .+ 1
+        x = rand(4)
+
+        gradient_tape = ReverseDiff.GradientTape(f, x)
+        jacobian_tape = ReverseDiff.JacobianTape(g, x)
+        hessian_tape = ReverseDiff.HessianTape(f, x)
+        compiled_gradient_tape = ReverseDiff.compile(gradient_tape)
+        compiled_jacobian_tape = ReverseDiff.compile(jacobian_tape)
+
+        gradient_result = similar(x)
+        jacobian_result = similar(x, 4, 4)
+        hessian_result = similar(x, 4, 4)
+
+        # `report_package` reports hundreds of problems in code generated with `@eval`
+        JET.@test_call target_modules = target_modules ReverseDiff.GradientConfig(x)
+        JET.@test_call target_modules = target_modules ReverseDiff.JacobianConfig(x)
+        JET.@test_call target_modules = target_modules ReverseDiff.HessianConfig(x)
+        JET.@test_call target_modules = target_modules ReverseDiff.GradientTape(f, x)
+        JET.@test_call target_modules = target_modules ReverseDiff.JacobianTape(g, x)
+        JET.@test_call target_modules = target_modules ReverseDiff.compile(gradient_tape)
+        JET.@test_call target_modules = target_modules ReverseDiff.jacobian(g, x)
+        JET.@test_call target_modules = target_modules ReverseDiff.jacobian(
+            g, x, ReverseDiff.JacobianConfig(x)
+        )
+        JET.@test_call target_modules = target_modules ReverseDiff.jacobian!(
+            jacobian_result, jacobian_tape, x
+        )
+        JET.@test_call target_modules = target_modules ReverseDiff.jacobian!(
+            jacobian_result, compiled_jacobian_tape, x
+        )
+        JET.@test_call target_modules = target_modules ReverseDiff.hessian!(
+            hessian_result, hessian_tape, x
+        )
+
+        # Scalar outputs reach `pull_value!(::TrackedReal{V,D,Nothing})` in `src/tracked.jl`,
+        # where `t.origin` is only guarded by the run-time check `hasorigin(t)`
+        JET.@test_call broken = true target_modules = target_modules ReverseDiff.HessianTape(
+            f, x
+        )
+        JET.@test_call broken = true target_modules = target_modules ReverseDiff.gradient(f, x)
+        JET.@test_call broken = true target_modules = target_modules ReverseDiff.gradient(
+            f, x, ReverseDiff.GradientConfig(x)
+        )
+        JET.@test_call broken = true target_modules = target_modules ReverseDiff.gradient!(
+            gradient_result, gradient_tape, x
+        )
+        JET.@test_call broken = true target_modules = target_modules ReverseDiff.gradient!(
+            gradient_result, compiled_gradient_tape, x
+        )
+        JET.@test_call broken = true target_modules = target_modules ReverseDiff.hessian(f, x)
+
+        # Executing an uncompiled tape dispatches on `AbstractInstruction` at run time
+        JET.@test_opt target_modules = target_modules ReverseDiff.gradient!(
+            gradient_result, compiled_gradient_tape, x
+        )
+        JET.@test_opt broken = true target_modules = target_modules ReverseDiff.gradient!(
+            gradient_result, gradient_tape, x
+        )
+        JET.@test_opt broken = true target_modules = target_modules ReverseDiff.jacobian!(
+            jacobian_result, jacobian_tape, x
+        )
+    end
+end
+
+end # module

test/runtests.jl

@@ -89,4 +89,10 @@ test_println(kind, f, pad="  ") = println(pad, "testing $(kind): `$(f)`...")
         t = @elapsed include(joinpath(TESTDIR, "compat/CompatTests.jl"))
         println("done (took $t seconds).")
     end
+
+    @testset "QATests" begin
+        println("running QATests...")
+        t = @elapsed include(joinpath(TESTDIR, "QATests.jl"))
+        println("done (took $t seconds).")
+    end
 end