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

Update calcCovarianceBasic #150

Closed
Affie opened this issue Aug 18, 2021 · 4 comments
Closed

Update calcCovarianceBasic #150

Affie opened this issue Aug 18, 2021 · 4 comments
Assignees
Labels
API enhancement New feature or request manifolds
Milestone

Comments

@Affie
Copy link
Member

Affie commented Aug 18, 2021

# Returns the covariance (square), not deviation
function calcCovarianceBasic(M::AbstractManifold, ptsArr::Vector{P}) where P
#TODO double check the maths,. it looks like its working at least for groups
μ = mean(M, ptsArr)
Xcs = vee.(Ref(M), Ref(μ), log.(Ref(M), Ref(μ), ptsArr))
Σ = mean(Xcs .* transpose.(Xcs))
@debug "calcCovarianceBasic" μ
@debug "calcCovarianceBasic" Σ
# TODO don't know what to do here so keeping as before, #FIXME it will break
# a change between this and previous is that a full covariance matrix is returned
msst = Σ
msst_ = 0 < sum(1e-10 .< msst) ? maximum(msst) : 1.0
return msst_
end

Manifolds.jl now has a cov function that this can be updated to use: cov(M, ptsArr; basis, kwargs... )

We should also consider changing the name to reflect the fact that it is a variance and not a covariance. We can then also use the manifolds var function.

@Affie Affie added enhancement New feature or request API labels Aug 18, 2021
@Affie Affie added this to the v0.x.0 milestone Aug 18, 2021
@dehann
Copy link
Member

dehann commented Jul 14, 2022

I just ran into this again, going to add this to the v0.6.1 planning.

@dehann dehann modified the milestones: v0.x.0, v0.6.1 Jul 14, 2022
@dehann
Copy link
Member

dehann commented Dec 7, 2022

quick work to close this:

@dehann dehann removed their assignment Dec 7, 2022
@dehann
Copy link
Member

dehann commented Dec 7, 2022

Ongoing tech debt is IIF.nullhypo code should spread on each dimension specifically, not just reduced to one number as these legacy function. We can open this as a separate ongoing issue, and close this one faster.

Also see:

  • calcStandardDev

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API enhancement New feature or request manifolds
Projects
Status: Done
Development

No branches or pull requests

2 participants