Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unit test failure for inflation #170

Closed
odunbar opened this issue Jun 14, 2022 · 3 comments · Fixed by #173
Closed

Unit test failure for inflation #170

odunbar opened this issue Jun 14, 2022 · 3 comments · Fixed by #173
Assignees

Comments

@odunbar
Copy link
Collaborator

odunbar commented Jun 14, 2022

On a Mac, and Julia 1.6.6

line 522 of test/EnsembleKalmanProcesses/runtests.jl.

Causes Domain-error with eigmax, (as well as the desired warning)

co-authored with @anagha548

@eviatarbach
Copy link
Contributor

Which test is failing specifically? Line 522 in test/EnsembleKalmanProcesses/runtests.jl in the latest version is not related to localization, nor eigendecomposition.

@odunbar
Copy link
Collaborator Author

odunbar commented Jun 17, 2022

Oh my apologies @eviatarbach, it's for inflation not localization, perhaps @ilopezgp knows. The test is indeed on those lines

@test_logs (:warn,) sample_empirical_gaussian(u, 2)

This calls

function sample_empirical_gaussian(
u::AbstractMatrix{FT},
n::IT;
inflation::Union{FT, Nothing} = nothing,
) where {FT <: Real, IT <: Int}
cov_u_new = cov(u, u, dims = 2)
if !isposdef(cov_u_new)
@warn string("Sample covariance matrix over ensemble is singular.", "\n Appplying variance inflation.")
if isnothing(inflation)
# Reduce condition number to 1/sqrt(eps(FT))
inflation = eigmax(cov_u_new) * sqrt(eps(FT))
end
cov_u_new = cov_u_new + inflation * I
end
mean_u_new = mean(u, dims = 2)
return rand(MvNormal(mean_u_new[:], cov_u_new), n)
end

The test is trying to get the warning for line 301, (which it does return) but then it also gives a DomainError in eigmax in line 304

@odunbar odunbar changed the title Unit test failure for localization Unit test failure for inflation Jun 17, 2022
@ilopezgp ilopezgp self-assigned this Jun 21, 2022
@ilopezgp
Copy link
Contributor

Based on the docs, I think the DomainError came from the floating point errors that made some of the eigenvalues complex. I will just cast it to Symmetric, since it is a covariance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants