Add DiffResult for MArray, take 2

[gdalle]

Reverts JuliaDiff/DiffResults.jl#32

[codecov[bot]]

Codecov Report

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

Project coverage is 86.84%. Comparing base (91614bb) to head (e260e02).

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           

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

[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.