diff --git a/.gitignore b/.gitignore index f16aaf3..9c413cc 100644 --- a/.gitignore +++ b/.gitignore @@ -10,6 +10,4 @@ dist/ htmlcov/ uv.lock -# slurm job logs from benchmarks/ -benchmarks/slurm-*.out diff --git a/CHANGELOG.md b/CHANGELOG.md index c419954..1681e5e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -900,7 +900,7 @@ Distribution remains **Arc-internal** — install from the `v0.3.0` git tag literal-reference path** — lands ~4096 on A100-40GB/CCL_2, the bench-measured throughput knee. Measured `de()` speedup (result-invariant): **~7% on A100-40GB** (GCP sweep `20260530T012835Z`, where 0.18 under-sizes to ~3712) - and **~3% on H100-80GB** (slurm: 47.6s → 46.2s, chunk 8256 → 9152); neutral / + and **~3% on H100-80GB** (47.6s → 46.2s, chunk 8256 → 9152); neutral / no regression on larger GPUs (the curve is flat past the knee). Still scales inversely with the reference-pool size. The `ALL_OTHERS` path now uses 0.20 as well, once #28 gave it the same OOM-recovery backstop. (#22, #28) diff --git a/CITATION.cff b/CITATION.cff new file mode 100644 index 0000000..48d9f5f --- /dev/null +++ b/CITATION.cff @@ -0,0 +1,28 @@ +cff-version: 1.2.0 +message: "If you use gpudge in your research, please cite it as below." +title: "gpudge: GPU Mann-Whitney differential expression for single-cell CRISPR screens" +abstract: >- + A GPU-only Mann-Whitney U differential expression library for single-cell + CRISPR perturbation screens. One entry point, de(), taking an in-memory + AnnData, a separate control AnnData, a caller-supplied cell source, or a + streamed shardad archive. Two-sided asymptotic Mann-Whitney with tie and + continuity corrections, matching scipy.stats.mannwhitneyu, with per-group + Benjamini-Hochberg adjustment and rank-based effect sizes. +type: software +authors: + - family-names: Dobin + given-names: Alexander + affiliation: "Arc Institute" +version: 0.8.0 +date-released: "2026-08-18" +license: MIT +repository-code: "https://github.com/ArcInstitute/gpudge" +keywords: + - single-cell + - differential-expression + - mann-whitney + - wilcoxon + - CRISPR + - perturbation-screen + - GPU + - CUDA diff --git a/CODE_OF_CONDUCT.md b/CODE_OF_CONDUCT.md new file mode 100644 index 0000000..f9f3adf --- /dev/null +++ b/CODE_OF_CONDUCT.md @@ -0,0 +1,130 @@ +# Contributor Covenant Code of Conduct + +## Our Pledge + +We as members, contributors, and leaders pledge to make participation in our +community a harassment-free experience for everyone, regardless of age, body +size, visible or invisible disability, ethnicity, sex characteristics, gender +identity and expression, level of experience, education, socio-economic status, +nationality, personal appearance, race, caste, color, religion, or sexual +identity and orientation. + +We pledge to act and interact in ways that contribute to an open, welcoming, +diverse, inclusive, and healthy community. + +## Our Standards + +Examples of behavior that contributes to a positive environment for our +community include: + +* Demonstrating empathy and kindness toward other people +* Being respectful of differing opinions, viewpoints, and experiences +* Giving and gracefully accepting constructive feedback +* Accepting responsibility and apologizing to those affected by our mistakes, + and learning from the experience +* Focusing on what is best not just for us as individuals, but for the overall + community + +Examples of unacceptable behavior include: + +* The use of sexualized language or imagery, and sexual attention or advances of + any kind +* Trolling, insulting or derogatory comments, and personal or political attacks +* Public or private harassment +* Publishing others' private information, such as a physical or email address, + without their explicit permission +* Other conduct which could reasonably be considered inappropriate in a + professional setting + +## Enforcement Responsibilities + +Community leaders are responsible for clarifying and enforcing our standards of +acceptable behavior and will take appropriate and fair corrective action in +response to any behavior that they deem inappropriate, threatening, offensive, +or harmful. + +Community leaders have the right and responsibility to remove, edit, or reject +comments, commits, code, wiki edits, issues, and other contributions that are +not aligned to this Code of Conduct, and will communicate reasons for moderation +decisions when appropriate. + +## Scope + +This Code of Conduct applies within all community spaces, and also applies when +an individual is officially representing the community in public spaces. +Examples of representing our community include using an official email address, +posting via an official social media account, or acting as an appointed +representative at an online or offline event. + +## Enforcement + +Instances of abusive, harassing, or otherwise unacceptable behavior may be +reported to the community leaders responsible for enforcement at +**alexander.dobin@arcinstitute.org**. + +All complaints will be reviewed and investigated promptly and fairly. + +All community leaders are obligated to respect the privacy and security of the +reporter of any incident. + +## Enforcement Guidelines + +Community leaders will follow these Community Impact Guidelines in determining +the consequences for any action they deem in violation of this Code of Conduct: + +### 1. Correction + +**Community Impact**: Use of inappropriate language or other behavior deemed +unprofessional or unwelcome in the community. + +**Consequence**: A private, written warning from community leaders, providing +clarity around the nature of the violation and an explanation of why the +behavior was inappropriate. A public apology may be requested. + +### 2. Warning + +**Community Impact**: A violation through a single incident or series of +actions. + +**Consequence**: A warning with consequences for continued behavior. No +interaction with the people involved, including unsolicited interaction with +those enforcing the Code of Conduct, for a specified period of time. This +includes avoiding interactions in community spaces as well as external channels +like social media. Violating these terms may lead to a temporary or permanent +ban. + +### 3. Temporary Ban + +**Community Impact**: A serious violation of community standards, including +sustained inappropriate behavior. + +**Consequence**: A temporary ban from any sort of interaction or public +communication with the community for a specified period of time. No public or +private interaction with the people involved, including unsolicited interaction +with those enforcing the Code of Conduct, is allowed during this period. +Violating these terms may lead to a permanent ban. + +### 4. Permanent Ban + +**Community Impact**: Demonstrating a pattern of violation of community +standards, including sustained inappropriate behavior, harassment of an +individual, or aggression toward or disparagement of classes of individuals. + +**Consequence**: A permanent ban from any sort of public interaction within the +community. + +## Attribution + +This Code of Conduct is adapted from the [Contributor Covenant][homepage], +version 2.1, available at +https://www.contributor-covenant.org/version/2/1/code_of_conduct.html. + +Community Impact Guidelines were inspired by +[Mozilla's code of conduct enforcement ladder][mozilla]. + +For answers to common questions about this code of conduct, see the FAQ at +https://www.contributor-covenant.org/faq. Translations are available at +https://www.contributor-covenant.org/translations. + +[homepage]: https://www.contributor-covenant.org +[mozilla]: https://github.com/mozilla/inclusion diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..84bb53d --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,106 @@ +# Contributing to gpudge + +Thanks for looking. gpudge is a small, deliberately narrow library — one public +entry point, `de()` — so most contributions are bug reports, test cases, and +documentation fixes rather than new features. + +## What is and is not in this repository + +`src/` is the library, `tests/` its suite, and `docs/` and `examples/` its +documentation. `benchmarks/` holds performance and comparison harnesses that +are not part of the installed package. + +Some code comments cite design specs by label (`spec 3.2b`, `Semantics A`). +Those documents are not shipped with the package and may not be in your +checkout at all. Where such a label carries reasoning you need, that reasoning +is restated in the code — if you find one that is a bare pointer with nothing +behind it, that is a bug worth reporting. + +## Setting up + +Python **≥ 3.11**, and a **CUDA GPU** for anything that calls `de()`. There is +no CPU fallback; without a GPU it raises rather than running slowly. + +```bash +git clone https://github.com/ArcInstitute/gpudge.git && cd gpudge +uv venv && uv pip install --torch-backend=cu126 -e ".[dev,fast]" +``` + +Use `uv pip install`, **not `uv sync`** — `uv sync` builds uv's universal lock, +which resolves every entry in `[tool.uv.sources]`, including the private +`shardad` source, even when you did not ask for the `streaming` extra; without +SSH access to that repository it fails with `Permission denied (publickey)`. +`uv pip install` resolves only the extras you name. `[fast]` adds numba, which +the fast CSR kernel needs; `[dev]` adds pytest, ruff, scanpy and pyyaml. + +## Running the tests + +```bash +uv run --no-sync pytest tests/ # most of it needs no GPU +uv run --no-sync ruff check src/ tests/ examples/ # exactly what CI runs +``` + +`uv venv` creates `.venv` but does not activate it, so `uv run --no-sync` is +what makes a bare shell use it — these are the two commands CI runs verbatim. + +Two things that surprise people: + +- **`pytest -m needs_cuda` selects nothing and exits 5.** `needs_cuda` is a + `skipif` decorator in `tests/conftest.py`, not a registered marker. Run the + whole suite on a GPU host instead. +- **CI is CPU-only**, so the GPU bit-identity gates and the GPU-backed parity + assertions never execute there. Green CI is necessary, not sufficient, for a + change touching a GPU path — say in your PR whether you ran the suite on a + CUDA host, and what it reported. + +Three suites (`test_shard_stream.py`, `test_cell_stream.py`, +`test_inmem_external_ref_gpu.py`) call `importorskip("shardad")` at module +level, so without that optional dependency they are not collected at all — they +appear as 3 skips rather than as their real case count. + +## The bar for tests + +This repository has been bitten repeatedly by tests that could not fail: a +byte-identity gate that was silently a tolerance check, because polars' +`assert_frame_equal` defaults to `check_exact=False`; assertions with tolerances +four orders looser than the measured agreement; an oracle branch that had never +once executed and was wrong when it finally did. Six such gates were repaired in +a single release. + +So the expectation for a test accompanying a fix is concrete: + +> **Break the fix and watch the test go red.** Then put it back. + +If a test cannot be made to fail by reverting the thing it covers, it is not +testing that thing. Saying so in the PR — "reverting X turns this red" — is the +most useful sentence you can write, and reviewers here will ask for it. + +## Documentation that is enforced + +Some prose is checked by tests, and changing it means changing the test too: + +- `docs/tutorial.md`'s published numbers are recomputed by + `tests/test_tutorial.py` from an independent SciPy oracle. Edit the transcript + and the test fails. +- The committed tutorial dataset is pinned by sha256. +- The test counts quoted in `CLAUDE.md` are measurements. If your change moves + them, re-measure rather than adjusting them by hand. + +## Pull requests + +Branch, open a PR, get CI green. Keep commits and PR descriptions factual about +what was verified and what was not — "I did not run the GPU suite" is a fine +thing to write, and much better than silence. If a reviewer's suggestion looks +wrong to you, say why rather than applying it; several suggestions in this +repository's history were technically reasoned and empirically false. + +## Reporting a bug + +Include the gpudge version, the Python and torch versions, the GPU, and the +shape of the input (`n_obs × n_vars`, number of groups, sparse or dense, dtype). +For a numerical disagreement the most useful report is a small reproducer plus +what you expected and why — a comparison against `scipy.stats.mannwhitneyu` with +`method="asymptotic"` is the reference gpudge is built to match. + +Security issues go through [SECURITY.md](SECURITY.md), not the public tracker. +Conduct concerns go through [CODE_OF_CONDUCT.md](CODE_OF_CONDUCT.md). diff --git a/SECURITY.md b/SECURITY.md new file mode 100644 index 0000000..9ce9fac --- /dev/null +++ b/SECURITY.md @@ -0,0 +1,50 @@ +# Security policy + +## Reporting a vulnerability + +Please report privately, through GitHub's **[private vulnerability +reporting](https://docs.github.com/en/code-security/security-advisories/guidance-on-reporting-and-writing-information-about-vulnerabilities/privately-reporting-a-security-vulnerability)** +— the *Security* tab of this repository, then *Report a vulnerability*. That +opens a draft advisory only the maintainers can see. + +Please do not open a public issue for a suspected vulnerability. + +What is useful in a report: what an attacker controls, what they achieve, and a +reproducer if you have one. gpudge is maintained by a small team alongside other +work, so expect an acknowledgement in days rather than hours. If a report turns +out to be a plain bug rather than a vulnerability we will say so and move it to +the public tracker, with your agreement. + +## Supported versions + +The latest release only. gpudge is pre-1.0 and there are no maintenance +branches: fixes land on `main` and go out in the next release rather than being +backported. + +## What the threat model actually is + +gpudge is a compute library. It does not open sockets, spawn processes, or +authenticate anyone. Realistically the interesting surface is **the input you +hand it**: + +- an untrusted `.h5ad` or `shardad` archive — parsing is done by `anndata`, + `h5py` and `shardad`, so a malicious file is mostly *their* attack surface, + but a crash or a wild allocation reachable through gpudge's own slicing and + chunk-sizing code is in scope here; +- `de(cell_source=…)`, which runs a callable you supply — that is by design, and + passing an untrusted callable is equivalent to running untrusted code; +- resource exhaustion: gpudge deliberately sizes GPU chunks against available + VRAM and recovers from OOM, so a hostile input shaped to defeat that sizing is + a legitimate report. + +Out of scope: that `de()` requires a CUDA GPU and raises without one; that +`densify_input=True` mutates `adata.X` in place and needs host RAM proportional +to the input (both documented); and anything that requires already being able to +run arbitrary code in the same process. + +## Dependencies + +Third-party licences and the environment a release was audited against are +recorded in [`docs/THIRD_PARTY_LICENSES.md`](docs/THIRD_PARTY_LICENSES.md). A +vulnerability in a dependency is best reported upstream first; tell us too, so +the floor can be raised here. diff --git a/docs/reviews/2026-06-27-gpudge-ultrareview.md b/docs/reviews/2026-06-27-gpudge-ultrareview.md index e405fc4..d56e53a 100644 --- a/docs/reviews/2026-06-27-gpudge-ultrareview.md +++ b/docs/reviews/2026-06-27-gpudge-ultrareview.md @@ -8,8 +8,13 @@ Severity breakdown: - **Critical:** 0 - **High:** 1 - **Medium:** 3 -- **Low:** 10 -- **Nit:** 8 +- **Low:** 11 +- **Nit:** 7 + +(The Low/Nit split read 10/8 until 2026-08-18. The findings below, and the +resolution table added the same day, are L1–L11 and N1–N7: a transcription +error in this summary, not a change to any finding. The total of 22 was always +right.) No live correctness bug exists in any realistic, supported code path. The core MWU statistic is bit-perfect vs scipy. The findings cluster into: one genuine silent-corruption risk on real-world input, a band of test-coverage gaps around the numerical core and the v0.3.0 streaming headline, a few version/doc drifts from the release, and a long tail of degenerate-input and budgeting nits. @@ -21,6 +26,43 @@ No live correctness bug exists in any realistic, supported code path. The core M --- +## Resolution status + +Added 2026-08-18. **All 22 findings were addressed in v0.3.1 ([#59](https://github.com/ArcInstitute/gpudge_arc/pull/59))**, the release immediately following this review; two of them deliberately by deciding no code should change. Statuses below were re-verified against the tree at `v0.8.0`, and the *Evidence* column names something you can open — a regression test that cites the finding ID, or the code that now carries the guard. + +| # | Severity | Finding | Status | Evidence | +|---|---|---|---|---| +| H1 | High | NaN/None `groupby` labels bucketed into `'nan'`/`'None'` groups | **Fixed** (v0.3.1); guard broadened in v0.8.0 ([#124](https://github.com/ArcInstitute/gpudge_arc/pull/124)) | `_ingest.MISSING_LABEL_SPELLINGS` and the mirroring guard in `_shard_stream.py`, which was the only streaming layout when this was fixed; `_cell_stream.py` gained the same screen when the cell layout arrived in v0.7.0. ⚠️ One backend-parity gap remains open as [#127](https://github.com/ArcInstitute/gpudge_arc/issues/127) — a group *genuinely* named `nan` is rejected on streaming but accepted in memory | +| M1 | Medium | MWU-vs-scipy correctness tests all `@needs_cuda` | **Fixed** | `test_mwu.py::test_mwu_ref_matches_scipy_cpu`, which cites M1 | +| M2 | Medium | Streaming equivalence checked Pearson correlation only | **Fixed** | `test_shard_stream.py::_assert_equiv` — `allclose(rtol=1e-5, atol=1e-7, equal_nan=True)` over full row coverage, with the M2 reasoning at the call site | +| M3 | Medium | `environment.yml` pinned `gpudge @v0.2.0` | **Fixed** | pins the current release tag; kept in step with README by a note in both | +| L1 | Low | `all_others` N==1 → `tie_corr` divide-by-zero NaN | **Fixed** | `test_api.py`, docstring cites L1 | +| L2 | Low | ref-mode empty target reported `p_value=0.0` | **Fixed** | `_mwu.py` returns zeros + NaN when `m == 0 or n_ref == 0` | +| L3 | Low | `epsilon` accepted NaN and +inf | **Fixed** | `math.isfinite(epsilon)` guard; `test_de_rejects_nonfinite_epsilon` (+ the streaming-dispatch twin) | +| L4 | Low | `output_columns={}` yielded a degenerate frame | **Fixed** | `test_api.py`, docstring cites L4 | +| L5 | Low | Non-string `reference` gave an opaque error | **Fixed** | `test_api.py`, docstring cites L5 | +| L6 | Low | `all_others` chunk budget under-estimated GPU peak | **Fixed** | `_stream.py` comment cites L6 | +| L7 | Low | Streaming heuristic budgeted accumulators Phase 1 never allocates | **Fixed** (v0.3.1); model refined again in v0.8.0 | the `UNCONDITIONAL:` comment on the sizer in `__init__.py`, which states that the Phase-1 target tile is modelled whenever the sizer runs | +| L8 | Low | Phase-1 `del shard, Xs` defeated by the `_chunk` closure | **Fixed** | v0.3.1 "`/dev/shm` cleanup"; `del Xs, Ls` per shard in `_shard_stream.py` | +| L9 | Low | `log2_fold_change` never checked against a closed form | **Fixed** | `test_api.py`, docstring cites L9 | +| L10 | Low | Multi-block path of `_tie_term_per_gene` never exercised | **Fixed** | `test_mwu.py::test_tie_term_per_gene_multi_block`, cites L10 | +| L11 | Low | `environment.yml` recommended `shardad @v0.2.0` | **Fixed** | pins `shardad[cell] v0.7.1`, the floor both archive layouts need | +| N1 | Nit | `epsilon=0` + all-zero gene → NaN/inf `log2_fold_change` | **Documented, then pinned** | documented in the `de()` docstring and README; the degenerate value is pinned by a test added in v0.8.0 | +| N2 | Nit | `.item()` forces per-chunk GPU→CPU syncs | **No change** | a perf nit with no correctness impact; the syncs are still there in `_mwu.py` | +| N3 | Nit | A group literally named `'all_others'` is remapped | **No change, deliberate** | this review's own recommendation was "no action before the legacy spelling is removed"; the behaviour is noted in `__init__.py` | +| N4 | Nit | `mwu_ref` sentinel test checked only the p-value half | **Fixed** | `test_mwu.py` now asserts `(U[ref_idx] == 0).all()`, citing N4 | +| N5 | Nit | `MeanCalc` / `__version__` absent from the README API docs | **Fixed** | both documented in the README ([#126](https://github.com/ArcInstitute/gpudge_arc/pull/126)) | +| N6 | Nit | `csr_row_sums` docstring said "CSR" but accepts dense | **Fixed** | now "Per-row sum of a CSR sparse OR dense matrix" | +| N7 | Nit | README/SKILL listed 5 of the 10 default output columns | **Fixed** | both now list all ten under a `columns (10):` heading | + +Two entries are **not** code changes, and are listed that way on purpose: N2 was +judged a perf nit not worth the churn, and N3 was a reserved-word behaviour this +review itself recommended leaving alone until the legacy `"all_others"` spelling +is removed. "Addressed" is not the same as "changed", and a resolution table +that blurred the two would be worth less than none. + +--- + ## 2. Findings by severity ### HIGH diff --git a/docs/reviews/README.md b/docs/reviews/README.md new file mode 100644 index 0000000..f4daad2 --- /dev/null +++ b/docs/reviews/README.md @@ -0,0 +1,31 @@ +# Code reviews + +Reports from the multi-agent whole-codebase reviews ("ultrareviews") run against +gpudge. They are published because a library that claims to be well-tested +should be willing to show what an adversarial read of it turned up. + +**Every report here carries a resolution status**, so a reader can tell a fixed +defect from a live one without cross-referencing the changelog. A findings list +without dispositions reads as a list of known, open bugs — the opposite of what +publishing it is for. + +| report | findings | disposition | +|---|---|---| +| [2026-06-13](2026-06-13-gpudge-ultrareview.md) | 23 survived verification, of 28 raw | status table in the report: 9 confirmed defects, 5 fixed in the same PR, 4 filed as issues | +| [2026-06-27](2026-06-27-gpudge-ultrareview.md) | 22 confirmed | all 22 addressed in v0.3.1 (gpudge_arc#59); status table added 2026-08-18, re-verified against the tree at `v0.8.0` | + +Not every review is published as a document. The **2026-08** ultrareview — 41 +findings filed, 39 put through adversarial verifiers on an H100, 3 refuted — is +recorded in `CHANGELOG.md` instead, under `0.8.0`, where each fix sits beside the +behaviour it changed. That is the more useful place for it: those findings were +acted on immediately rather than triaged over weeks, so a separate report and the +release notes would have been the same document twice. + +Two caveats worth stating plainly: + +- The 2026-06-13 table's "Issue filed" rows record the disposition **as of that + date**. Some of those issues have since been closed; the table was not + rewritten, because a review report is a dated record rather than a live + tracker. +- Statuses are verified by reading the tree, not by trusting the changelog. Where + a finding was answered by deciding *not* to change anything, it says so.