Repository: JuliaDiff/FiniteDifferences.jl (julia)

Issue:
Mutation of input arguments
Code like this https://github.com/JuliaDiff/FiniteDifferences.jl/blob/c7a4624092d90b6d49d5701967982226231d4478/src/grad.jl#L18

mutates the input argument, but before returning makes sure the input argument is restored to its original value. However, if the user-provided function errors, the input argument is left in a mutated state, which can cause bugs that are very hard to detect.

Pull request description:
Fixes #221 
needs tests still

Patch:
diff --git a/src/grad.jl b/src/grad.jl
index 316d6ab..1aac95d 100644
--- a/src/grad.jl
+++ b/src/grad.jl
@@ -15,10 +15,12 @@ function jacobian(fdm, f, x::Vector{<:Real}; len=nothing)
     ẏs = map(eachindex(x)) do n
         return fdm(zero(eltype(x))) do ε
             xn = x[n]
-            x[n] = xn + ε
-            ret = copy(first(to_vec(f(x))))  # copy required incase `f(x)` returns something that aliases `x`
-            x[n] = xn  # Can't do `x[n] -= ϵ` as floating-point math is not associative
-            return ret
+            try
+                x[n] = xn + ε
+                return copy(first(to_vec(f(x))))  # copy required incase `f(x)` returns something that aliases `x`
+            finally
+                x[n] = xn  # Can't do `x[n] -= ϵ` as floating-point math is not associative
+            end
         end
     end
     return (reduce(hcat, ẏs), )


Test patch:
diff --git a/test/grad.jl b/test/grad.jl
index 9ebf197..cf36893 100644
--- a/test/grad.jl
+++ b/test/grad.jl
@@ -84,6 +84,19 @@ using FiniteDifferences: grad, jacobian, _jvp, jvp, j′vp, _j′vp, to_vec
         @test J ≈ one(Matrix{T}(undef, size(J)))
     end
 
+    @testset "jacobian that throws errors" begin
+        # https://github.com/JuliaDiff/FiniteDifferences.jl/issues/221
+        fdm = central_fdm(5, 1)
+        x = zeros(5)
+        try
+            jacobian(fdm, error, x)
+        catch err
+            @assert err isa ErrorException
+        end
+        # Make sure state of `x` is restored.
+        @test x == zeros(5)
+    end
+
     @testset "multi vars jacobian/grad" begin
         rng, fdm = MersenneTwister(123456), central_fdm(5, 1)