0.1.18: DDP fix + API hardening + repo polish + relax torch pin#29
Merged
Conversation
… DDP
Two related issues caused DDP training on mosaic data (non-uniform
per-sub-batch modality combinations) to either run with no throughput
gain or hang on an NCCL all-reduce. Both are fixed together because
fixing only the first would silently turn previously working slow DDP
runs into hanging fast DDP runs.
(1) Sampler selection (model.py):
The default sampler_type='auto' fell through to MultiBatchSampler, a
rank-agnostic sampler. Under DDP, every rank then iterated the same
N batches, giving 1x throughput at NxGPU cost. Fix: 'auto' now picks
MyDistributedSampler when torch.distributed is initialized.
(2) MyDistributedSampler.__iter__ (data.py):
The sampler called random.shuffle() on the global Python random
module, so each post-fork DDP rank used its own random state. Two
consequences:
- The dataset-visit order diverged across ranks. With non-uniform
per-sub-batch modality combinations, ranks ran the encoder over
different modality subsets at the same step, producing different
sets of "unused" parameters per rank. With find_unused_parameters
=False (Lightning default), DDP all-reduce expects identical grad
buffers per step and hung waiting for collectives that never
arrived; NCCL watchdog killed the run after the timeout.
- random.shuffle(indices) mutated self.all_indices in place, so
successive epochs cumulatively shuffled an already-shuffled list.
Fix: introduce two seeded random.Random instances per epoch — one
shared across ranks for the dataset-visit order, one rank-specific
for the within-dataset shuffle — and copy self.all_indices[idx]
before shuffling. Also call super().__init__() so self.epoch is
initialised and Lightning's automatic sampler.set_epoch(epoch) takes
effect each epoch.
Behavior change to call out:
- Single-GPU users (MultiBatchSampler) are unaffected.
- DDP users who passed sampler_type='ddp' explicitly: same fix path
applies; sampling order changes, so prior seeded DDP runs will not
reproduce numerically.
- DDP users on the implicit 'auto' default: now actually shard data
across ranks; expect a roughly NxGPU throughput improvement.
Release notes updated in docs/source/release.md.
Five user-visible bugs that survived 0.1.17, plus a regression test
file to pin them down.
* Encoder.forward: replace in-place ``data[m] *= mask`` with out-of-place
``data[m] = data[m] * mask``. Mathematically equivalent (mask is a
binary 0/1 modality-presence indicator and calc_recon_loss already
multiplies the loss by the same mask), but the in-place form mutated
the caller's batch dict for any modality without a ``trsf_before_enc_*``
transform. Defensive — makes the encoder safe to re-call on the same
batch (e.g. predict's mod_latent / translate paths).
* configure_optimizers: read ``load_optimizer_state`` via ``getattr(...,
False)``. The attribute is only set on the class by
configure_data_from_dir / configure_data_from_mdata / load_checkpoint,
so users entering through the simpler configure_data API previously
crashed on first trainer.fit with AttributeError.
* configure_data: default batch_names now use f-string formatting
(``f'batch_{i}'``) instead of the literal string ``'batch_%d'``
repeated len(datalist) times.
* configure_data: bad ATAC config now raises ValueError instead of
calling exit() — exit() killed the Jupyter kernel with no traceback.
* download_file: accept str or pathlib.Path for dest_path. The
signature was annotated str but the body called .name; both are
now supported.
* VAE.forward: drop the bare ``try / except: logging.debug(...)``
around the PoE call. The except swallowed real errors and the
debug call had the wrong arg shape; the PoE itself is well-behaved
for the supported input contract.
* model.py: drop unused ``import threading`` and the dead
``self.thread_lock = threading.Lock()`` (never acquired anywhere).
* .gitignore: add .claude/ and .nfs* to keep editor / NFS-stale-handle
files out of working tree status.
tests/test_invariants.py adds 5 regression tests covering each of
the above plus the DDP sampler determinism fix from 857e8f5
(cross-rank disjoint indices, set_epoch actually changes ordering).
Each basics demo now exposes a small ``# === GPU configuration ===`` block at the top of cell 1 (``GPUS`` + ``STRATEGY`` + derived ``DEVICES``), matching the pattern introduced by 2b5494b ("rewrite multi-GPU tutorial to match basics demos"). Switching from single-GPU to multi-GPU now only requires editing those two values, with a comment spelling out the ddp_notebook vs ddp script trade-off. With the demos themselves carrying the multi-GPU instructions, the standalone ``advanced/multi_gpu.rst`` tutorial is redundant — the common failure modes it described (devices='gpu' typo, hard-coded CUDA_VISIBLE_DEVICES='0', model.train vs L.Trainer split) are now addressed inline where they would actually be encountered. Drop the file and its toctree entry in advanced/index.rst. demo3 also gains a small refinement in cell 7: z_c / z_u stay as dense ndarray (they're 32-dim and 2-dim, sparsifying buys nothing), while x_bc reconstructions remain csr (RNA/ATAC are ~85% zeros, so csr saves real memory).
…e notes
* README:
- Drop the standalone "Update: Load data from MuData" section. The
from_mdata path is now a one-line "alternative input formats"
callout linking to the docs, instead of a 50-line near-duplicate
of the Quick Example.
- Quick Example: correct the misleading "input should be an AnnData
object" comment — configure_data_from_dir reads a directory of
per-batch MTX matrices, not AnnData.
- License badge: fix the broken link
(github.com/labomics/midas/LICENSE was 404; now points to
.../blob/main/LICENSE).
* Single-source the version. ``pyproject.toml`` is the sole
authority; ``scmidas.__version__`` and Sphinx ``release`` both
read it via ``importlib.metadata.version("scmidas")``. Removes
the previous three-place duplication that drifted historically.
* docs/source/release.md: promote the "Unreleased" section to
``v0.1.18 (2026-05-02)`` and add entries for the new API
hardening fixes, the test suite, the docs reorganisation, and
the packaging change.
* Bump version 0.1.17 → 0.1.18.
…ick Example with API sketch The README repeated "what is MIDAS" four times (title, tagline, two-paragraph prose, Key Features) and surfaced the paper / documentation links twice each. Trimming: * Drop the two-paragraph "MIDAS is a powerful deep probabilistic framework..." block — the tagline above it and Key Features below it cover the same ground without the marketing register. * Drop the standalone "Publication" bullet under the description; the Citation section already has the same link. * Drop "For more detailed tutorials..." — the docs link is now in the Quick Start preamble where it actually fits. * Tighten Key Features bullets — strip "powerful", "Seamlessly", "Accurately", "Effectively", "Advanced", "highly efficient"; keep the concrete capability per bullet. Quick Start is also reshaped: * The previous "Quick Example" looked runnable but wasn't — ``'path/to/your/data'`` was a placeholder, ``predict()`` / ``get_emb_umap()`` depended on session state. Users who tried to copy-paste it would hit failures. * Replaced with a 4-call API sketch using ``...`` placeholders so it's clearly not a script. The MuData alternative is now a comment in the first step instead of a duplicated block below. Net: 113 lines → 95 lines, no information loss for the reader who clicks through to the docs.
…n contributing
* Title: ``MIDAS: A Deep Generative Model for Mosaic Integration and
Knowledge Transfer of Single-Cell Multimodal Data`` → ``MIDAS``.
The tagline below already states what MIDAS does; the long subtitle
was paper-style and made the H1 line wrap on narrow screens.
* Drop section-header emoji (✨🚀⚡📈📜🙌📝). Matches the convention
of scvi-tools / scanpy and prints cleanly in non-emoji terminals.
* Logo width 900px → 450px so it doesn't get squashed in GitHub's
mobile rendering (and isn't oversized on desktop either).
* Reproducibility section: drop the trailing slash on the branch URL
and reword to one line.
* Citation:
- Drop the duplicated plain-text human-readable line — the bibtex
contains the same information.
- Fix bibtex inaccuracies:
* Author list now full (15 authors, no "and others"); previously
truncated at 11 with et-al wildcard.
* pages: ``1--12`` (placeholder) → ``1594--1605``.
* Add volume (42), number (10), and doi.
* publisher: ``Nature Publishing Group US New York`` (auto-tool
output) → ``Nature Publishing Group``.
* Wrap ``MIDAS`` in braces so bibtex preserves the casing.
* Contributing section: replace generic boilerplate with concrete
guidance (open issue / branch from main / pytest passes / discuss
non-trivial changes first).
* Drop ``Get started with MIDAS by setting up a conda environment.``
preamble and the comment-numbered steps in the install block;
three commands are self-explanatory.
Net: 95 lines → 92 lines, but readability up.
The previous trim to 450px was an over-correction. The source PNG is 2626×535 (~5:1 banner) — at 450px the rendered height drops to about 91px and the wordmark goes blurry. Mobile browsers already auto-scale the image to fit the viewport, so the original 900px was not actually being squashed; it just rendered smaller on narrower screens, which is the desired behaviour.
…roject URLs * CI: the test workflow now runs on Python 3.10, 3.11 and 3.12 (matrix, fail-fast disabled). Previously only 3.12 was exercised, even though pyproject.toml declares ``requires-python = ">=3.10"``. This makes the declared support promise an actually-enforced one. * PEP 561: add empty ``src/scmidas/py.typed`` marker and ship it via ``[tool.setuptools.package-data]`` so type checkers (mypy / pyright / pylance) honour scmidas's annotations instead of treating the package as untyped. Improves IDE jump-to-definition / hover for downstream users. * README: add a CI status badge so users / reviewers can see at a glance whether ``main`` is green (matches scvi-tools / scanpy README convention). * pyproject.toml ``[project.urls]``: add Documentation, Source, and Changelog entries. PyPI surfaces these as a "Project links" sidebar; the previous two URLs (Homepage + Issues) hid the docs link from the PyPI page despite the docs being well-developed.
…plates) GitHub's Insights → Community Standards page treats these as the baseline a "professional" repo is expected to surface, and they match the file set scvi-tools / scanpy / numpy / pandas all ship. None of them touch source code or runtime behaviour. * CONTRIBUTING.md (root): concrete dev setup (clone + dev extras + pytest), bug-report what-to-include, PR workflow, when to file an issue first. Replaces the generic "We welcome contributions!" paragraph that was in the README. * CHANGELOG.md (root): one-paragraph stub pointing to docs/source/release.md (the canonical changelog rendered on the docs site). Exists so the repo header surfaces a "CHANGELOG" link and tools that scan repo roots can find it. * CODE_OF_CONDUCT.md (root): Contributor Covenant 2.1 verbatim from contributor-covenant.org, with the contact placeholder set to omicshub@outlook.com (the project maintainer address from pyproject.toml). * .github/ISSUE_TEMPLATE/bug_report.md and feature_request.md: Markdown templates pre-populated with the fields that actually help diagnose MIDAS issues (scmidas/torch versions, single vs multi-GPU, full traceback in a code block). Preferred over an empty "Describe the issue" textarea. * .github/ISSUE_TEMPLATE/config.yml: disables blank issues so users pick a template, and adds a Discussions contact link for questions that aren't bug reports. * .github/PULL_REQUEST_TEMPLATE.md: short PR checklist (tests pass, docstrings updated, release notes touched, design discussed for non-trivial changes).
The previous ``torch>=2.5,<2.6`` cap was a defensive workaround for a suspected Lightning DDP incompatibility on torch >= 2.6. That worry has not held up in practice: * The 0.1.18 hardening pass (notably the ``MyDistributedSampler`` fix in 857e8f5) was developed and validated on torch 2.8.0 + cu128. * A full 1000-epoch demo3 mosaic DDP run on 4× GPU with the new code was completed; the resulting UMAP and per-modality embeddings match the single-GPU baseline. No NCCL hangs, no watchdog timeouts. Widen to ``torch>=2.5,<3`` (with matching ``torchvision>=0.20,<1`` / ``torchaudio>=2.5,<3``) so users on torch 2.6 / 2.7 / 2.8 no longer have to manually override the pin. The major-version cap stays in place so a hypothetical breaking torch 3.x release does not silently land via a regular ``pip install --upgrade``. CI continues to pin ``torch==2.5.1+cpu`` explicitly in the workflow file, so this change does not alter what gets exercised in the matrix runs — it only widens the surface ``pip install scmidas`` will accept in the wild.
zhen-he
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
8 commits, target 0.1.18 release.