Add DiffResult for MArray, take 2

[gdalle]

Reverts #32

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.84%. Comparing base (91614bb) to head (e260e02).
⚠️ Report is 8 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master      #35   +/-   ##
=======================================
  Coverage   86.84%   86.84%           
=======================================
  Files           1        1           
  Lines          76       76           
=======================================
  Hits           66       66           
  Misses         10       10           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[gdalle]

How would you spot if there is one SArray among the Vararg?

[devmotion]

How would you spot if there is one SArray among the Vararg?

I'd define something like

# It would be safer to use `false` as the default fallback...
ismutablearray(::AbstractArray) = true
ismutablearray(::SArray) = false
ismutablearray(::Diagonal{T,SMatrix{T}}) = false
...

DiffResult(value::Number, derivs::Tuple{Vararg{Number}}) = ImmutableDiffResult(value, derivs)
function DiffResult(value::Union{Number,AbstractArray}, derivs::Tuple{Vararg{AbstractArray}})
    if all(ismutablearray, derivs)
        return MutableDiffResult(value, derivs)
    else
        return ImmutableDiffResult(value, derivs)
    end
end

[gdalle]

Is it type-inferrable to do all(f, tuple) in the constructor?

[devmotion]

I'm sure up to a certain extent 😄 But I haven't tested it. IIRC sometimes one can help the compiler by defining such checks recursively. And if the checks only depend on types (and instances are not needed), one could use Base.tuple_type_tail and Base.tuple_type_head.

But how many orders of derivatives do people realistically work with? So I think in most cases all should be fine.

[tpapp]

FWIW, I am not sure that mixing SArrays and mutable array types in a DiffResult is a use case we really need to support. For practical purposes, it is as likely to be a bug on the caller's side as anything else.