Skip to content

Fix pca when the sample nodes are not 0..n-1 - #3498

Draft
Mohit-Ak wants to merge 1 commit into
tskit-dev:mainfrom
Mohit-Ak:fix/pca-sample-ids
Draft

Mohit-Ak wants to merge 1 commit into
tskit-dev:mainfrom
Mohit-Ak:fix/pca-sample-ids

Conversation

@Mohit-Ak

Copy link
Copy Markdown

Description

TreeSequence.pca raised IndexError whenever the sample nodes were not 0, ..., n - 1, e.g. the census-node case in #3365 or the relabelled example in #3443. _expand_indices wrote each row of the weight matrix at the node ID, but genetic_relatedness_vector expects the rows in ts.samples() order.

This maps the node IDs to their position in ts.samples() first, and raises a ValueError if a node passed in is not a sample.

Tested with a new TestPCA::test_samples_not_first_nodes: it renumbers the nodes of a simulated tree sequence so the samples start at node 5 and checks that the PCA (all samples, a subset of samples, and individuals) matches the original. It fails on main with the IndexError and passes with the change. pytest tests passes and ruff check/format are clean.

Fixes #3443
Fixes #3365

PR Checklist:

  • Tests that fully cover new/changed functionality.
  • Documentation including tutorial content if appropriate.
  • Changelogs, if there are API changes.

_expand_indices used node IDs as row indices into the weight matrix,
which has one row per sample in ts.samples() order, so pca raised
IndexError whenever the samples were not nodes 0..n-1.

Fixes tskit-dev#3443
Fixes tskit-dev#3365

This branch has not been deployed

No deployments
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.

PCA fails sometimes (maybe if nodes are not 0...n?) ts.pca gives IndexError for unsimplified tree sequence

1 participant