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

Small fixes for FitMvNormal #257

Merged
merged 4 commits into from
Apr 20, 2023
Merged

Small fixes for FitMvNormal #257

merged 4 commits into from
Apr 20, 2023

Conversation

Crown421
Copy link
Contributor

Hey,

working with MvNormal, I noticed that the mean, var, and cov function were not defined for FitMvNormal, even though they exist for FitNormal. This MR adds them.

Further, I noticed that in the edge cases that the same (exact) same value is added a few times, the fallback mean and variance were returned. With this MR the behaviour becomes:

julia> m = FitMvNormal(2)
FitMvNormal: n=0 | value=([0.0, 0.0], [-0.0 -0.0; -0.0 -0.0])

julia> a = rand(2)
2-element Vector{Float64}:
 0.5760181131344163
 0.5890715541785612

julia> for i in 1:5
       fit!(m, a)
       end

julia> m
FitMvNormal: n=5 | value=([0.576018, 0.589072], [0.0 0.0; 0.0 0.0])

@joshday
Copy link
Owner

joshday commented Apr 19, 2023

Looks good to me. Thanks for the PR!

@joshday
Copy link
Owner

joshday commented Apr 19, 2023

I'll merge when CI goes green.

@Crown421
Copy link
Contributor Author

It seems CI has passed except for the AppVeyor job, which seems to be broken?

@joshday joshday merged commit 459b2b4 into joshday:master Apr 20, 2023
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 this pull request may close these issues.

2 participants