Skip to content

Merge scanorama back into one component and fix its scrambled output - #113

Merged
rcannood merged 1 commit into
mainfrom
fix-scanorama
Aug 21, 2026
Merged

Merge scanorama back into one component and fix its scrambled output#113
rcannood merged 1 commit into
mainfrom
fix-scanorama

Conversation

@rcannood

Copy link
Copy Markdown
Member

Describe your changes

Reverts the split from #88 and fixes the bug that motivated it.

The low score that #88 set out to correct wasn't caused by reporting two output types in one entry -- it was caused by methods/scanorama scrambling its own output along both axes:

  • scanorama.correct_scanpy() returns one object per batch, so AnnData.concatenate() produced the cells grouped by batch, while obs=adata.obs[[]] labelled them with the original order
  • scanorama also returns the genes sorted by name, while var=adata.var[[]] labelled them with the original order

Both outputs were affected, and since transform derives X_emb and the kNN graph from corrected_counts for a feature method, every metric was scored on mismatched data -- not just hvg_overlap. Splitting the component happened to fix the embedding side, because scanorama_integrate scatters X_scanorama back into the original row order instead of concatenating. scanorama_correct kept the bug, so its 0.3051 is still not meaningful.

So this PR puts the two components back together and fixes the ordering instead. Both outputs come from a single correct_scanpy(return_dimred=True) run, which is what the component did before #88, so there's no reason to run scanorama twice. If we ever want the modes as separate entries, an argument is a cheaper way to get there than two components.

Verification

Mean per-cell correlation between corrected_counts and the input cell it claims to describe, on resources_test/task_batch_integration/cxg_immune_cell_atlas:

corrected_counts vs its own input cell gene axis vs input gene order
scanorama_correct on main -0.0006 -0.0021
scanorama in this PR +0.7900 +0.9883

+0.7900 is exactly the value you get by taking main's output and manually undoing the two permutations, so the fix recovers the intended result rather than changing it.

No regression on the embedding side: X_emb from this component is bit-identical to scanorama_integrate on main (600 x 100, max abs diff 0.000e+00), which also confirms correct_scanpy(return_dimred=True, dimred=100) and integrate_scanpy(dimred=100) compute the same thing.

viash test src/methods/scanorama/config.vsh.yaml succeeds.

Also dropped the merge_adata() helper, which only existed to wrap the deprecated AnnData.concatenate().

Checklist before requesting a review

  • I have performed a self-review of my code

  • Check the correct box. Does this PR contain:

    • Breaking changes
    • New functionality
    • Major changes
    • Minor changes
    • Bug fixes
  • Proposed changes are described in the CHANGELOG.md

  • CI Tests succeed and look good!

* both outputs come from a single `correct_scanpy(return_dimred=True)` run, so there is no reason to run scanorama twice
* put the cells and genes back in the order of the input -- scanorama returns one object per batch with the genes sorted by name, so both axes were permuted with respect to the `obs` and `var` they were labelled with
* dropped the `merge_adata()` helper along with the deprecated `AnnData.concatenate()` it wrapped
@rcannood
rcannood merged commit 42d089a into main Aug 21, 2026
2 checks passed
@rcannood
rcannood deleted the fix-scanorama branch August 21, 2026 14:43
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