Skip to content

Preserve selected negative velocity graph in transition_matrix - #1347

Open
dnncha wants to merge 3 commits into
theislab:mainfrom
dnncha:fix/transition-obsp-negative-graph
Open

dnncha wants to merge 3 commits into
theislab:mainfrom
dnncha:fix/transition-obsp-negative-graph

Conversation

@dnncha

@dnncha dnncha commented Sep 30, 2026

Copy link
Copy Markdown

When both velocity graphs are stored in adata.obsp, transition_matrix initially selects both graphs there, then replaces the selected negative graph with the .uns copy. AnnData reorders pairwise .obsp matrices when cells are reordered; the .uns copy retains its original ordering. The resulting transition kernel mixes two different cell orders. A missing .uns negative copy also raises KeyError despite the selected .obsp negative graph being present.

Remove that later reassignment so the already selected negative graph is used. The added tests check cell-permutation equivariance with and without self transitions, missing .uns negative copies, .uns-only inputs, and explicit vgraph inputs, in both negative-cosine modes.

Related to #1330, which already identifies the overwrite in the basis-constraint path. This PR demonstrates and fixes its graph-storage consequence; it does not add the negative-graph basis mask proposed there and does not claim to fully resolve that issue.

Validation against main f63c0e70596ced2f1bee8cf07e8ab66037cf86b2:

  • Before this deletion: PYTHONPATH=. .venv/bin/python -m pytest tests/tools/test_transition_matrix.py -q — 6 failed, 4 passed.
  • After: the same focused command — 10 passed.
  • Test-file Ruff lint and formatting, selected production-file correctness/style lint (F,E,W,B,C4,UP,RUF100), and git diff --check pass. Full suite was not run. Existing public-docstring D413 warnings in the production file remain outside this change.
  • A separate public-API reproduction on release 0.3.4 and current main, using a selected synthetic four-cell stress fixture, gives a maximum default-mode transition-probability discrepancy of 0.7188126488078231 after permutation. The fix reduces it to floating-point roundoff (2.22e-16). .uns-only and explicit-vgraph controls are numerically identical across release, main and patch in both modes. This selected fixture is not a prevalence estimate or a mathematical worst-case bound; no real-study impact has been assessed. With use_negative_cosines=True, outputs are signed kernel values rather than nonnegative probabilities.

The existing positive-graph .uns validation requirement is unchanged; this is not a general .obsp-only API expansion. AI-assisted implementation and verification.

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.

1 participant