Skip to content

feat: lightweight PCA backend (scipy/numpy SVD)#1431

Open
adRn-s wants to merge 4 commits into
4.0.0from
pca_back
Open

feat: lightweight PCA backend (scipy/numpy SVD)#1431
adRn-s wants to merge 4 commits into
4.0.0from
pca_back

Conversation

@adRn-s

@adRn-s adRn-s commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

Replace the sklearn PCA and StandardScaler in Correlation.plot_pca with a scipy/numpy SVD implementation, reproducing sklearn's output (column standardization with population std, deterministic svd_flip sign convention, explained_variance_ from singular values). Drop the now-unused pandas and scikit-learn dependencies.

Replace the sklearn PCA and StandardScaler in Correlation.plot_pca with a
scipy/numpy SVD implementation, reproducing sklearn's output (column
standardization with population std, deterministic svd_flip sign convention,
explained_variance_ from singular values). Drop the now-unused pandas and
scikit-learn dependencies.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@adRn-s adRn-s requested a review from WardDeb July 8, 2026 15:50
…CA tests

The scipy/SVD PCA rewrite inherited three pre-existing plot_pca bugs from
the sklearn version:

- --transpose crashed with a shape mismatch (np.dot(m, Wt.T)); U*S already
  gives sample projections, so orient as (components, samples) via Wt.T.
- --log2 / --rowCenter were no-ops: they mutated self.matrix after m had
  been copied during ntop filtering. Now applied to a float copy before
  variance filtering.
- --ntop below the sample count crashed the scatter with IndexError; guard
  with a clear sys.exit instead.

Adds test_plotPCA.py coverage (sign-invariant coordinate/eigenvalue
regressions, ntop behavior, transpose, CLI validation exits) that passes
against both the sklearn and scipy implementations.
@adRn-s

adRn-s commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator Author

Added PCA test coverage + fixed three inherited plot_pca bugs (a04e88c)

While building a regression test suite for plotPCA (to guard the sklearn → scipy/SVD switch), we discovered that the SVD rewrite produces the same default output as sklearn — good — but that three pre-existing bugs in the surrounding plot_pca logic were carried over unchanged. This commit adds the tests and fixes all three.

Discoveries

  • The scipy/SVD PCA is numerically equivalent to sklearn for the default path (matches to rtol=1e-5), differing only by per-principal-component sign — expected gauge freedom. Confirmed by running the same suite against both implementations.
  • --transpose was fully broken (in both sklearn and scipy versions): Wt = np.dot(m, Wt.T) raises a shape mismatch whenever features ≠ samples, i.e. essentially always. This is the option meant to "match what R does," and it could never run.
  • --log2 and --rowCenter were silent no-ops. They mutated self.matrix after m had already been copied during --ntop filtering, so the transforms never reached the PCA. Output was byte-identical to default.
  • --ntop below the sample count crashed the scatter with an IndexError (fewer components than samples to lay out).

Fixes

  1. Transpose — dropped the broken np.dot; U * S already gives each sample's projection onto the PCs, so we just orient it as (components, samples).
  2. log2 / rowCenter — now applied to a float copy of the matrix, before variance filtering, so both flags actually affect the result (and self.matrix is no longer mutated as a side effect).
  3. Small --ntop — added guards that sys.exit with a clear message instead of crashing.

Tests (test_plotPCA.py, 14 passing)

Sign-invariant coordinate + eigenvalue regressions (default and transpose), --ntop behavior, variance/eigenvalue consistency, --PCs selection, and CLI validation exits. The sign-invariance helper normalizes along the correct component axis (columns for the untransposed table, rows for the transposed one) and stores raw goldens, so the suite is a fair cross-implementation net — it passes against both the sklearn and scipy backends rather than only agreeing with itself.

Note

pca_back currently fails to build its Rust extension locally (maturin errors), so the PCA tests were run via the working env pointed at this branch's pure-Python source. The rest of the suite (anything needing the compiled deeptools.hp) was not exercised here.

adRn-s added 2 commits July 9, 2026 11:03
# Conflicts:
#	pydeeptools/deeptools/test/test_plotPCA.py
test_plotPCA_default_coordinates failed on macOS CI: after PC1 the
untransposed eigenvalues are near-degenerate, so the eigenvectors rotate
freely and np.argpartition breaks top-ntop variance ties differently on
Accelerate vs OpenBLAS, making per-feature coordinates non-reproducible.
Replace it with test_plotPCA_default_eigenvalues, asserting only the
portable eigenvalues; coordinate-level regression stays covered by the
well-separated transpose case (test_plotPCA_transpose).

@WardDeb WardDeb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ty, looks good to me !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants