Skip to content

plotPCA: write and plot the per-sample loadings in the untransposed layout - #1460

Open
cindykrafft wants to merge 2 commits into
deeptools:masterfrom
cindykrafft:fix4/plotpca-loadings
Open

plotPCA: write and plot the per-sample loadings in the untransposed layout#1460
cindykrafft wants to merge 2 commits into
deeptools:masterfrom
cindykrafft:fix4/plotpca-loadings

Conversation

@cindykrafft

Copy link
Copy Markdown

Fixes #1459.

The re-implemented Correlation.plot_pca set Wt = U * S for both layouts. With the samples as the variables (the default layout) U * S holds the PC scores of the rows of the matrix — one row per selected bin — so the "Component i" row of --outFileNameData and the plotted points were the first rows' scores across the components, labelled with the sample names (row norms 1.5–1.9 instead of 1; the groups that PC1 separates in the loadings are separated by no row of the table). The help documents "the loadings for each sample in each principal component"; those are the rows of V^T, which 3.5.x wrote. --transpose (each sample's projection, (U * S).T) was already right and is unchanged, as are the eigenvalues, the variance fractions and the scree plot. The "not enough principal components to plot N samples" exit is removed: every component row has one loading per sample, so --ntop below the sample count plots fine as long as the requested PCs exist (the remaining check). Review notes: the golden test_plotPCA_default.png/.tsv and test_plotPCA_ggplot.png/.tsv showed the bin scores and are regenerated (the corrected plots were 6.8 and 5.7 RMS from the old goldens, under the image tolerance of 50, which is why the image tests did not catch this); test_plotPCA_ntop_smaller_than_samples now expects one loading per sample (a 2 × 8 table for --ntop 2, plot included) and test_plotPCA_requested_PC_beyond_ntop_errors_cleanly keeps the clean-exit coverage (--ntop 2 --PCs 1 3).

Test: test_plotPCA_default_table_holds_sample_loadings — the rows of the default table are orthonormal (W @ W.T == I) and PC1 equals a numpy SVD's V^T[0] on the same standardised rows, sign-invariantly. On master it fails (36/36 elements of W @ W.T mismatch) together with the adjusted --ntop 2 test (SystemExit: Not enough principal components); with the patch test_plotPCA.py + test_plotcorrelation.py 20 passed, correlation.py doctests pass, full pytest suite passes (numbers in test-runs.txt).

Found in Mytochondria, a volunteer project that checks the numerical core of research software and verifies every finding by execution (methods and harnesses: https://github.com/cindykrafft/mytochondria/tree/main/audits/deeptools)


Generated by Claude Code


Cynthia Condra and others added 2 commits September 5, 2026 15:00
…ayout

The re-implemented plot_pca set Wt = U * S for both layouts.  With samples
as the variables (the default, untransposed layout) U * S holds the PC
scores of the rows of the matrix -- one row per selected bin -- so the
table's "Component i" row and the plotted points were the first rows'
scores across components, labelled with the sample names.  The help
documents "the loadings for each sample in each principal component";
those are the rows of V^T.  --transpose (samples as observations, each
sample's projection U * S) was already correct.

The "not enough principal components to plot N samples" exit is
removed: every component row has one loading per sample, so --ntop
below the sample count plots fine as long as the requested PCs exist
(the remaining check).  The golden plotPCA PNGs/TSVs showed the bin
scores and are regenerated.

Test: test_plotPCA_default_table_holds_sample_loadings (rows of the
default table are orthonormal and PC1 equals a numpy SVD of the same
standardised rows); test_plotPCA_ntop_smaller_than_samples now expects
one loading per sample (2 x 8 table) and
test_plotPCA_requested_PC_beyond_ntop_errors_cleanly keeps the
clean-exit coverage.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TaHntBDKuZJpMAAMenkC44
The wrapper's second test pinned the values plotPCA wrote before this
change (1.99949, 0.96079: PC scores of the first bins). With the two
identical input samples the loadings are +-1/sqrt(2) on PC1 with
eigenvalue 3 and a degenerate PC2 (BLAS-dependent noise), so the
assertions now check the PC1 loadings sign-agnostically and only the
structure of the second row. The golden PNGs and tabular are
regenerated from the corrected output (the PNGs compare by sim_size).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TaHntBDKuZJpMAAMenkC44
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.

plotPCA (4.0.0 re-implementation): the default table and plot hold the PC scores of the first bins, not the loadings of the samples

2 participants