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

@@ -5,6 +5,8 @@
 [![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)
 [![code style: runic](https://img.shields.io/badge/code_style-%E1%9A%B1%E1%9A%A2%E1%9A%BE%E1%9B%81%E1%9A%B2-black)](https://github.com/fredrikekre/Runic.jl)
+[![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)
 
 ReverseDiff is a fast and compile-able tape-based **reverse mode automatic differentiation (AD)** that 
 implements methods to take **gradients**, **Jacobians**, **Hessians**, and

src/ReverseDiff.jl

@@ -1,24 +1,19 @@
 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, Diagonal, LowerTriangular, Transpose,
+    UnitLowerTriangular, UnitUpperTriangular, UpperTriangular, 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, SArray, SVector, StaticArray, StaticMatrix, similar_type
 
 # 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: AbstractArrayStyle, BroadcastStyle, Broadcasted, DefaultArrayStyle
+using Base.Broadcast: AbstractArrayStyle, Broadcasted, DefaultArrayStyle
 using ForwardDiff: ForwardDiff, Dual
 
 """
@@ -119,7 +119,8 @@ function holdsperturbations(::Type{D}, v::Union{Real, AbstractArray, Tuple}) whe
     if isconcretetype(E)
         return ForwardDiff.tagtype(E) === Nothing || promote_type(D, E) <: D
     else
-        return all(x -> holdsperturbations(D, x), v)
+        # `all` does not infer as `Bool` for an abstract `Tuple` or array
+        return all(x -> holdsperturbations(D, x), v)::Bool
     end
 end
 

src/derivatives/linalg/arithmetic.jl

@@ -292,4 +292,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

@@ -332,16 +332,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,
@@ -350,25 +350,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,11 +1,14 @@
 [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"
@@ -14,24 +17,29 @@ 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.3"
+ExplicitImports = "1.15"
 FillArrays = "1"
 ForwardDiff = "1"
 IrrationalConstants = "0.2"
+JET = "0.9.18, 0.12"
 LinearAlgebra = "1"
 LogExpFunctions = "1"
 NaNMath = "1"
 ParallelTestRunner = "2.0.1"
 Random = "1"
 SpecialFunctions = "2"
 StaticArrays = "1.6.4"
+StaticArraysCore = "1"
 Statistics = "1"
 Test = "1"
 

test/QATests.jl

@@ -0,0 +1,136 @@
+using ReverseDiff, Test
+using Aqua: Aqua
+using ExplicitImports: ExplicitImports
+using JET: JET
+using StaticArrays: StaticArrays
+using StaticArraysCore: StaticArraysCore
+
+# Activates `StatisticsExt`
+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
+    # A package extension is a top-level module, so ExplicitImports does not treat its use of
+    # ReverseDiff's internals as internal, and the names have to be listed
+    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
+    UPSTREAM_INTERNALS = (
+        # `Base` and `Base.Broadcast`
+        :AbstractArrayStyle, :Broadcasted, :DefaultArrayStyle, :LogicalIndex, :RefValue,
+        :combine_eltypes, :flatten, :instantiate, :print_array, :promote_op, :result_style,
+        :rtoldefault,
+        # `DiffResults`
+        :DiffResult, :GradientResult, :ImmutableDiffResult, :derivative, :gradient, :gradient!,
+        :hessian, :jacobian,
+        # `DiffRules`, `ForwardDiff` and `FunctionWrappers`
+        :diffrules, :Dual, :DualMismatchError, :Tag, :derivative!, :partials, :tagtype, :valtype,
+        :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
+            # Julia 1.10 has no `public` keyword, so it reports names that later versions mark public
+            if VERSION >= v"1.11"
+                @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
+end
+
+f(x) = sum(abs2, x) + prod(x)
+g(x) = x .^ 2 .+ 1
+
+@testset "JET" begin
+    if JET_AVAILABLE
+        target_modules = (ReverseDiff, STATISTICS_EXT)
+
+        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