diff --git a/.github/workflows/Downgrade.yml b/.github/workflows/Downgrade.yml index 9ccbd19..f7213c7 100644 --- a/.github/workflows/Downgrade.yml +++ b/.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: diff --git a/README.md b/README.md index 36a105c..ca19aff 100644 --- a/README.md +++ b/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) diff --git a/src/ReverseDiff.jl b/src/ReverseDiff.jl index 011a026..25a6e5d 100644 --- a/src/ReverseDiff.jl +++ b/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. diff --git a/src/derivatives/broadcast.jl b/src/derivatives/broadcast.jl index abecc67..b89152e 100644 --- a/src/derivatives/broadcast.jl +++ b/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} diff --git a/src/derivatives/linalg/arithmetic.jl b/src/derivatives/linalg/arithmetic.jl index 56e78c3..47a555c 100644 --- a/src/derivatives/linalg/arithmetic.jl +++ b/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)) diff --git a/src/derivatives/linalg/special.jl b/src/derivatives/linalg/special.jl index 8de1080..afdee4c 100644 --- a/src/derivatives/linalg/special.jl +++ b/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) diff --git a/src/macros.jl b/src/macros.jl index 458d873..aa3016d 100644 --- a/src/macros.jl +++ b/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 diff --git a/test/Project.toml b/test/Project.toml index 33298a9..9c140e1 100644 --- a/test/Project.toml +++ b/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" @@ -13,23 +16,28 @@ 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" diff --git a/test/QATests.jl b/test/QATests.jl new file mode 100644 index 0000000..5419f9b --- /dev/null +++ b/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 diff --git a/test/runtests.jl b/test/runtests.jl index b53f0e8..9b75107 100644 --- a/test/runtests.jl +++ b/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