improve type stability with StaticArrays

mapclyps commented 1 year ago

Replace an anonymous function with Base.Fix1 for type stability in some cases, see #639

codecov[bot] commented 1 year ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 89.59%. Comparing base (c310fb5) to head (4bc44b6). Report is 6 commits behind head on master.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #640 +/- ## ========================================== + Coverage 89.57% 89.59% +0.02% ========================================== Files 11 11 Lines 969 971 +2 ========================================== + Hits 868 870 +2 Misses 101 101 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

mapclyps commented 1 year ago

I tried to replicate the type instability in the tests and put a test at the end of JacobianTest.jl like

ff(x) = sin.(x)
ssx = SA[1.0, 2.0]
ssy = ff(ssx)
result_store = DiffResults.JacobianResult(ssx)
@test begin
    result = (@inferred ForwardDiff.jacobian!(result_store, ff, ssx)) #the actual test
    jac = ForwardDiff.jacobian(ff, ssx)
    expected = DiffResults.ImmutableDiffResult(ssy, (jac,)) 
    result == expected
end

But to my surprise, there was no type instability with this test also on master.

However, just spinning up a REPL and executing this throws the error as expected and reported in #639. Then I just moved this test to the top of the JacobianTest.jl file, and then it also threw the error on master (so it was working correctly as a test for the fix for #639). But it made me wonder what is actually going on there... and where this "hidden state" comes from.

Or is there a better, more stable way to test it?

JuliaDiff / ForwardDiff.jl

improve type stability with StaticArrays #640

Codecov Report