Document `PCA` #3677

rcurtin · 2024-04-03T17:59:23Z

This PR adds documentation for PCA (principal components analysis). This was a pretty simple method to document, but I had to make some minor changes:

Add non-inplace overloads for dimensionality reduction and variance keeping overloads. This is necessary so you can, e.g., pass in a sparse dataset and get back a dense one.
Templatize PCA::Apply() so it can work on different matrix types (e.g. arma::fmat). That in turn meant templatizing all of the decomposition strategies... as those get individually documented they will be tested further. As part of this, I refactored MakeAlias(), which existed in two places, so that it now only exists in one place. I also updated the documentation in core.md accordingly.
Fix a minor bug in the block Krylov randomized SVD; @zoq can you make sure my changes here are correct? I believe according to the paper that k must be less than or equal to d (see Algorithm 2's Input: section).
Limit the width of the sidebar so that it doesn't overlap with the content.

You can click around the documentation here:

shrit

Everything looks good, I left a couple of comments but even the docs webpage looks great.

shrit · 2024-04-05T14:41:19Z

src/mlpack/methods/lmnn/lmnn_function_impl.hpp

+LMNNFunction<MetricType>::LMNNFunction(const arma::mat& datasetIn,
+                                       const arma::Row<size_t>& labelsIn,


Any reason why not move this to MatType ?

I'll come back later for it, the goal here was just to modify MakeAlias().

okay, good for me

shrit · 2024-04-05T14:43:10Z

src/mlpack/methods/nca/nca_softmax_error_function_impl.hpp

+    const arma::mat& datasetIn,
+    const arma::Row<size_t>& labelsIn,
    MetricType metric) :


The same comment here ?

Same, it will happen, just not in this PR (likely when I document NCA or LMNN).

it is fine then

shrit · 2024-04-05T14:43:31Z

src/mlpack/methods/logistic_regression/logistic_regression_function_impl.hpp

-    const MatType& predictors,
-    const arma::Row<size_t>& responses,
+    const MatType& predictorsIn,
+    const arma::Row<size_t>& responsesIn,


here as well?

This one is already refactored, LogisticRegression already takes whatever MatType as input. The labels have to be arma::Row<size_t> though.

shrit · 2024-04-05T14:49:20Z

src/mlpack/methods/ann/make_alias.hpp

-/**
- * @file make_alias.hpp
- * @author Ryan Curtin
- *
- * Implementation of `MakeAlias()`, a utility function.  This is meant to be
- * used in `SetWeights()` calls in various layers, to wrap internal weight
- * objects as aliases around the given memory pointers.
- *
- * mlpack is free software; you may redistribute it and/or modify it under the
- * terms of the 3-clause BSD license.  You should have received a copy of the
- * 3-clause BSD license along with mlpack.  If not, see
- * http://www.opensource.org/licenses/BSD-3-Clause for more information.
- */
-#ifndef MLPACK_METHODS_ANN_MAKE_ALIAS_HPP
-#define MLPACK_METHODS_ANN_MAKE_ALIAS_HPP
-


where this class has been moved? are you using directly the one that is in the core/

I am using it in the coot refactoring 😅

Yes, the signature is exactly the same but it is moved to core/ now. I cleaned up a few other overloads. I think everything is the same for your usage though: https://www.ratml.org/misc/mlpack-markdown-doc/user/core.html#aliases

shrit · 2024-04-05T14:51:57Z

src/mlpack/methods/pca/pca_impl.hpp

+  BaseMatType eigvec;
+  BaseColType eigVal;


what is the difference between BaseColType and ColType ?

This was to catch expressions that are passed in. So if I call, e.g., pca.Apply(2 * x, ...), MatType will be inferred as an Armadillo expression (some complex type). So BaseMatType and BaseColType are the "base" types that we can use to store results in. The ensmallen code does similar things at the beginning of every Optimize() function: https://github.com/mlpack/ensmallen/blob/master/include/ensmallen_bits/de/de_impl.hpp#L40

shrit · 2024-04-05T14:52:32Z

HISTORY.md

+  * Allow PCA to take different matrix types (#3677).
+


I should remember do the same for all the PR we merged

shrit · 2024-04-05T15:16:24Z

src/mlpack/core/math/make_alias.hpp

+ * Reconstruct `m` as an alias around the memory `newMem`, with size `numRows` x
+ * `numCols`.
 */
-template<typename ElemType>
-arma::Cube<ElemType> MakeAlias(arma::Cube<ElemType>& input,
-                               const bool strict = true)
+template<typename MatType>
+void MakeAlias(MatType& m,
+               typename MatType::elem_type* newMem,
+               const size_t numRows,
+               const size_t numCols,
+               const bool strict = true,
+               const typename std::enable_if_t<!IsCube<MatType>::value>* = 0)
 {


okay I see it here.

zoq · 2024-04-06T15:35:01Z

doc/user/methods/pca.md

+
+#### See also:
+
+<!-- TODO: add link -->


I guess something for later?

Right, I can't add the link to RADICAL until the documentation page for that method is done.

zoq · 2024-04-06T15:36:33Z

src/mlpack/methods/block_krylov_svd/randomized_block_krylov_svd_impl.hpp

+    blockSize = std::min((size_t) data.n_rows,
+                         std::min((size_t) data.n_cols, rank + 10));


Looks like the indentation is off?

Fix looks good to me.

Fixed in 787e14b. 👍

zoq

No more comments from my side.

rcurtin added 9 commits April 2, 2024 14:51

Clean up MakeAlias() and put it in only one place.

e9be8af

Templatize PCA and its dependencies to allow different matrix types.

41ad950

Add documentation for PCA.

b3fd17e

Revisions to PCA documentation.

294d80d

Limit width of sidebar.

2c390b0

Add additional overloads for PCA::Apply() and tests.

e4f33e2

Fix links in PCA documentation.

276dc85

Don't fail on 301s (bans for crawling).

e8792de

Update Weka links.

fb6baba

mlpack-bot bot added s: needs review s: unanswered s: unlabeled labels Apr 3, 2024

Update HISTORY.md.

58d5e02

conradsnicta added c: documentation and removed s: unanswered s: unlabeled labels Apr 5, 2024

conradsnicta approved these changes Apr 5, 2024

View reviewed changes

shrit approved these changes Apr 5, 2024

View reviewed changes

mlpack-bot bot removed the s: needs review label Apr 5, 2024

zoq reviewed Apr 6, 2024

View reviewed changes

Fix indentation.

787e14b

zoq approved these changes Apr 8, 2024

View reviewed changes

rcurtin merged commit 6586ed6 into mlpack:master Apr 9, 2024

rcurtin deleted the pca-doc branch April 9, 2024 12:52

This was referenced May 26, 2024

Release version 4.4.0 #3716

Closed

Release version 4.4.0 #3717

Merged

		LMNNFunction<MetricType>::LMNNFunction(const arma::mat& datasetIn,
		const arma::Row<size_t>& labelsIn,

		blockSize = std::min((size_t) data.n_rows,
		std::min((size_t) data.n_cols, rank + 10));

Uh oh!

Document PCA #3677

Document PCA #3677

Uh oh!

Conversation

rcurtin commented Apr 3, 2024

Uh oh!

shrit left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zoq left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Document `PCA` #3677

Document `PCA` #3677