-
-
Couldn't load subscription status.
- Fork 2
Integration with MultivariateStats.jl, enabling PCA/whitening across many SDMLayers
#132
base: main
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## main #132 +/- ##
==========================================
+ Coverage 54.13% 55.08% +0.95%
==========================================
Files 35 36 +1
Lines 1101 1120 +19
==========================================
+ Hits 596 617 +21
+ Misses 505 503 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
Can I be a nuisance? I'd like it a lot if we could do something like W = fit(PCA, stack_of_layers)
smaller_stack_of_layers = transform(W, stack_of_layers)In some cases, we don't want to reduce the dimensionality as much as we would like to e.g. whiten the data. I'd drop that in a file loaded when |
|
This makes me realize that we may want to have a new type, like |
|
overloading |
|
@tpoisot mostly there, current code works for also given the function call structure it requires computing the input matrix in both do you want to merge this into a different branch to implement |
|
I don't think this needs to have stacks yet - but we can merge, and add the stacks later? |
|
just pushed docs and a type assert for |
|
So I don't think a type assert is required - Whitening fails sometimes because it requires a specific matrix type. I'll review and merge/tag, there's no dev branch. |
|
removed the type assert |
|
Can you add a documentation update? |
|
there's a |
|
looks like i dropped that when shifting to the |
this probably "just" need a little explanation that sometimes |
|
fixed the whitening example (i think), needed a slightly different overload of |
|
actuatlly i might be overcomplicating it a bit |
|
The implementation of
|
|
now implemented via a single function call, the only irritant being in the case that if |
MultivariateStats.jl, enabling PCA/whitening across many SDMLayers
Wait, there's a |
|
not at the moment, but implementing a edit: no need to export if it is dependent on MVStats being loaded, it just requires essentially this is a problem with |
|
In the long term I think the julianesque solution is for MVStats.jl to define all its ( |
|
So can't we temporarily solve this with generated code? |
|
macros are probably the feature of julia i understand the least, but if that's a neat way to resolve the |
|
Let me try |
|
OK so I don't think it needed the code generation - I'll check the example page before releasing |
|
yeah it runs into the same dispatch ambiguity without defining a |
|
there's also an inconsistency in how |
|
seems there is an MVStats.jl issue about exactly this issue, not sure how active MVStats.jl development is |
|
Mhhhh. Running test doesn't show a method error on my side, are you sure the test suite is complete? |
|
i think it doesn't fail because i use |
|
running locally gives me the same dispatch ambiguity |
|
and i should 100% add a test of |
|
Visited this again, still can't find a simple way around this ambiguity The possible fix from the error message is implemented but doesn't actually fix the problem |
0a99946 to
f2b8627
Compare
What the pull request does
Implements methods for taking a set of SDMLayers and applying multivariate transformations from
JuliaStats/MultivariateStats.jlto do a variety of things: reduce the dimensionality of a data set (PCA, PPCA, KernelPCA), make layers without correlation (Whitening), or the other methods mvstats providesType of change
Please indicate the relevant option(s)
Checklist
.zenodo.jsonProject.tomlfieldversionhas been updatedv0.0.xseries release, a bug fix, or a performance improvement