-
-
Notifications
You must be signed in to change notification settings - 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.jl
to 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.json
Project.toml
fieldversion
has been updatedv0.0.x
series release, a bug fix, or a performance improvement