Make docstrings #39
Make docstrings #39
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #39 +/- ##
=======================================
Coverage 92.71% 92.71%
=======================================
Files 5 5
Lines 247 247
=======================================
Hits 229 229
Misses 18 18
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
@josephsdavid I have completed my first review. Phew! 😥
Please address the todo list above to support #47 .
Ping me when you are ready, and I'll make a final review.
src/MLJMultivariateStatsInterface.jl
Outdated
| Bayesian Multiclass linear discriminant analysis. Suitable for high dimensional data | ||
| (Avoids computing scatter matrices `Sw` ,`Sb`). The algorithm learns a projection | ||
| matrix `P = W*L` (`Sw`), that projects a feature matrix `Xtrain` onto a lower | ||
| dimensional space of dimension `nc-1` such that the trace of the transformed |
There was a problem hiding this comment.
| dimensional space of dimension `nc-1` such that the trace of the transformed | |
| dimensional space of dimension `min(rank(Sw), nc-1)` such that the trace of the transformed |
same here.
There was a problem hiding this comment.
should be resolved
I cannot check them out but this checklist is done! However, I did notice one issue, or have a user error in the following ICA example: using MLJ
ICA = @load ICA pkg=MultivariateStats
time = 8 .\ 0:2001
sine_wave = sin.(2*time)
square_wave = sign.(sin.(3*time))
sawtooth_wave = repeat(collect(0:10) / 4, 182)
signal = [sine_wave, square_wave, sawtooth_wave]
add_noise(x) = x + randn()
signal = map((x -> add_noise.(x)), signal)
signal = permutedims(hcat(signal...))'
mixing_matrix = [ 1 1 1; 0.5 2 1; 1.5 1 2]
X = MLJ.table(signal * mixing_matrix)
model = ICA(k = 3, tol=0.1)
mach = machine(model, X) |> fit! # this errors ERROR: MethodError: no method matching size(::MultivariateStats.ICA{Float64}, ::Int64)
Xproj = transform(mach, X)
@info sum(abs, Xproj - signal)output of [add582a8] MLJ v0.18.4
[a7f614a8] MLJBase v0.20.16
[1b6a4a23] MLJMultivariateStatsInterface v0.3.2It also appears that ICA is still using k, if i use outdim, i get a methoderror |
|
Well, we probably just need to rebase on the new dev, which has had breaking changes (and, I suspect, a bug fix addressing your issue). I will investigate. Thanks for this progress. |
|
Thanks @josephsdavid for all your work on this mammoth PR. 🚀 |
|
Woohoo! It's done! |
In service of #38 and #913