Covariance matrix obs_noise_cov is restricted to Array{FT, 2}

[glwagner]

The type of EnsembleKalmanProcess.obs_noise_cov is restricted to Array{FT, 2}, which is inconvenient and prevents the use of common LinearAlgebra abstractions that represent arrays.

For example:

julia> using EnsembleKalmanProcesses, EnsembleKalmanProcesses.ParameterDistributions, LinearAlgebra, Distributions

julia> distribution = ParameterDistribution(Parameterized(Normal(0, 1)), no_constraint(), "");

julia> sample = sample_distribution(distribution, 1);

julia> obs_noise_cov = 0.1*I
UniformScaling{Float64}
0.1*I

julia> ekp = EnsembleKalmanProcess(sample, zeros(2), obs_noise_cov, Inversion())
ERROR: MethodError: no method matching EnsembleKalmanProcess(::Matrix{Float64}, ::Vector{Float64}, ::UniformScaling{Float64}, ::Inversion)
...

0.1 * I is a valid and efficient way to specify a covariance matrix. The code itself likely works as is, since key lines like

julia> noise = rand(MvNormal(zeros(2), 0.1 * I), 2)
2×2 Matrix{Float64}:
 0.0551659  0.720873
 0.057236   0.289555

are valid for a wide variety of matrix types. Thus probably the only change needed is to release obs_noise_cov from Array{FT, 2}.

For large observation vectors the need to convert objects like UniformScaling to Array{FT, 2} is a waste of memory:

julia> Matrix(0.1*I, 5, 5)
5×5 Matrix{Float64}:
 0.1  0.0  0.0  0.0  0.0
 0.0  0.1  0.0  0.0  0.0
 0.0  0.0  0.1  0.0  0.0
 0.0  0.0  0.0  0.1  0.0
 0.0  0.0  0.0  0.0  0.1

Another important matrix type is Diagonal representing a diagonal matrix.

cc @simonbyrne

[odunbar]

For sure, the creation of the package was done with types being used to restrict arguments (before i even knew about the difficulties with matrix != array{,2} != DiagonalMatrix type problems)

I think for Covariance matrices, I am happy to take the broadest umbrella type that can have a dimension specified (i.e. must be 2D)? Or would you be looking for all typing completely removed as we don't dispatch?

[glwagner]

You want to use a variable concrete type:

obs_noise_cov::M

The type union Union{UniformScaling, AbstractMatrix} might cover enough cases:

julia> using LinearAlgebra

julia> a = rand(2, 2)
2×2 Matrix{Float64}:
 0.479173  0.77404
 0.842289  0.230474

julia> b = Diagonal(a)
2×2 Diagonal{Float64, Vector{Float64}}:
 0.479173   ⋅
  ⋅        0.230474

julia> c = 0.1 * I
UniformScaling{Float64}
0.1*I

julia> a isa AbstractMatrix
true

julia> b isa AbstractMatrix
true

julia> c isa AbstractMatrix
false

I would be cautious about using a type constraint for a user-facing object like this. Type constraints are more appropriate for low-level code where the possible range of types are actually known. It's difficult to have omniscient knowledge about anything users might do. @simonbyrne probably has enough knowledge to say whether Union{UniformScaling, AbstractMatrix} is appropriate, since his Julia knowledge approaches omniscience.

As an aside, I think it's more user-friendly for user-facing stuff to intentionally validate input rather than relying on the often-obscure MethodError:

obs_noise_cov isa Union{UniformScaling, AbstractMatrix} ||
    throw(ArgumentError(string(summary(obs_noise_cov), " obs_noise_cov is not supported. Use UniformScaling or AbstractMatrix"))

or something like that.