Feature/mda #206
Feature/mda #206
Conversation
|
Thanks @james-oldfield! On a high-level, would it make sense to also have an additional solver based on SVD (e.g. like for the linear case in scikit-learn)? This would avoid building the covariance matrix (it seems we make the assumption that all classes have the same covariance anyway here?). However, it's not clear whether it would be computationally advantageous in the end. Looking at the update rule, an online version might be nice to add down the line. As a side note, it's good to make the docstring as easily readible as possible (e.g. not too much latex) -- we can have a short explanation in the docstring itself and have a longer, more math heavy version in the user guide. |
|
Definitely! Although off the top of my head I'm not quite sure how one would extend the SVD solver for the multilinear case (i.e. how we'd solve for the factor matrices with the SVD directly on the batch of mode-n unfolding (a 3rd-order tensor), without computing the scatter matrices explicitly?) It certainly would make a nice optional feature though if it were possible. One thing that would be nice in the future could be to provide a flag to instead take the solution via the eigendecomposition of Good idea with the docstring, I've removed a lot of the bulk :) |
| ############### | ||
| # check correct # of ranks have been supplied | ||
| ############### | ||
| assert len(ranks) == len(T.shape(X)[1:]), 'Expected number of ranks: {}. \ |
There was a problem hiding this comment.
It's good for end users to make the error messages as informative as possible. Something like:
| assert len(ranks) == len(T.shape(X)[1:]), 'Expected number of ranks: {}. \ | |
| if len(ranks) != (T.ndim(X) - 1): | |
| msg = f'Got as input a tensor of shape {T.shape(X)} corresponding to {T.shape(X)[0]} samples of shape {T.shape(X)[1:]} of order {T.ndim(X) -1}.' | |
| msg += f'But got {len(ranks)} ranks != {T.ndim(X) -1}.' | |
| raise ValueError(msg) |
| # store the mean of all tensor samples with label i | ||
| for i in range(len(set(y))): | ||
| # tensorflow is only backend to not support indexing into tensor with a list | ||
| if backend == 'tensorflow': |
There was a problem hiding this comment.
I really don't like/want to have these in the code (it should be backend agnostic). However tensorflow really does not make it easy.. Would be ideal to find another way if it doesn't slow down the other backends.
| if backend == 'tensorflow': | ||
| class_means += [T.mean(T.tensor([X[j] for j in class_idx[i]]), axis=0)] | ||
| else: | ||
| class_means += [T.mean(X[class_idx[i], ...], axis=0)] |
There was a problem hiding this comment.
We can directly get the means without creating class_idx:
| class_means += [T.mean(X[class_idx[i], ...], axis=0)] | |
| class_means += [T.mean(X[y == i, ...], axis=0)] |
Of course tensorflow doesn't support this directly but through a function:
tf.boolean_mask(X, y==i)equivalently, for TensorFlow, we can do:
tl.stack([X[j, ...] for j, e in enumerate(tl.to_numpy(y)) if e == i], 0)| # -- | ||
| # [1] Q. Li et al. "Multilinear Discriminant Analysis for Higher-Order Tensor Data Classification" | ||
| ################################################### | ||
| U, _, _ = T.partial_svd(T.dot(W_scat_inv, B_scat)) |
There was a problem hiding this comment.
We should probably be using SVD_FUNS but this is being refactored in #217 and should be easier to use when that gets merged.
| return factors | ||
|
|
||
|
|
||
| def compute_modek_wb_scatters(X, mode, factors, global_mean, class_means, class_idx): |
There was a problem hiding this comment.
What about naming it mode_scatter_matrices? That would be consistent with mode_dot.
Or even just scatter_matrices and we document well that this corresponds to mode-n between and within class scatter matrices?
| # recover X, using the inverse projection matrices | ||
| X_hat = multi_mode_dot(Z, [tl.tensor(inv(tl.to_numpy(f))) for f in factors], modes=[1, 2, 3], transpose=True) | ||
|
|
||
| assert_array_almost_equal(X, X_hat, decimal=tol) |
There was a problem hiding this comment.
Isn't this always true as long as the factors are not singular? Also the factors are obtain through SVD so orthogonal, we can just take their transpose instead of converting to NumPy and using inv. It would be good to have some specific tests.
JeanKossaifi
force-pushed
the
master
branch
2 times, most recently
from
d45beab
to
2aa9834
Compare
Also opening a PR for Li's constrained multilinear discriminant analysis [1]. This could also perhaps suit
tensorly/decompositionbest, given the current structure? Along with MPCA, there's also a notebook walkthrough that could potentially fit into the examples!