From 2bb03d2ca12e39f118e4086650337084caf40360 Mon Sep 17 00:00:00 2001 From: Alexander Dobin Date: Thu, 20 Aug 2026 12:56:38 -0700 Subject: [PATCH] release: v0.9.1 Reads cellstream's public `group_spans()` instead of a private accessor, adds `AUTHORS`, and removes a third-party licence audit that carried no notice obligation -- gpudge redistributes no third-party code, and the wheel contains only the package. Also fixes metadata and messages that had gone stale: `CITATION.cff` named the previous release, so citing this library from its own metadata cited the wrong version; two `pip install gpudge[fast]` hints were unquoted, which zsh treats as a glob and refuses; and the streaming extra's missing-dependency error still told callers to install from a checkout, which stopped being necessary when gpudge reached PyPI. The publishing workflow now asserts that every file carrying the release version agrees with the tag, which is what would have caught the citation metadata. Full notes in CHANGELOG.md. --- .github/workflows/ci.yml | 17 +- .github/workflows/release.yml | 183 +++++++++++++++- .gitignore | 1 + AUTHORS | 83 +++++++ CHANGELOG.md | 176 ++++++++++++++- CITATION.cff | 4 +- CONTRIBUTING.md | 9 +- README.md | 9 +- SECURITY.md | 7 +- docs/THIRD_PARTY_LICENSES.md | 394 ---------------------------------- docs/release-notes/v0.3.0.md | 3 +- environment.yml | 8 +- pyproject.toml | 17 +- src/gpudge/__init__.py | 14 +- src/gpudge/_cell_stream.py | 139 ++++++++---- src/gpudge/_csr_dense.py | 2 +- src/gpudge/_stream_backend.py | 39 +++- tests/test_api.py | 31 +++ tests/test_cell_stream.py | 158 +++++++++++--- tests/test_shard_stream.py | 14 +- 20 files changed, 776 insertions(+), 532 deletions(-) create mode 100644 AUTHORS delete mode 100644 docs/THIRD_PARTY_LICENSES.md diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 25e9507..db21240 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -10,8 +10,10 @@ name: CI # tests/test_cell_stream.py, tests/test_inmem_external_ref_gpu.py) each # carry a module-level `pytest.importorskip("cellstream")`, so all three files # skip entirely without the optional `[streaming]` extra. Between them that -# is 141 cases, 70 of which need no GPU -- and since cellstream is on PyPI -# those 70 are now runnable on a stock runner, which wants its own job. +# is 147 cases, 76 of which need no GPU (measured 2026-08-20; per suite, +# test_shard_stream 32/37, test_cell_stream 44/8, test_inmem_external_ref_gpu +# 0/26) -- and since cellstream is on PyPI those 76 are now runnable on a +# stock runner, which wants its own job. # - The real-data tests (tests/test_real_data.py) skip when the dataset path # is absent (it always is on CI). # So this job goes green on a stock CPU runner with no GPU and without the @@ -71,17 +73,16 @@ jobs: cache-dependency-glob: pyproject.toml # `uv pip install` does pip-style resolution of only the requested - # extras. It used to be mandatory: a fresh `uv sync` resolves EVERY source - # in [tool.uv.sources], and that table carried a private cellstream (then - # shardad) git-SSH entry the no-SSH runner could not clone, even when the - # `streaming` extra wasn't requested. That entry is gone since the rename, - # so `uv sync` would work now too. torch resolves from PyPI here (the cu126 + # extras. It used to be mandatory, back when [tool.uv.sources] carried a + # git-SSH entry a no-SSH runner could not clone and `uv sync` resolved every + # source in that table whether or not the extra was requested. That entry is + # gone, so `uv sync` would work now too. torch resolves from PyPI here (the cu126 # source is skipped), which is fine: CI is CPU-only and GPU tests skip. # `[fast]` (numba) IS installed so the numba CSR kernel — a CPU code path — # is exercised rather than the scipy fallback. # # `[streaming]` is deliberately NOT installed here: this job's purpose is - # to prove gpudge's CORE install needs nothing optional. Covering the 70 + # to prove gpudge's CORE install needs nothing optional. Covering the 76 # non-CUDA cellstream cases is now possible on a stock runner and wants its # own job rather than widening this one. - name: Install dependencies diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 8581e5d..db85e40 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -57,7 +57,7 @@ jobs: set -euo pipefail expected=$(printf '%s\n' \ .gemini .github .gitignore .python-version \ - CHANGELOG.md CITATION.cff CODE_OF_CONDUCT.md CONTRIBUTING.md \ + AUTHORS CHANGELOG.md CITATION.cff CODE_OF_CONDUCT.md CONTRIBUTING.md \ LICENSE PKG-INFO README.md SECURITY.md \ benchmarks docs environment.yml examples pyproject.toml src tests \ | sort) @@ -102,4 +102,185 @@ jobs: echo "::error::tag $GITHUB_REF_NAME does not match built version $built"; exit 1; } echo "tag and built version agree: $built" + # Four files carry the release version independently of pyproject.toml -- + # CITATION.cff, CHANGELOG.md, environment.yml, README.md -- and nothing + # used to check any of them: CITATION.cff silently shipped `0.8.0` INSIDE + # the 0.9.0 release, because "these files move together" lived only in a + # human's head. Asserted here instead. + # + # The two YAML files are PARSED and the two Markdown files are matched as + # whole lines. Both decisions are findings rather than preferences: three + # rounds of review produced file contents that passed a text-matching + # draft of this step while publishing the wrong thing. + # + # grep -F matched a version named in a COMMENT while the real pin + # sat one release behind. + # grep -c counts matching LINES, so an exact heading string buried + # in a sentence of prose read as the heading. + # digits a date checked for SHAPE let `2026-02-31` through, and + # `fromisoformat` alone lets `20260820` through -- it + # canonicalises, and CFF 1.2 wants the dashes. + # ERE interpolating a version into a regex breaks on the `+` + # that PEP 440 permits. + # .strip() made the checks indentation-blind, so a pin de-indented + # out of the `pip:` block (where it installs nothing) and a + # heading indented into a code block both passed. + # startswith a duplicate `"version":` key, which YAML resolves to the + # LAST value, hid behind the first one being right. + # substring counting occurrences of the install pin accepted one + # inside an HTML comment, with none left in the document. + # truthiness an emptied or fully-commented-out YAML file parses to + # None, and a bare `{}` parses to an empty mapping; both are + # falsy, and both skipped their structural check entirely. + # `.*?-->` an HTML comment opener with the closer forgotten hides the + # rest of the document when rendered and stripped nothing. + # + # Every one of those was demonstrated against this step. (Codex review, + # gpudge_arc#149.) + - name: The other files that carry the version agree with it + run: | + set -euo pipefail + uv run --with pyyaml --no-project python - <<'PY' + import glob, re, sys + from datetime import date + from pathlib import Path + + import yaml + + fail = [] + + class Strict(yaml.SafeLoader): + """Rejects duplicate keys, which SafeLoader resolves to the last.""" + + def _no_dupes(loader, node, deep=False): + seen = set() + for k, _ in node.value: + key = loader.construct_object(k, deep=deep) + if key in seen: + raise yaml.YAMLError(f"duplicate key {key!r}") + seen.add(key) + return yaml.SafeLoader.construct_mapping(loader, node, deep) + + Strict.add_constructor( + yaml.resolver.BaseResolver.DEFAULT_MAPPING_TAG, _no_dupes) + + def load(name): + """A mapping, or None with the reason already recorded. + + Everything that is not a mapping is a failure, `None` included: an + emptied or fully-commented-out file parses to None, and letting + that through skipped the whole structural check below. Callers + test `is not None`, not truthiness -- `{}` is a mapping, parses + from a plain `{}`, and is falsy, which skipped the check just the + same. + """ + try: + doc = yaml.load(Path(name).read_text(encoding="utf-8"), Strict) + except yaml.YAMLError as exc: + fail.append(f"{name} is not valid YAML: {exc}") + return None + if not isinstance(doc, dict): + fail.append(f"{name} did not parse to a mapping, got {type(doc).__name__}") + return None + return doc + + def lines(name): + return Path(name).read_text(encoding="utf-8").splitlines() + + sdists = sorted(glob.glob("dist/*.tar.gz")) + if len(sdists) != 1: + print(f"::error::want exactly one dist/*.tar.gz, found {len(sdists)}: {sdists}") + sys.exit(1) + stem = Path(sdists[0]).name.removesuffix(".tar.gz") + if not stem.startswith("gpudge-"): + print(f"::error::cannot read a version out of {sdists[0]}") + sys.exit(1) + v = stem[len("gpudge-"):] + + # CITATION.cff, as a citation consumer sees it. `date-released` is a + # date in CFF, so an unquoted value parses to a date object; a quoted + # one must be the canonical YYYY-MM-DD, which is what CFF 1.2 asks for + # and is stricter than `fromisoformat`, whose 3.11+ form also accepts + # `20260820` and `2026-W34-4` and quietly canonicalises them. + cff, cff_date = load("CITATION.cff"), None + if cff is not None: + if str(cff.get("version")) != v: + fail.append(f"CITATION.cff version is {cff.get('version')!r}, want {v}") + raw = cff.get("date-released") + if isinstance(raw, date): + cff_date = raw.isoformat() + else: + try: + parsed = date.fromisoformat(str(raw)).isoformat() + except (ValueError, TypeError) as exc: + fail.append(f"CITATION.cff date-released {raw!r} is not a real date: {exc}") + else: + if parsed != str(raw): + fail.append(f"CITATION.cff date-released {raw!r} is not written as {parsed}") + else: + cff_date = parsed + + # environment.yml: the pin has to be in the PIP list. Checked + # structurally because a pin de-indented into the conda list is still + # a well-formed list entry -- it just never installs gpudge. + env = load("environment.yml") + want = f"gpudge[fast]=={v}" + if env is not None: + pips = [item["pip"] for item in env.get("dependencies") or [] + if isinstance(item, dict) and "pip" in item] + if len(pips) != 1: + fail.append(f"environment.yml has {len(pips)} pip: blocks, want exactly 1") + elif [r for r in pips[0] if r == want] != [want]: + fail.append(f"environment.yml pip: block does not carry exactly one {want!r}: {pips[0]}") + + # And no OTHER version pinned anywhere in either file, including in a + # comment a parser cannot see. Every occurrence is extracted and + # compared: deciding per LINE pardoned a stale pin sitting in a + # trailing comment beside the right one, and a substring test let + # `==0.9.10` satisfy a check for `==0.9.1`. + for name in ("environment.yml", "README.md"): + found = re.findall(r"gpudge\[fast\]==([0-9A-Za-z._!+-]*)", + Path(name).read_text(encoding="utf-8")) + other = sorted({f for f in found if f != v}) + if other: + fail.append(f"{name} pins gpudge[fast] at {other} as well as {v}") + + # The changelog heading, whole-line and UNINDENTED: `.strip()` accepts + # four leading spaces, which CommonMark renders as a code block, so the + # release's sections would stay under Unreleased. + if cff_date: + want_h = f"## [{v}] \u2014 {cff_date}" + n = sum(1 for ln in lines("CHANGELOG.md") if ln == want_h) + if n != 1: + fail.append(f"want exactly one unindented CHANGELOG heading {want_h!r}, found {n}") + + # README's install pin is the one version in this tree NOT edited by + # hand: the mirror build substitutes it from a version argument typed + # on the command line, so it can name a different tag while every + # hand-edited file agrees -- and it is the line readers copy. HTML + # comments are stripped first: the pin has to be in the document a + # reader sees, and commenting the install block out (delimiters on + # their own lines, pin untouched) satisfied a plain line match. An + # opener runs to `-->` OR to end of file, because that is what + # CommonMark does with an unclosed one -- and `.*?-->` strips nothing + # at all when the closer was forgotten. + text = re.sub(r"|\Z)", "", + Path("README.md").read_text(encoding="utf-8"), flags=re.S) + want_pin = f'"gpudge[fast] @ git+https://github.com/ArcInstitute/gpudge.git@v{v}"' + n = sum(1 for ln in text.splitlines() if ln.strip() == want_pin) + total = text.count("ArcInstitute/gpudge.git@") + if n != 1 or total != 1: + fail.append( + f"README.md: want exactly one uncommented line {want_pin} and no " + f"other 'ArcInstitute/gpudge.git@'; found {n} and {total}. The " + f"mirror build's version argument disagrees with the tag") + + for f in fail: + print(f"::error::{f}") + if fail: + sys.exit(1) + print(f"version parity ok: {v}, dated {cff_date}, in CITATION.cff, " + f"CHANGELOG.md, environment.yml and README.md") + PY + - uses: pypa/gh-action-pypi-publish@release/v1 diff --git a/.gitignore b/.gitignore index 9c413cc..f9da33c 100644 --- a/.gitignore +++ b/.gitignore @@ -11,3 +11,4 @@ htmlcov/ uv.lock + diff --git a/AUTHORS b/AUTHORS new file mode 100644 index 0000000..e7a869d --- /dev/null +++ b/AUTHORS @@ -0,0 +1,83 @@ +gpudge — authors and contributors +================================= + +Maintainer +---------- + +Alexander Dobin (@alexdobin) + Author and maintainer. + + +Inspiration and prior art +------------------------- + +Abhinav Adduri (@abhinadduri) + gpudge's direct inspiration is the GPU fork of pdex — [`abhinadduri/pdex`, branch + `gpu`](https://github.com/abhinadduri/pdex/tree/gpu), where + [`138335ae`](https://github.com/abhinadduri/pdex/commit/138335ae97b8) "Add CUDA backend for + ref-mode MWU" added `src/pdex/_torch_mwu.py` on 2026-05-19. gpudge's own design document is + dated two days later. That branch put reference-mode Mann–Whitney on CUDA — sorting the + fixed reference pool once per gene chunk and reusing it across every target group — and + added `mwu_backend={"auto","cpu","torch"}` and a `gpu_gene_chunk_size` knob to + `pdex.pdex(...)` to drive it. As of 2026-08-20 those interfaces are the fork's alone: + upstream pdex has no `_torch_mwu.py`, and no `mwu_backend`, `gpu_gene_chunk_size` or + `default_gene_chunk_size`. + + gpudge took two concrete things from that branch as well as the idea. The name of the + `gpu_gene_chunk_size` parameter is that branch's, and so is the shape of gpudge's gene-chunk + sizer: a working-set estimate of ~24 bytes per reference cell per gene, capped at 16 GB and + at a fraction of free device memory, floored and rounded down to a multiple of 64. gpudge + takes 20% of free memory where the fork takes 18%, and additionally accounts for the + per-chunk accumulators the reference path holds resident — but it is the same heuristic, + adopted rather than reinvented. The attribution comment at that call site said plain "pdex", + which read as upstream; it now names the fork. + + Thanks also for a number of useful suggestions along the way. + +gpudge exists to replace the parts of [pdex](https://github.com/ArcInstitute/pdex) that one +production DE stage used, and pdex is the CPU baseline behind gpudge's parity and speedup +claims. It is prior art in the strong sense: those numbers only mean anything because there +was a correct, well-tested CPU implementation to measure against. (What the committed +real-data check asserts is a bounded agreement, not bit-identity — see the tolerances in +`tests/test_real_data.py`.) + + +Production evaluation +--------------------- + +Kevin Feng (@kfenggg) + Evaluated gpudge in a production single-cell QC pipeline and adopted it as that pipeline's + default differential-expression engine, reporting close log2-fold-change agreement with CPU + pdex on real data at production scale, and a large reduction in wall time against the CPU + path it replaced. An evaluation run by someone other than this library's author, on data + and a workload chosen without reference to gpudge's own test suite, is a different kind of + evidence from the in-repo benchmarks — complementary to them, not a replacement. + + Establishing that agreement meant working out the correspondence between the two engines, + which is his. On log1p input, pdex's default target and reference means are, in exact + arithmetic, `expm1` of gpudge's ARITHMETIC means — and log2FC then has to be recomputed + from those transformed means, because gpudge's arithmetic-mode `log2_fold_change` is a + ratio of the untransformed means and is not on pdex's scale. Arithmetic is the mode to use + here because of a real gpudge constraint: gpudge refuses `mean_calc="geometric"` together + with `ALL_OTHERS`, the rest-mean being computed by sum-subtraction, which is implemented + for the arithmetic case only. + + +AI tooling +---------- + +Much of this codebase was written with Claude Code (Anthropic), and OpenAI Codex was used inside +those same sessions to review the work before it reached human review — a deliberate +cross-provider check, on the theory that a second model from a second vendor is harder to talk +into agreement than the one that wrote the code. Gemini Code Assist and GitHub Copilot review +every pull request on top of that. The development history carries `Co-Authored-By` trailers +naming the specific Claude models. + +Design decisions, rulings and acceptance of every change are the maintainer's. + + +Copyright +--------- + +Copyright (c) 2026 Arc Research Institute. +MIT — see LICENSE. diff --git a/CHANGELOG.md b/CHANGELOG.md index d424ca3..cbe3ea3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ All notable changes to gpudge are documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +> Entries before 0.9.0 call the optional streaming dependency **`shardad`**. That +> is the same package now published as +> [`cellstream`](https://github.com/ArcInstitute/cellstream); it was renamed when +> it went to PyPI in 0.9.0. Those entries keep the name they were written with +> rather than being rewritten. + > **A note on issue and PR numbers.** References such as `#124`, `gpudge_arc#115` > and `shardad #246` throughout this file are **provenance, not links**: they > identify entries in issue trackers that are not public, and will not resolve @@ -13,6 +19,167 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.9.1] — 2026-08-20 + +### Changed + +- **`de(archive=…, layout='cell')` no longer reaches into cellstream's private + `CellStore._load_groups()`.** It reads the public `group_spans()` that + cellstream 0.8.0 added for exactly this (`shardad #248`), plus the manifest's + `reference` — which loading the spans validates against groups.json's own copy, + so the two cannot disagree. Behaviour is unchanged: same group table, same + guards, same messages apart from the one that named the accessor. Verified on + real archives in both reference modes that `group_spans()` returns the private + records element-for-element and that the manifest's `reference` matches + groups.json's. (`gpudge_arc#139`) + + gpudge still **rejects duplicate group labels**, which cellstream explicitly + permits — under the default sort order, distinct raw obs values that are equal + once stringified produce several records carrying one label, which is why + `group_spans()` returns a list rather than a mapping. For a *target* label that + is a wrong answer, and not by collapsing the spans: the target list keeps both + entries while the label→index map resolves both to the last, so one result row + is never written and the other is overwritten span by span. For a *reference* + label the rejection is stricter than necessary — duplicates are handled + correctly downstream — and is left in place deliberately, that path having no + fixture; loosening it is a follow-up, not part of this change. + + `reference_row_count()` is deliberately **not** used. The label split has to + happen regardless, and deriving the reference block's end from the same table + keeps the two consistent by construction rather than by agreement. + +### Added + +- **`AUTHORS`**, a curated credit record. `git shortlog` describes this project + badly — squash merges collapse each pull request into one author entry, a shape + that cannot represent a review, a report or prior art, and the contribution + that matters most here arrived as exactly those rather than as patches. It + credits the GPU fork of pdex (`abhinadduri/pdex@gpu`) as + the work that demonstrated the idea gpudge is built on, and from which gpudge + adopted both the `gpu_gene_chunk_size` parameter name and the shape of its + automatic gene-chunk sizer. The attribution comment at that call site said + "pdex", which read as upstream; upstream pdex has no such function, and the + comment now names the fork. + + It also records a production evaluation by someone other than this library's + author: gpudge was adopted as the default DE engine in a production single-cell + QC pipeline, with close log2-fold-change agreement against CPU pdex reported on + real data at production scale. The entry states the correspondence that makes + the two comparable — on log1p input pdex's means are `expm1` of gpudge's + *arithmetic* means, and log2FC must be recomputed from those, because + arithmetic-mode `log2_fold_change` is a ratio of the untransformed means. + + A `.mailmap` accompanies it so identity counts over the private history are + correct. It is development-only and is not part of a release. + +### Removed + +- **`docs/THIRD_PARTY_LICENSES.md`.** It was a dated audit of one resolved + dependency environment, and gpudge **redistributes** no third-party code — the + wheel carries only the `gpudge/` package, which the release workflow asserts on + every publish — so there was no notice obligation for the file to discharge. + What it did instead was restate, less authoritatively, what each dependency + declares in its own metadata. And not every package declares anything: + `pyfastpfor`, a transitive dependency of the streaming extra's `[cell]` variant, + publishes an sdist with no licence file at all, which is the kind of row a + point-in-time table records once and then cannot keep true. It had also gone materially + stale: its rows described the streaming dependency under a name and a version + that no longer exist, and refreshing them meant reproducing a 93-package + point-in-time resolve rather than editing text. `SECURITY.md` now states the + actual situation instead of linking to it. + + The mirror pipeline loses two things with it: a fixup rule that de-sited the + audit's regeneration recipe, and — more usefully — the last exception in + `leak_check.py`'s SSH-URL check, which was scoped to that one file. A GitHub + SSH URL anywhere in the published tree is now a leak with no exception at all — + as this very entry discovered, having first been written with a literal one in + it. + +### Fixed + +- **`de(archive=…)`'s missing-dependency error gave dated install advice.** It + told the caller to run `pip install '.[streaming]'` *from a checkout*, because + the bare PyPI name still resolved to the reservation stub. gpudge is on PyPI + as of 0.9.0, so the message now leads with `pip install 'gpudge[streaming]'` + and keeps the checkout form as the alternative. Deferred from the 0.9.0 + release review rather than re-cutting the tag. + +- **Two install hints were unquoted.** `pip install gpudge[fast]`, as printed by + the no-numba warning and by `_csr_dense`'s module docstring, is a glob pattern + to zsh — the default shell on macOS — which fails with `no matches found` + rather than installing anything. Both now quote the argument. + +- **The pre-release GPU gate now reports what it actually measured.** Its first + run ever to reach the test suite passed every case on an unpinned `[dev,fast]` + resolve on an L4 and still reported failure, on two faults in the gate's own + harness rather than anything in the library: a correct new skip its verdict + allow-list had not been told about, and one artifact copy that failed + transiently with the reason discarded. Reviewing that fix surfaced two further + faults that had never been observed — the retrieval loop leaked a non-zero + status, and its error-reporting path could end the run at SIGPIPE before the + verdict or the diagnostics were written. All four are fixed and the loop now + has unit tests driven against the gate implementation itself rather than a + copy of it. + + This is the machinery that decides whether a release may happen; the gate + implementation itself is deliberately not part of any distribution. + (`gpudge_arc#147`) + +- **The streaming extra's missing-dependency error no longer names a package that + no longer exists.** It read "installs cellstream (>=0.9.0, the former + shardad)"; the parenthetical helped nobody who had hit the error, and named a + repository the reader cannot see. The same aside appeared across the prose — the + install documentation, two packaging comments and a CI comment each explained a + pre-rename `[tool.uv.sources]` git-SSH entry that broke `uv sync`, a failure + impossible on any supported version now that the floor is cellstream 0.9.0 and + that entry is gone. All removed. + +- **`CITATION.cff` said `0.8.0` while 0.9.0 was being published**, so anyone who + cited this library from its own metadata cited the wrong release. Corrected, + and the publishing workflow now asserts it rather than trusting that "these + files move together": four files carry the release version independently of + `pyproject.toml`, and nothing checked any of them. + + The two YAML files are **parsed** — with a loader that rejects duplicate keys, + and a structural search for the pin inside the `pip:` block — and the two + Markdown files are matched as whole lines, the changelog heading unindented + and the README pin after HTML comments are stripped out. Those decisions are + findings rather than preferences. Three rounds of review produced file + contents that passed a text-matching draft of this check while publishing the + wrong thing: + + - a version named in a *comment* while the real pin sat one release behind, + and a stale pin in a trailing comment beside the correct one; + - the install pin inside an *HTML comment*, with none left in the document a + reader sees — and then the same pin under an *unclosed* comment opener, + which hides the rest of the page when rendered while `` strips + nothing at all; + - an exact heading string buried inside a sentence of prose, because `grep -c` + counts matching *lines*; + - a heading indented four spaces, which renders as a code block, so the + release's sections stay under `Unreleased`; + - a date checked for digit shape, so `2026-02-31` passed — and, once parsing + replaced the shape check, `20260820`, because `date.fromisoformat` accepts + it and canonicalises it while CFF wants the dashes; + - a duplicate `"version":` key, which YAML resolves to the *last* value, + hidden behind a first one that was right; + - the pin de-indented out of the `pip:` block, where it is still a well-formed + list entry and installs nothing; + - an emptied, fully-commented-out file, which parses to `None`, and a bare + `{}`, which parses to an empty mapping — both falsy, both skipping their + structural check entirely, and the second introduced by the fix for the + first; + - `==0.9.10` satisfying a check for `==0.9.1`, twice, in two different checks. + + Interpolating a version into a regular expression fails on a different axis — + the `+` that PEP 440 permits. Every one of those was demonstrated against this + check rather than argued about, and each is now a case that fails. + + `README.md` is checked because it holds the install pin readers copy, and in + the published tree that version is the one thing *not* hand-edited: the mirror + build substitutes it from a version argument typed on the command line, so it + can name a different tag while every hand-edited file agrees. (`gpudge_arc#149`) + ## [0.9.0] — 2026-08-19 ### Changed @@ -905,8 +1072,8 @@ Distribution remains **Arc-internal** — install from the `v0.3.0` git tag - First CI workflow (`.github/workflows/ci.yml`): `ruff` + CPU `pytest` on every push / PR (ubuntu-latest, Python 3.12). Installs via - `uv pip install -e ".[dev,fast]"` so the private shardad git-SSH source is - never resolved on the runner; GPU (`needs_cuda`), shardad streaming + `uv pip install -e ".[dev,fast]"` so the streaming dependency's git-SSH source + is never resolved on the runner; GPU (`needs_cuda`), streaming (`importorskip`), and real-data tests auto-skip. The `ruff` gate is blocking; the repo was brought to zero lint errors (47 fixed). Green at 121 passed / 53 skipped. (#53) @@ -1089,11 +1256,12 @@ ULP of the CPU baseline on every assertable column. until shardad v0.3 lands an on-GPU read path. -[Unreleased]: https://github.com/ArcInstitute/gpudge/compare/v0.9.0...HEAD +[Unreleased]: https://github.com/ArcInstitute/gpudge/compare/v0.9.1...HEAD +[0.9.1]: https://github.com/ArcInstitute/gpudge/compare/v0.9.0...v0.9.1 [0.9.0]: https://github.com/ArcInstitute/gpudge/compare/v0.8.0...v0.9.0 [0.8.0]: https://github.com/ArcInstitute/gpudge/compare/v0.7.0...v0.8.0 [0.7.0]: https://github.com/ArcInstitute/gpudge/compare/v0.3.1...v0.7.0 diff --git a/CITATION.cff b/CITATION.cff index 0e21239..4197678 100644 --- a/CITATION.cff +++ b/CITATION.cff @@ -13,8 +13,8 @@ authors: - family-names: Dobin given-names: Alexander affiliation: "Arc Institute" -version: 0.8.0 -date-released: "2026-08-18" +version: 0.9.1 +date-released: "2026-08-20" license: MIT repository-code: "https://github.com/ArcInstitute/gpudge" keywords: diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 09cc362..1cb25e3 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -28,10 +28,7 @@ uv venv && uv pip install --torch-backend=cu126 -e ".[dev,fast]" `uv pip install` resolves only the extras you name, and skips `[tool.uv.sources]` — which is why `--torch-backend=cu126` is passed explicitly. -`uv sync` also works now; it did not before the `shardad` → `cellstream` rename, -because `[tool.uv.sources]` then carried a private git-SSH entry that `uv sync` -resolved even when the `streaming` extra was not requested, failing with -`Permission denied (publickey)` for anyone without a key. `[fast]` adds numba, +`uv sync` also works. `[fast]` adds numba, which the fast CSR kernel needs; `[dev]` adds pytest, ruff, scanpy and pyyaml. ## Running the tests @@ -93,6 +90,10 @@ 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. +Add yourself to `AUTHORS` in the same PR. That applies to a report, a +measurement or a review that changed the code or a documented rule, not only to +a patch — `AUTHORS` says so, and does not rank the two. + ## Reporting a bug Include the gpudge version, the Python and torch versions, the GPU, and the diff --git a/README.md b/README.md index 3e00ed5..23eedeb 100644 --- a/README.md +++ b/README.md @@ -122,7 +122,7 @@ not been released yet: ```bash pip install --extra-index-url https://download.pytorch.org/whl/cu126 \ - "gpudge[fast] @ git+https://github.com/ArcInstitute/gpudge.git@v0.9.0" + "gpudge[fast] @ git+https://github.com/ArcInstitute/gpudge.git@v0.9.1" ``` Extras: @@ -172,11 +172,8 @@ uv venv && uv pip install --torch-backend=cu126 -e ".[dev,fast]" deliberately — it is CPU-only. `uv sync --extra streaming` also works and pulls the cu126 torch build from its -pin with no manual `--extra-index-url`. (Before the `shardad` → `cellstream` -rename this was not true: `[tool.uv.sources]` carried a private git-SSH entry -that `uv sync` resolved even when the `streaming` extra was not requested, so it -failed with `Permission denied (publickey)` for anyone without a key. That entry -is gone — cellstream comes from PyPI.) +pin with no manual `--extra-index-url`. cellstream resolves from PyPI like any +other dependency, so no repository access is involved. ### conda / mamba diff --git a/SECURITY.md b/SECURITY.md index c88c512..6095846 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -44,7 +44,10 @@ 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 +gpudge **redistributes** no third-party code: the wheel carries only the +`gpudge/` package and its `dist-info`, which the release workflow asserts on +every publish. Dependencies are resolved at install time from the set declared +in `pyproject.toml`, and each carries its own licence — in its own metadata +where it declares one at all, which not every package on PyPI does. A vulnerability in a dependency is best reported upstream first; tell us too, so the floor can be raised here. diff --git a/docs/THIRD_PARTY_LICENSES.md b/docs/THIRD_PARTY_LICENSES.md deleted file mode 100644 index f6cce84..0000000 --- a/docs/THIRD_PARTY_LICENSES.md +++ /dev/null @@ -1,394 +0,0 @@ -# Third-party licenses - -License audit of the packages `gpudge` depends on. **`gpudge` itself is -licensed `MIT`** (see `LICENSE` and `pyproject.toml`). - -> **Audited 2026-08-02** against a **recorded point-in-time clean environment -> resolved from the `v0.7.0` tree** (CPython 3.12.11, uv 0.7.12, the `cu126` -> PyTorch build, `dev` + `fast` + `streaming-gpu` extras; 93 distributions). The -> recipe below is the method — use it for the next audit; it is not guaranteed to -> reproduce these exact versions, and is not meant to. -> -> **These versions are a point-in-time resolve, not committed project state.** -> gpudge declares floors (`anndata>=0.12`, `torch>=2.5`, …) against a mutable -> index, so only `shardad` — tag-pinned via `[tool.uv.sources]` — is fixed by the -> repository; everything else is what the resolver returned on 2026-08-02. The -> `uv.lock` generated by that run — `$AUDIT/tree/uv.lock` in the recipe below — -> has SHA-256 -> `dd5a4aa115eb0a9ddaca8a1b83024745452cd13328e2258959f8f04fa15e01d8`. Re-audit -> after dependency bumps. -> -> **Not** the developer `.venv`: that environment installs `shardad` from a local -> working checkout (`direct_url.json` → `file:///…/shardad`, version -> `0.7.2.dev24+…`), which is not evidence for any released version, and it carries -> unrelated editable installs that are outside gpudge's dependency closure. -> -> **Package-level** licenses were read from installed distribution metadata, not -> assumed. Where a package carries no **structured license identifier** — no -> `License-Expression`, no legacy `License` value, no `License ::` classifier — -> the `LICENSE` file shipped in the distribution (referenced by `License-File:`) -> is cited instead and called out as such. Where the *only* identifier is the -> coarse legacy `BSD License` classifier (SciPy, numba), the precise SPDX -> identifier is resolved from the shipped license text and marked "from -> `LICENSE`" — metadata alone cannot distinguish 2-clause from 3-clause. Where a -> package has **neither** — no identifier *nor* any license file in any published -> artifact (`pyfastpfor`) — the grant is taken from the license headers carried in -> that release's own source, cited by file, and flagged as an artifact-level gap -> rather than recorded as a clean license. Claims about *bundled components* whose -> license information the wheel does not itself carry (notably hdf5plugin's HDF5 -> filters — the wheel ships the eleven filter `.so` binaries, but nothing stating -> their terms) are attributed to upstream and flagged as such. -> -> Supersedes the 2026-06-09 audit, which predated the MIT relicense (#60) and -> therefore described gpudge as BSD-3-Clause throughout. -> -> Supersedes the 2026-07-25 audit, which was taken against the developer `.venv` -> and therefore recorded a `shardad` version predating the `[cell]` extra entirely, -> and omitted `pyfastpfor` and `pybind11` (`gpudge_arc#120`). -> -> ⚠️ **The `shardad` row below is stale as of `gpudge_arc#139`.** That package has -> been renamed **`cellstream`**, is published on PyPI, and the `streaming` extras -> now pin `cellstream>=0.9.0` — so the private-repo access this recipe -> describes is no longer needed, and `[tool.uv.sources]` no longer carries a -> git-SSH entry for it. Everything below is left exactly as it was measured on -> the date in the banner: this file is a dated record, and the correct fix is to -> re-run the regeneration recipe against the new dependency rather than to edit -> the recorded rows by hand. - -## How to regenerate - -First build the environment. Resolve it from a clean checkout of the tagged tree, -never from a developer `.venv` — this is the step whose absence made the previous -audit ambiguous about which environment it described. - -**This recipe needs access to the private `ArcInstitute/shardad` repository**: -`--extra streaming-gpu` resolves shardad over SSH, so the full 93-package audit -is maintainer-only. Everything the core install pulls can be audited without it -by dropping that extra. - -```bash -set -euo pipefail # without errexit a failed mkdir/tar/cd falls through to - # `uv sync` in whatever directory you started in - -# Put $AUDIT on a large scratch filesystem, never /home: the resolved -# environment is ~8 GB. Substitute your own site's path -- and check that you -# can create a directory there: a shared scratch root is often root-owned 0755, -# in which case $SCRATCH/$USER is NOT creatable and a per-group path is. -SCRATCH_ROOT=/path/to/scratch -AUDIT=$SCRATCH_ROOT/gpudge-license-audit-$(date +%Y%m%d-%H%M%S) -# A fresh, unique directory: `mkdir -p` over a populated path silently overlays -# it, which is exactly the contamination this recipe exists to avoid. -mkdir "$AUDIT" "$AUDIT/tree" -# `-C /path/to/gpudge` is deliberate, not redundant: nothing above this line -# puts you in the gpudge checkout, so a bare `git archive` would depend on an -# unstated cwd. `tar -xf -` names stdin explicitly, which BSD/macOS tar wants. -git -C /path/to/gpudge archive v0.7.0 | tar -xf - -C "$AUDIT/tree" -cd "$AUDIT/tree" -uv sync --python 3.12 --extra dev --extra fast --extra streaming-gpu -``` - -`[tool.uv.sources]` pins shardad to its `v0.7.1` tag, so that sync resolves -`shardad==0.7.1` from -`git+ssh://git@github.com/ArcInstitute/shardad.git@fbba761` — the version this -audit describes, and the one version here the repository actually fixes. Confirm -it before reading anything else: - -```bash -.venv/bin/python -c "import importlib.metadata as M; print(M.version('shardad'))" -# 0.7.1 -``` - -Then dump the closure: - -```bash -.venv/bin/python - <<'PY' # the $AUDIT/tree/.venv from above -import importlib.metadata as M -for d in sorted(M.distributions(), key=lambda d: (d.metadata.get("Name") or "").lower()): - md = d.metadata - name = md.get("Name") - if not name: - continue - lic = (md.get("License-Expression") - or "; ".join(dict.fromkeys( - v.split("::")[-1].strip() for k, v in md.items() - if k == "Classifier" and v.startswith("License ::"))) - or (md.get("License") or "").strip().splitlines()[0:1] or ["(none declared)"]) - print(f"{name}\t{d.version}\t{lic if isinstance(lic, str) else lic[0]}") -PY -``` - -Four caveats when reading that output: - -- It reads only the **identifier fields**. Several packages below leave those - empty yet ship a real `LICENSE` file; check `/licenses/` before - concluding anything is unlicensed. -- It prefers a `License ::` classifier over the legacy `License` value, so for - SciPy it prints just `BSD License` and **never reaches** that package's - ~47 kB `License` value — which is where the bundled binary components and - their SPDX identifiers are declared. Inspect that field separately; see the - SciPy subsection below. -- It reports **whatever is installed**. In the clean environment above that *is* - gpudge's dependency closure for those extras (93 distributions, `gpudge` itself - included). Run it against a developer `.venv` and it is not: that environment - also carries unrelated editable installs, which is one more reason the recipe - above starts from `git archive`. -- A `License ::` classifier can be **present and still name nothing**. `umap-learn` - declares exactly `License :: OSI Approved`; because the snippet keeps only the - segment after the final `::`, that prints as a bare `OSI Approved`. Treat any - output that does not name a license family as "not yet audited". - -## Declared dependencies (`pyproject.toml`) - -| Package | Version | Extra | License | Notes | -|---|---|---|---|---| -| anndata | 0.13.2 | core | BSD-3-Clause | permissive | -| hdf5plugin | 7.0.0 | core | MIT (wrapper only) | **no structured identifier**; taken from the shipped `LICENSE`, which grants MIT for "the hdf5plugin code itself". The bundled HDF5 filters carry **separate** terms (BSD / Zlib / etc.), and neither their notices nor upstream's `doc/information.rst` are included in the installed wheel — see flag 3 | -| numpy | 2.4.6 | core | BSD-3-Clause AND 0BSD AND MIT AND Zlib AND CC0-1.0 | the vendored components are the non-BSD terms — all permissive | -| polars | 1.43.2 | core | MIT | permissive; polars 1.43 splits its compiled runtime into an unconditional `polars-runtime-32==1.43.2` requirement (also MIT) | -| pyarrow | 25.0.0 | core | Apache-2.0 | permissive | -| scipy | 1.18.0 | core | BSD-3-Clause (project) — from `LICENSE.txt` | metadata carries only the coarse `BSD License` classifier; the shipped `LICENSE.txt` is the three-clause text, including the non-endorsement clause. **The binary wheel bundles non-permissive shared libraries** — see below | -| torch | 2.13.0+cu126 | core | BSD-3-Clause (project); wheel declares `Apache-2.0 AND Apache-2.0 WITH LLVM-exception AND BSD-2-Clause AND BSD-3-Clause AND BSL-1.0 AND MIT` | **new since the 2026-07-25 audit:** 2.13 ships a real `License-Expression` covering its vendored third-party code, plus 97 `licenses/third_party/` files alongside its own `LICENSE` (still the PyTorch BSD-3-Clause text). The compound expression is *torch's own* vendored set — it does **not** cover the `cu126` build's **proprietary** NVIDIA CUDA libraries, which remain outside the wheel; see below | -| pytest | 9.1.1 | dev | MIT | permissive | -| ruff | 0.15.22 | dev | MIT | permissive | -| scanpy | 1.12.3 | dev | BSD-3-Clause | permissive | -| PyYAML | 6.0.3 | dev | MIT | permissive | -| numba | 0.66.0 | fast | BSD-2-Clause — from `LICENSE` | metadata carries only the coarse `BSD License` classifier; the shipped `LICENSE` has two clauses and no non-endorsement clause, hence 2- rather than 3-clause | -| shardad | 0.7.1 | streaming, streaming-gpu | MIT | Arc Institute private package, pinned by `[tool.uv.sources]` to its `v0.7.1` tag. `streaming` takes `shardad[cell]`, `streaming-gpu` takes `shardad[cell,gpu]` (`pyproject.toml:51,56`). **No structured identifier**; taken from the `LICENSE` shipped in the wheel (MIT, © 2026 Arc Research Institute). The `[cell]` extra pulls `pyfastpfor` — see below | - -### SciPy's bundled shared libraries - -The manylinux wheel vendors compiler-runtime and BLAS libraries into -`scipy.libs/`. These are **declared by the wheel**, in the legacy `License` -metadata value, each with an SPDX identifier and the file it covers: - -| Bundled component | Files | License | -|---|---|---| -| OpenBLAS | `libscipy_openblas*.so` | BSD-3-Clause | -| LAPACK | `libscipy_openblas*.so` | BSD-3-Clause-Open-MPI | -| GCC runtime library (`libgfortran`) | `libgfortran*.so` | GPL-3.0-or-later WITH GCC-exception-3.1 | -| libquadmath | `libquadmath*.so` | LGPL-2.1-or-later | - -(The wheel's CycloneDX SBOM, `/sboms/auditwheel.cdx.json`, lists -`libgfortran` and `libquadmath` as components but carries **no** license -entries for them, and does not inventory `libscipy_openblas` at all. The legacy -`License` value is the authoritative source here, not the SBOM.) - -This matters only for **redistribution** of an assembled environment, not for -using gpudge. Two notes on the copyleft entries: - -- **GCC-exception-3.1** permits Target Code that combines the runtime library - with Independent Modules to be conveyed under terms of your choice, provided - the Target Code came from an Eligible Compilation Process. That is what stops - GPL copyleft reaching the qualifying combined work — it does **not** make - `libgfortran` itself permissive; that library remains GPL-covered with its own - redistribution obligations. -- **`libquadmath` is LGPL-2.1-or-later** with no such exception, so it carries - the usual LGPL notice, source-availability and relinking obligations on - redistribution. - -## Transitive dependencies requiring attention - -### The `[cell]` extra: `pyfastpfor` and `pybind11` - -New since the 2026-07-25 audit. Both arrive through the `streaming` extras. Only the -first edge of the chain is visible in gpudge's own `pyproject.toml` (lines 51, 56); -the rest is not, which is why neither package appeared in earlier audits: - -``` -gpudge[streaming] -> shardad[cell]>=0.7.1 (pyproject.toml:51) -gpudge[streaming-gpu] -> shardad[cell,gpu]>=0.7.1 (pyproject.toml:56) -shardad[cell] -> pyfastpfor>=1.4 (shardad's `extra == "cell"`) -pyfastpfor -> pybind11>=2.4 (unconditional Requires-Dist) -``` - -`pyfastpfor` is not optional at install time: shardad requires it with no condition -beyond `extra == "cell"`, and both of gpudge's streaming extras select that extra, -so every `streaming` install pulls it in. At runtime shardad imports it -lazily, and only for `codec='pfordelta'` cell archives — `codec='zstd'` opens -without it — but `pfordelta` is what every production `.csad` uses. - -| Package | Version | License | Notes | -|---|---|---|---| -| pyfastpfor | 1.4.0 | Apache-2.0 **per the release's own source headers** — no license file in any published artifact | see flag 6 | -| pybind11 | 3.0.4 | BSD-3-Clause | permissive; declares a real `License-Expression` **and** ships `LICENSE` (© 2016 Wenzel Jakob). No gap | - -**`pyfastpfor` ships no license file in any artifact.** Being precise about *which* -artifact matters here: - -- **PyPI publishes only an sdist** for 1.4.0 — `pyfastpfor-1.4.0.tar.gz`, uploaded - 2023-10-14, with an empty `license` field and an empty `classifiers` array. There - is **no published wheel**; the `.dist-info` seen in an installed environment - belongs to a wheel the resolver **built locally** from that sdist. -- **The sdist contains no license file.** All 75 entries scanned: `MANIFEST.in`, - `PKG-INFO`, `fastpfor/`, `pyfastpfor.cc`, `pyfastpfor.egg-info`, - `requirements.txt`, `setup.cfg`, `setup.py` — nothing matching - `licen|notice|copying`. -- **The locally built wheel loses even the in-source notices.** Seven files: - - ``` - pyfastpfor-1.4.0.dist-info/{INSTALLER,METADATA,RECORD,REQUESTED,WHEEL,top_level.txt} - pyfastpfor.cpython-312-x86_64-linux-gnu.so (17 MB) - ``` - - No `License-Expression`, no legacy `License`, no `License ::` classifier, no - `License-File`, and no `LICENSE`/`COPYING`/`NOTICE`. `strings` over the `.so` - matches no occurrence of `apache`, `copyright`, or `licen[sc]e` (a pattern - deliberately spanning both spellings) either — the - compiled artifact carries the grant nowhere at all. - -**The grant itself is not in doubt, and the citation is the release's own source** — -not GitHub's current license detector, whose repository-level `LICENSE` is not part -of the 1.4.0 sdist: - -- `pyfastpfor.cc`, header comment: *"This code is released under the Apache License - Version 2.0 http://www.apache.org/licenses/."* -- **51 of the 58** files in the bundled `fastpfor/` tree carry an `Apache License` - notice. Upstream `lemire/FastPFor` states *"This code is licensed under Apache - License, Version 2.0 (ASL2.0)"*. - -So the terms are Apache-2.0 and permissive. What is missing is artifact-level: the -license copy that **Apache-2.0 §4(a)** obliges a redistributor to give recipients. -§4(d)'s attribution-`NOTICE` duty is **conditional on the original work carrying a -`NOTICE` file**, and neither upstream repository has one — `searchivarius/PyFastPFor` -ships `LICENSE` + `README.md`, `lemire/FastPFor` ships `LICENSE` + `AUTHORS`. So -§4(a) alone is unmet. - -### NVIDIA CUDA runtime - -The `cu126` PyTorch build (plus `streaming-gpu`) pulls these in. The set is -**mostly proprietary** NVIDIA software; the open-source exceptions are marked. -Installation as a Torch runtime dependency does not by itself establish that -gpudge exercises every library listed. - -Package names and license strings below are exactly as installed and declared — -the core CUDA set that arrives with Torch uses **four** different spellings for -what is effectively the same proprietary grant, and `nvidia-nvcomp-cu12` (via -`streaming-gpu`) names its SDK agreement separately, for five proprietary strings -in all. They are grouped by the string each one actually carries rather than -lumped together. - -| Package(s) | Declared license | -|---|---| -| `nvidia-cublas-cu12`, `nvidia-cuda-cupti-cu12`, `nvidia-cuda-nvrtc-cu12`, `nvidia-cuda-runtime-cu12`, `nvidia-cudnn-cu12`, `nvidia-cufft-cu12`, `nvidia-cufile-cu12`, `nvidia-curand-cu12`, `nvidia-cusolver-cu12`, `nvidia-cusparse-cu12`, `nvidia-nvjitlink-cu12` | `Other/Proprietary License` (NVIDIA EULA) | -| `nvidia-cusparselt-cu12` | `NVIDIA Proprietary Software` | -| `nvidia-nccl-cu12`, `nvidia-nvshmem-cu12` | `LicenseRef-NVIDIA-Proprietary` | -| `cuda-bindings` | `LicenseRef-NVIDIA-SOFTWARE-LICENSE` | -| `nvidia-nvcomp-cu12` | `LICENSE AGREEMENT FOR NVIDIA SOFTWARE DEVELOPMENT KITS` — via `streaming-gpu` (shardad's GPU decode) | -| `cuda-toolkit` | **none at all** — no identifier and no shipped license file. An empty NVIDIA metapackage; treat as undeclared/unknown rather than assuming particular terms. See flag 4 | -| `cuda-pathfinder` | Apache-2.0 ✅ | -| `nvidia-nvtx-cu12` | Apache-2.0 ✅ — per its `License` value and shipped Apache text; note its classifier contradictorily says `Other/Proprietary License` | -| `triton` | MIT ✅ | -| `cupy-cuda12x` | MIT ✅ — via `streaming-gpu` | - -### MPL-2.0 (weak, file-level copyleft) - -- `certifi` (MPL-2.0) -- `tqdm` (MPL-2.0 AND MIT) -- `fast-array-utils`, `legacy-api-wrap`, `session-info2` (via the `scanpy` dev extra) - -**Mere internal use does not trigger MPL distribution or source-availability -obligations.** On **distribution in executable form**, MPL-2.0 §3.2 requires -that the Source Code Form of the covered files be made available under the MPL, -together with the required notices — this applies whether or not you modified -them. "File-level" copyleft bounds *which files* the obligation reaches (not -unrelated files in the Larger Work); it does not mean the obligation attaches -only to files you changed. - -### Everything else - -The remaining transitive packages in gpudge's dependency closure are permissive -— MIT / BSD / Apache-2.0 / PSF. (No count is given deliberately: it drifts with -every dependency bump.) - -## Compliance flags - -1. **Proprietary NVIDIA CUDA components are present.** Most of the CUDA runtime - set is proprietary (the Apache/MIT exceptions are marked in the table above), - and `streaming-gpu` adds `nvidia-nvcomp-cu12`. This does not change gpudge's - own MIT licensing, but use *and* redistribution of those components remain - governed by NVIDIA's terms — which is what blocks redistributing the - assembled binary environment under open terms. - -2. **Three packages ship a license but declare no identifier.** `shardad` - (MIT), `hdf5plugin` (MIT, wrapper only — see flag 3), and `google-crc32c` - (Apache-2.0, a direct requirement of `zarr`; `numcodecs` offers it only under - an optional extra that is not active here) each have a `License-File` and a - real grant, but no expression, legacy value, or classifier. No *grant* is - missing; this is an SBOM/tooling defect. Scanners keyed on the identifier - fields cannot resolve these from those fields, whereas scanners that read - `License-File` can. Worth adding a license expression to shardad's - `pyproject.toml` before any public / PyPI release that references it. - - *Resolved since the previous audit:* shardad had no `LICENSE` file at all at - 0.2.0, which that audit correctly flagged as all-rights-reserved by default. - It has since been licensed MIT. - - A neighbouring case where the identifier fields *are* populated but say nothing: - `umap-learn` (transitive, via the `scanpy` dev extra) declares the bare - classifier `License :: OSI Approved` — naming no license family — over a legacy - `License` value of just `BSD`. Its shipped `LICENSE.txt` is BSD 3-Clause - (© 2017 Leland McInnes). Same remedy, same non-issue for rights. - -3. **`hdf5plugin`'s bundled filters are not covered by its MIT grant.** The - shipped `LICENSE` grants MIT for the hdf5plugin code itself; the bundled HDF5 - filters have their own terms, and the wheel includes neither their notices - nor upstream's `doc/information.rst` enumerating them. Redistributing the - assembled artifact needs a separate review of those filter licenses — this is - not merely a metadata gap. - -4. **`cuda-toolkit` has no grant to point at.** No identifier, no `License-File`, - no license text in the wheel. It is an empty NVIDIA metapackage, so exposure - is **low — not nil**: its redistribution status is genuinely unknown rather - than resolved, and the installed wheel does not establish which NVIDIA terms - govern. - -5. **SciPy's wheel bundles `LGPL-2.1-or-later` (`libquadmath`) and - `GPL-3.0-or-later WITH GCC-exception-3.1` (`libgfortran`).** Use is - unaffected; redistribution of the assembled environment is not purely - permissive. - -6. **`pyfastpfor` ships no license copy in any published artifact, and it is an - unconditional member of the `streaming` closure.** shardad requires it with no - condition beyond `extra == "cell"` (`Requires-Dist: pyfastpfor>=1.4; extra == - "cell"`), and both streaming extras select that extra, so installing either one - always installs it — which is what puts it in scope here. (At *runtime* it is imported - lazily and only for `codec='pfordelta'` cell archives; shardad's cell layout - also supports `codec='zstd'`, which opens without it. `pfordelta` is what every - production `.csad` uses.) PyPI carries only an sdist for 1.4.0, - and that sdist has no `LICENSE`/`NOTICE`/`COPYING` among its 75 entries; the - wheel a resolver builds from it is seven files with no license metadata and a - `.so` that embeds no license text. The grant is nevertheless clear from the - release's **own source** — `pyfastpfor.cc`'s header states Apache-2.0, as do 51 - of the 58 bundled `fastpfor/` files — so the **terms are permissive and the - license is established**; what is absent is the artifact-level copy. - **Apache-2.0 §4(a)** requires a redistributor to give recipients a copy of the - License, so **redistributing an assembled environment containing this package - means adding that copy yourself, from an authoritative source** — §4(a) governs - conveying the text, not where you obtain it, and the package supplies it - nowhere. (§4(d)'s `NOTICE` duty is conditional on the original work having a - `NOTICE` file; neither `searchivarius/PyFastPFor` nor `lemire/FastPFor` has one, - so it does not apply.) This is a **missing-license-copy** problem: strictly worse - than flag 2's packages, which ship a real grant and merely fail to declare an - identifier, and distinct from flag 3's hdf5plugin case, which is a - missing-*notice* problem for components under terms the wheel never states. - Worth reporting upstream. - -**Net:** everything gpudge itself authors is MIT, and every direct dependency's -*project-level* license is permissive. `pyfastpfor`, although transitive rather than -direct, is likewise Apache-2.0 at the project level even though its published -artifacts carry no license copy (flag 6). **Using** gpudge triggers no copyleft or -disclosure obligation. - -**Redistributing an assembled environment is a separate question and is not -purely permissive.** It carries NVIDIA's proprietary terms; MPL-2.0 §3.2 -source-availability for five packages; SciPy's bundled `LGPL-2.1-or-later` and -`GPL-3.0-or-later WITH GCC-exception-3.1` runtime libraries (so the installed SciPy *artifact* is -not purely permissive even though the SciPy *project* is BSD-3); hdf5plugin's -bundled-filter terms, whose notices the wheel does not ship; `pyfastpfor`'s -Apache-2.0 license copy, which §4(a) obliges a redistributor to convey and which -must be added from an authoritative source because no published artifact contains -it (flag 6); and `cuda-toolkit`, whose governing terms are simply unknown. -Separately, the identifier gaps in -flag 2 — and `umap-learn`'s content-free classifier — affect SBOM generation -rather than rights; flag 6 is **not** in that category, because a missing license -copy is a redistribution obligation, not a tooling defect. diff --git a/docs/release-notes/v0.3.0.md b/docs/release-notes/v0.3.0.md index 7fd30e3..dbff93f 100644 --- a/docs/release-notes/v0.3.0.md +++ b/docs/release-notes/v0.3.0.md @@ -10,7 +10,8 @@ ArcInstitute SSH access); gpudge is not on PyPI. ## 🌊 Shard-streaming `de(shard_archive=…)` -Stream a [shardad](https://github.com/ArcInstitute/shardad) target-aware v2 +Stream a [shardad](https://github.com/ArcInstitute/cellstream) — since renamed +**cellstream**, which is where that link goes — target-aware v2 archive directly, keeping the reference pool resident-sorted on the GPU and streaming guide-shards past it. Host RAM is bounded to ~reference + one shard — the full **5.54 M-cell CCL_2** screen runs in **~31 GB host RAM** on a single diff --git a/environment.yml b/environment.yml index d7a6bf1..9021286 100644 --- a/environment.yml +++ b/environment.yml @@ -6,7 +6,7 @@ # The gpudge pin is EXACT, unlike the README's `pip install gpudge`. The two are # for different things: someone following the README wants the current release, # while this file is checked in beside the code it belongs to, so unpinned, -# `mamba env create` from a v0.9.0 checkout would install whatever PyPI has +# `mamba env create` from a v0.9.1 checkout would install whatever PyPI has # latest. It is one of the files a release cut must bump, and it always has # been; it was pinned to a git tag before gpudge was published and stays pinned # now that it resolves from PyPI. @@ -27,8 +27,8 @@ # For the optional `de(archive=...)` streaming path, switch the extra to # [fast,streaming]; cellstream resolves from PyPI and needs no separate step. # (For GPU device decode use [fast,streaming-gpu], which adds cellstream's -# [gpu] extra. cellstream is the former shardad, renamed and first published -# at 0.9.0, which is the floor for both archive layouts.) +# [gpu] extra. cellstream's floor is 0.9.0, the first release carrying both +# archive layouts.) name: gpudge channels: - conda-forge @@ -37,4 +37,4 @@ dependencies: - pip - pip: - --extra-index-url https://download.pytorch.org/whl/cu126 - - "gpudge[fast]==0.9.0" + - "gpudge[fast]==0.9.1" diff --git a/pyproject.toml b/pyproject.toml index 9336607..1836858 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "gpudge" -version = "0.9.0" +version = "0.9.1" description = "Lightweight GPU-only Mann–Whitney U differential expression." readme = "README.md" authors = [{ name = "Arc Institute" }] @@ -61,9 +61,9 @@ fast = ["numba>=0.59"] # Optional: streaming a cellstream archive directly (de(archive=...)). # The core library takes an in-memory AnnData and does NOT need cellstream. # `streaming` is the CPU-friendly base: pulls no CUDA-only wheels onto host-only -# / CPU-CI installs. cellstream is the former `shardad`, renamed and published -# to PyPI at 0.9.0; the floor is that first public release rather than the old -# private v0.7.1 pin. The [gpu] extra is cupy + nvcomp, needed for device +# / CPU-CI installs. The floor is 0.9.0, cellstream's first PyPI release, which +# is also the first version carrying both archive layouts. +# The [gpu] extra is cupy + nvcomp, needed for device # decode; it belongs # here rather than surfacing as a bare ImportError at open time. # @@ -94,11 +94,10 @@ explicit = true [tool.uv.sources] torch = [{ index = "pytorch-cu126", marker = "platform_system == 'Linux'" }] -# NOTE: there is deliberately no `cellstream` entry here. It used to be a -# private git-SSH source pinned to `shardad`'s v0.7.1 tag; since the rename it -# resolves from PyPI like any other dependency, so the `streaming` extra above -# is the whole story. Removing it also removes the reason CI had to use -# `uv pip install` rather than `uv sync` -- see .github/workflows/ci.yml. +# NOTE: there is deliberately no `cellstream` entry here. It resolves from PyPI +# like any other dependency, so the `streaming` extra above is the whole story -- +# which is also why CI no longer has to prefer `uv pip install` over `uv sync`; +# see .github/workflows/ci.yml. # # Two caveats that hung off the old pin are gone with it: cellstream's Rust # extension is no longer optional (a source build without a toolchain now fails diff --git a/src/gpudge/__init__.py b/src/gpudge/__init__.py index 99d0330..d3b8847 100644 --- a/src/gpudge/__init__.py +++ b/src/gpudge/__init__.py @@ -58,7 +58,7 @@ warnings.warn( "gpudge: numba is not installed; falling back to scipy " "for sparse CSR row slicing (~3x slower on multi-million-cell " - "inputs). Install with `pip install gpudge[fast]` (or " + "inputs). Install with `pip install 'gpudge[fast]'` (or " "`uv pip install -e \".[fast]\"`) to enable the numba kernel.", stacklevel=2, ) @@ -1272,11 +1272,13 @@ def _finish(result): labels_t = torch.from_numpy(state.labels).to(device) # Auto-pick gene chunk size from free GPU memory if not provided. - # Heuristic adopted from pdex's default_gene_chunk_size: working memory is - # dominated by ref-cell ranking buffers (~24 bytes per ref cell per gene: - # float32 values + int64 sort indices + float64 ranks + workspace), capped - # at 16 GB and 20% of free GPU memory. For all_others, ranks cover every - # cell (1-vs-rest semantics), so use n_cells as the budget basis instead. + # Heuristic adapted from `default_gene_chunk_size` in the `gpu` branch of + # abhinadduri/pdex -- the GPU fork, not upstream pdex; see AUTHORS. Working + # memory is dominated by ref-cell ranking buffers (~24 bytes per ref cell per + # gene: float32 values + int64 sort indices + float64 ranks + workspace), + # capped at 16 GB and 20% of free GPU memory. For all_others, ranks cover + # every cell (1-vs-rest semantics), so use n_cells as the budget basis + # instead. # # In ref-mode we also allocate per-chunk GPU accumulators of shape # (n_groups, ch) × float64. Count: arithmetic / U / p (= 3) for diff --git a/src/gpudge/_cell_stream.py b/src/gpudge/_cell_stream.py index dcd5a34..6951217 100644 --- a/src/gpudge/_cell_stream.py +++ b/src/gpudge/_cell_stream.py @@ -7,7 +7,7 @@ from ._ingest import reject_missing_group_labels from ._csr_dense import csr_row_sums -from ._stream_backend import validate_archive_reference +from ._stream_backend import label_str, validate_archive_reference logger = logging.getLogger(__name__) @@ -67,42 +67,107 @@ def _plan_batches(ranges, *, bytes_per_row, target_bytes=None): def _cell_group_ranges(store): """``(reference_labels | None, [(label, start, stop), ...])`` for a CellStore. - Reads cellstream's **private** ``CellStore._load_groups()``. That upstream - ask has since been answered: cellstream 0.8.0 added public - ``CellStore.group_spans()`` (a list of ``GroupSpan(label, start, stop)``) and - ``CellStore.reference_row_count()``, which give exactly this table without - loading obs. Migrating to them is tracked separately and is a cleanup, not a - fix -- the private call still exists in the pinned 0.9.0 and the suites pass - against it. Until then the reasoning below is history, not a live claim: run-detection - over ``store.obs[group_by]`` would load the whole obs DataFrame -- - 36 columns x 1.27 M rows on a production archive -- to recover a table the - archive already stores, which is why the private reach-in was taken; ``test_cell_group_ranges_private_api_gone`` makes - its removal fail loudly rather than silently. - - Validates what cellstream's own ``read_reference()`` silently assumes: the - reference groups lead contiguously from row 0, so its ``[0, max stop)`` - gather cannot pull target cells into the reference pool. + Reads cellstream's **public** ``CellStore.group_spans()`` -- every group's + ``[start, stop)`` row range in storage order -- plus the manifest's + ``reference``. gpudge used to reach into the private ``_load_groups()`` + because no public equivalent existed; cellstream 0.8.0 added one (upstream + #248) and the ``>=0.9.0`` floor makes it always available. + ``test_cell_group_ranges_public_api_gone`` makes its removal fail loudly + rather than silently. Neither call reads obs: run-detection over + ``store.obs[group_by]`` would load the whole DataFrame -- 36 columns x 1.27 M + rows on a production archive -- to recover a table the archive already stores. + + ``reference_row_count()`` is deliberately NOT used as the source of the + reference block's end. The label split below has to happen regardless, and + deriving both from one table makes them consistent by construction instead + of by agreement. + + **Duplicate labels are rejected, and upstream declares them LEGAL.** Under + the default sort order, distinct raw obs values that stringify identically + produce several records carrying one label -- which is exactly why + ``group_spans()`` returns a list and not a mapping. For a **target** label + gpudge genuinely cannot handle it, and the mechanism is worth stating + precisely because it is not "the spans collapse into one group": + ``_CellBackend._targets`` keeps BOTH entries, while + ``_tgt_index = {lab: i for i, lab in enumerate(...)}`` maps both to the LAST + of them. So one accumulator row is never written at all and the other is + overwritten span by span, leaving the final span's subset -- a wrong answer in + two rows rather than a short read in one. + + For a **reference** label the rejection is stricter than it needs to be: + duplicate reference spans are handled correctly downstream + (``_ref_stop`` takes the max stop, and ``_target_ranges`` excludes every + reference-labelled span), so gpudge could accept them. It does not, because + that path has no fixture and no coverage, and loosening it is not what this + change is for. Tracked as a follow-up rather than done here. + + The tiling, coverage and reference-prefix checks below are kept, but they are + **future-contract assertions, not live cross-checks**: ``group_spans()`` + routes through ``_load_groups()``, which runs cellstream's own + ``_validate_groups_record`` first, so against any real cellstream >=0.9.0 + store a violation raises ``IncompatibleSchemaError`` upstream and these can + never fire. They exist so that a cellstream which stops validating fails here + instead of silently mis-slicing, and the fake-store tests exercise that + fallback -- not a production path. What IS load-bearing today is the call + ORDER: validation-before-use, not merely reading the manifest second. """ - # The per-record parse is INSIDE the try: a record missing 'label'/'start'/ - # 'stop', or carrying a non-numeric bound, is the same class of failure as - # the table itself changing shape, and should get the same explanatory - # error rather than a bare KeyError/ValueError from a list comprehension. + # One `where` for every message below, computed with getattr and therefore + # incapable of raising: a store whose contract has moved far enough may not + # carry `path`, and `store.path` inside the except block would replace the + # explanation with "during handling of the above exception, another exception + # occurred" -- while the same access in the ValueError paths would raise a + # bare AttributeError instead of the error those paths exist to give. Doing + # it once is also how the two stay consistent; being defensive in the except + # block alone was a half-measure. (Gemini review, PR #146.) + where = getattr(store, "path", "") + + # The per-span parse is INSIDE the try: a span that does not unpack into + # three values, or carries a non-numeric bound, is the same class of failure + # as the method disappearing, and should get the same explanatory error + # rather than a bare TypeError/ValueError from a list comprehension. try: - rec = store._load_groups() - raw = rec["groups"] - ref = rec["reference"] - ranges = [(str(g["label"]), int(g["start"]), int(g["stop"])) for g in raw] - except (AttributeError, KeyError, TypeError, ValueError) as e: + # label_str, not str(): a bytes label would become "b'ntc'" and reach the + # output frame as a label nobody wrote. Upstream types labels as `str`, so + # this is contract insurance rather than a live case -- but both sides of + # the comparison below have to normalize the same way, and they now share + # one helper. (Gemini review, PR #146.) + ranges = [(label_str(lab), int(start), int(stop)) + for lab, start, stop in store.group_spans()] + # AFTER group_spans(): loading the spans is what type-checks the + # manifest's 'reference' and cross-checks it against groups.json's own + # copy, so reading it first would read an unvalidated value. + # + # `.get`, not `["reference"]`, and that is deliberate -- a reviewer asked + # for the subscript so a missing key would fail loudly. Measured instead: + # a cell archive with NO reference and no `reference` key is VALID + # upstream (group_spans() returns its spans), so the subscript would + # refuse a good archive; and an archive that DOES have a reference but + # whose manifest lost the key already raises IncompatibleSchemaError from + # _validate_groups_record's manifest-vs-groups.json check, above, so the + # silent-misread this would guard against cannot reach here. + # (Copilot review, PR #146.) + ref = store.manifest.get("reference") + except (AttributeError, KeyError, TypeError, ValueError, OverflowError) as e: + # OverflowError is in the tuple because `int(float("inf"))` raises it + # rather than ValueError, so an inf-bounded span would otherwise escape + # this wrapper. NaN is NOT such a case -- `int(float("nan"))` raises + # ValueError, which was already caught. (codex; corrected by Copilot.) + # The message names BOTH surfaces because the try covers both: a + # `manifest` that is None or lacks `.get` lands here too, and blaming + # group_spans() for it would point at the call that succeeded. + # (Gemini review, PR #146.) raise RuntimeError( - f"gpudge cannot read the group table of {store.path}: cellstream's " - f"private CellStore._load_groups() did not return the expected " - f"{{'reference': ..., 'groups': [...]}} shape. gpudge pins " - f"cellstream>=0.9.0; see gpudge _cell_stream._cell_group_ranges." + f"gpudge cannot read the group table of " + f"{where}: cellstream's " + f"CellStore.group_spans() did not return the expected list of " + f"(label, start, stop) spans, or CellStore.manifest is not a mapping. " + f"gpudge pins cellstream>=0.9.0; see " + f"gpudge _cell_stream._cell_group_ranges." ) from e if not ranges: raise ValueError( - f"{store.path}: cell archive has no group table (was it written " + f"{where}: cell archive has no group table (was it written " f"with group_by=?)." ) labels = [lab for lab, _, _ in ranges] @@ -111,7 +176,7 @@ def _cell_group_ranges(store): for lab in labels: (dup if lab in seen else seen).add(lab) raise ValueError( - f"{store.path}: duplicate group labels {sorted(dup)} in the group table." + f"{where}: duplicate group labels {sorted(dup)} in the group table." ) # An unassigned cell reaches the archive as a group literally named 'nan' # (cellstream stringifies the obs column at write time), which the in-memory @@ -121,37 +186,37 @@ def _cell_group_ranges(store): # this function exists to avoid, so the string level is the right level. # (ultrareview 2026-08) reject_missing_group_labels( - labels, where=str(store.path), + labels, where=str(where), remedy="Drop or assign the unassigned cells before writing the archive.") pos = 0 for lab, start, stop in ranges: if start != pos or stop < start: raise ValueError( - f"{store.path}: group ranges do not tile [0, n_obs) -- group " + f"{where}: group ranges do not tile [0, n_obs) -- group " f"{lab!r} is [{start}, {stop}) but the previous group ended at " f"{pos}." ) pos = stop if pos != int(store.n_obs): raise ValueError( - f"{store.path}: group ranges cover {pos} rows but the archive has " + f"{where}: group ranges cover {pos} rows but the archive has " f"{int(store.n_obs)}." ) if ref is None: return None, ranges - ref_labels = [str(x) for x in (ref if isinstance(ref, list) else [ref])] + ref_labels = [label_str(x) for x in (ref if isinstance(ref, list) else [ref])] label_set = set(labels) # hoisted: rebuilt per reference label otherwise missing = [lab for lab in ref_labels if lab not in label_set] if missing: raise ValueError( - f"{store.path}: the manifest names reference label(s) {missing} that " + f"{where}: the manifest names reference label(s) {missing} that " f"are not in the group table." ) lead = {lab for lab, _, _ in ranges[:len(ref_labels)]} if lead != set(ref_labels): raise ValueError( - f"{store.path}: reference labels {sorted(ref_labels)} are not the " + f"{where}: reference labels {sorted(ref_labels)} are not the " f"leading groups (rows from 0 start with {sorted(lead)}). cellstream's " f"read_reference() gathers [0, max reference stop), so a non-leading " f"reference block would silently include target cells in the " diff --git a/src/gpudge/_csr_dense.py b/src/gpudge/_csr_dense.py index b0adca4..6f36b28 100644 --- a/src/gpudge/_csr_dense.py +++ b/src/gpudge/_csr_dense.py @@ -7,7 +7,7 @@ remaining post-FDR-fix bottleneck). One pass + parallel-rows kernel removes most of that overhead. -Install with ``pip install gpudge[fast]`` (or ``uv pip install -e +Install with ``pip install 'gpudge[fast]'`` (or ``uv pip install -e ".[fast]"``) to enable; the extra is just ``numba``. """ from __future__ import annotations diff --git a/src/gpudge/_stream_backend.py b/src/gpudge/_stream_backend.py index ad6a103..feb6ac8 100644 --- a/src/gpudge/_stream_backend.py +++ b/src/gpudge/_stream_backend.py @@ -50,6 +50,30 @@ from __future__ import annotations +def label_str(x): + """One group/reference label as a `str`, decoding bytes rather than repr-ing it. + + `str(b"ntc")` is `"b'ntc'"` -- the Python 3 str(bytes) trap -- which would + reach the output frame as a label nobody wrote. `str(...)` on the str arm too: + `np.str_` subclasses `str` and would otherwise leak out as `np.str_`. + + Module level, and used by BOTH `validate_archive_reference` and + `_cell_stream._cell_group_ranges`, so the two cannot disagree about what a + label is. It was a nested helper in the former until a review pointed out the + latter was missing it. (Gemini review, PR #146.) + + `isinstance`, not `hasattr(x, "decode")`: a reviewer asked for duck-typing on + the grounds that `np.bytes_` might not inherit from `bytes`. Measured -- it + does. `np.bytes_.__mro__` is `(bytes_, bytes, character, flexible, generic, + object)`, `np.dtype("S3").type` IS `np.bytes_`, an element taken from an + S-dtype array satisfies `isinstance(x, bytes)`, and this function already + returns `'ntc'` for all of them. The concern was scoped to NumPy 1.x, which + `pyproject.toml` excludes anyway (`numpy>=2`). Left as isinstance, which is + also the more precise test. (Gemini review, PR #146.) + """ + return x.decode() if isinstance(x, (bytes, bytearray)) else str(x) + + def validate_archive_reference(reference, labels): """Validate ``reference=`` against an archive's OWN reference labels. @@ -70,9 +94,7 @@ def validate_archive_reference(reference, labels): ``reference=['ntc', 'safe'] is not among the archive's reference labels ['ntc', 'safe']``. A sequence is now checked element-wise. """ - def _label(x): - return x.decode() if isinstance(x, (bytes, bytearray)) else str(x) - + _label = label_str # module-level now; see label_str's docstring label_list = sorted(labels) if reference is None: return "|".join(label_list) @@ -114,13 +136,14 @@ def _label(x): def _import_cellstream(): try: import cellstream - except ImportError as e: # pragma: no cover - exercised via monkeypatch + except ImportError as e: + # No `pragma: no cover` here: test_api.py's + # test_missing_cellstream_import_message reaches this branch on CPU CI. raise ImportError( "de(archive=...) requires the optional 'streaming' extra, which " - "installs cellstream (>=0.9.0, the former shardad) from PyPI. " - "Reinstall gpudge with the extra -- e.g. `pip install '.[streaming]'` " - "from a checkout, or add `streaming` to the pin in the README's " - "Install section." + "installs cellstream (>=0.9.0) from PyPI. " + "Reinstall gpudge with the extra -- `pip install 'gpudge[streaming]'`, " + "or `pip install '.[streaming]'` from a checkout." ) from e return cellstream diff --git a/tests/test_api.py b/tests/test_api.py index bcfed98..cdc27de 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -434,6 +434,37 @@ def test_iter_kwargs_serial_and_prefetch(): assert ss._iter_kwargs(8, 4) == {"prefetch": 4, "n_workers": 8} +def test_missing_cellstream_import_message(monkeypatch): + """The missing-streaming-extra error must name BOTH installs that work. + + Lives here, not in test_shard_stream.py, whose module-level + importorskip('cellstream') skips it precisely when cellstream is absent -- + the one situation this message exists for. The test could therefore never + run in the environment it describes. + + Asserts the commands in full. The previous `match="streaming"` is satisfied + by the extra's name alone, so it stayed green while every install command in + the message was rewritten. (codex, checkpoint 2.) + + `sys.modules[name] = None` is the import machinery's own "this module is + unavailable" marker, so it raises ImportError for this one name and leaves + every other import alone -- unlike replacing builtins.__import__, which was + the previous technique and hooks every import in the interpreter for the + duration. Verified against a venv that HAS cellstream installed; without one + the guard is unfalsifiable, because the bare import already fails. + (Gemini review, PR #144.) + """ + import sys + from gpudge._stream_backend import _import_cellstream + + monkeypatch.setitem(sys.modules, "cellstream", None) + with pytest.raises(ImportError) as excinfo: + _import_cellstream() + msg = str(excinfo.value) + assert "pip install 'gpudge[streaming]'" in msg # from PyPI + assert "pip install '.[streaming]'" in msg # from a checkout + + def test_should_device_decode_matrix(monkeypatch): """_should_device_decode truth table (pure helper, no cellstream/cupy needed — monkeypatched). Lives here, not in test_shard_stream.py, whose module-level diff --git a/tests/test_cell_stream.py b/tests/test_cell_stream.py index d65419a..fb5dcba 100644 --- a/tests/test_cell_stream.py +++ b/tests/test_cell_stream.py @@ -46,9 +46,11 @@ def cell_mode2(tmp_path): def test_cell_group_ranges_shape(cell_mode1): - """The private cellstream group table has the shape gpudge depends on, and the + """cellstream's public group_spans() has the shape gpudge depends on, and the ranges tile [0, n_obs) with the reference leading. This test is the tripwire - for cellstream changing CellStore._load_groups().""" + for cellstream changing CellStore.group_spans(), and -- because it reads a + real archive -- for the manifest's `reference` drifting from the labels the + spans carry.""" from gpudge._cell_stream import _cell_group_ranges p, adata = cell_mode1 store = cellstream.open(p) @@ -83,35 +85,42 @@ def test_cell_group_ranges_no_reference(cell_mode2): class _FakeStore: - """Minimal CellStore stand-in for the guard tests.""" + """Minimal CellStore stand-in for the guard tests. + + ``groups`` is a list of ``(label, start, stop)`` triples, handed back as real + ``cellstream.GroupSpan`` values so the fake cannot drift from the type the + store actually returns -- a plain tuple would keep passing after GroupSpan + grew a field or stopped unpacking. + """ path = "" def __init__(self, groups, reference, n_obs): - self._groups = {"reference": reference, "groups": groups} + self._spans = [cellstream.GroupSpan(*g) for g in groups] + self.manifest = {"reference": reference} self.n_obs = n_obs - def _load_groups(self): - return self._groups + def group_spans(self): + return list(self._spans) @pytest.mark.parametrize("groups, reference, n_obs, match", [ # gap between two groups - ([{"label": "a", "start": 0, "stop": 5}, - {"label": "b", "start": 7, "stop": 10}], None, 10, "tile"), + ([("a", 0, 5), ("b", 7, 10)], None, 10, "tile"), # overlap - ([{"label": "a", "start": 0, "stop": 5}, - {"label": "b", "start": 3, "stop": 10}], None, 10, "tile"), + ([("a", 0, 5), ("b", 3, 10)], None, 10, "tile"), # ranges stop short of n_obs - ([{"label": "a", "start": 0, "stop": 5}], None, 10, "cover"), - # duplicate label - ([{"label": "a", "start": 0, "stop": 5}, - {"label": "a", "start": 5, "stop": 10}], None, 10, "duplicate"), + ([("a", 0, 5)], None, 10, "cover"), + # duplicate label -- cellstream ALLOWS these (several records can share a + # stringified label); gpudge cannot. Not because the spans collapse: + # _targets keeps BOTH, while _tgt_index resolves both to the LAST, so one + # accumulator row is never written and the other is overwritten span by + # span. See _cell_group_ranges. (Copilot review, PR #146.) + ([("a", 0, 5), ("a", 5, 10)], None, 10, "duplicate"), # reference label absent from the group table - ([{"label": "a", "start": 0, "stop": 10}], "zzz", 10, "not in the group table"), + ([("a", 0, 10)], "zzz", 10, "not in the group table"), # reference group is NOT leading -- cellstream's read_reference() would gather # [0, ref_stop) and silently pull group 'a' into the reference pool - ([{"label": "a", "start": 0, "stop": 5}, - {"label": "ntc", "start": 5, "stop": 10}], "ntc", 10, "leading"), + ([("a", 0, 5), ("ntc", 5, 10)], "ntc", 10, "leading"), # empty group table ([], None, 0, "no group table"), ]) @@ -121,30 +130,115 @@ def test_cell_group_ranges_guards(groups, reference, n_obs, match): _cell_group_ranges(_FakeStore(groups, reference, n_obs)) -def test_cell_group_ranges_private_api_gone(): - """A cellstream that drops _load_groups() must fail loudly, not silently.""" +@pytest.mark.parametrize("manifest", [None, object(), 42], + ids=["none", "no-get", "not-a-mapping"]) +def test_cell_group_ranges_bad_manifest_is_wrapped(manifest): + """A `manifest` that is not a mapping lands in the SAME wrapper as a broken + group_spans(), because one try covers both -- so the message has to name both + surfaces. Before it did, this case reported that group_spans() had not + returned the expected spans, blaming the call that succeeded. + (Gemini review, PR #146.)""" + from gpudge._cell_stream import _cell_group_ranges + + class _BadManifest: + path = "" + n_obs = 10 + + def group_spans(self): + return [cellstream.GroupSpan("a", 0, 10)] + + store = _BadManifest() + store.manifest = manifest + with pytest.raises(RuntimeError, match="manifest"): + _cell_group_ranges(store) + + +def test_cell_group_ranges_without_path_still_explains(): + """A store lacking `path` must still get the explanatory error. + + Every message in _cell_group_ranges names the archive, and each one used to + read `store.path` directly. On a store whose contract has moved far enough to + drop it that raised a bare AttributeError instead -- in the except block it + would additionally have masked the original exception. They all route through + one getattr-computed local now, so the guard still speaks. + (Gemini review, PR #146.)""" + from gpudge._cell_stream import _cell_group_ranges + + class _NoPath: # deliberately no `path` + n_obs = 10 + manifest = {"reference": None} + + def group_spans(self): # a gap: does not tile [0, n_obs) + return [cellstream.GroupSpan("a", 0, 4), + cellstream.GroupSpan("b", 6, 10)] + + with pytest.raises(ValueError, match=".*tile"): + _cell_group_ranges(_NoPath()) + + +def test_cell_group_ranges_decodes_bytes_labels(): + """A bytes label must decode, not stringify to "b'ntc'". + + `str(b"ntc")` is the Python 3 str(bytes) trap: the label would reach the + output frame as a name nobody wrote, and the reference-membership comparison + would be made between two reprs. Upstream types labels as `str`, so this is + contract insurance -- but BOTH sides of that comparison (the spans and the + manifest's reference) now normalize through the same `label_str` helper, and + this pins it. (Gemini review, PR #146.)""" + from gpudge._cell_stream import _cell_group_ranges + + ref_labels, ranges = _cell_group_ranges( + _FakeStore([(b"ntc", 0, 4), (b"A", 4, 10)], b"ntc", 10)) + assert ref_labels == ["ntc"] + assert ranges == [("ntc", 0, 4), ("A", 4, 10)] + + +def test_cell_group_ranges_public_api_gone(): + """A cellstream that drops group_spans() must fail loudly, not silently.""" from gpudge._cell_stream import _cell_group_ranges class _NoGroups: path = "" n_obs = 10 + manifest = {"reference": None} - with pytest.raises(RuntimeError, match="_load_groups"): + with pytest.raises(RuntimeError, match="group_spans"): _cell_group_ranges(_NoGroups()) @pytest.mark.parametrize("bad", [ - {"start": 0, "stop": 5}, # missing 'label' - {"label": "a", "stop": 5}, # missing 'start' - {"label": "a", "start": 0, "stop": "five"}, # non-numeric bound -], ids=["no-label", "no-start", "bad-bound"]) -def test_cell_group_ranges_malformed_record_is_wrapped(bad): - """A malformed group RECORD is the same class of failure as the table - changing shape, and must get the same explanatory RuntimeError rather than - a bare KeyError/ValueError escaping a list comprehension.""" + ("a", 0), # too few values to unpack -> ValueError + ("a", 0, 5, 7), # too many -> ValueError + ("a", 0, "five"), # non-numeric bound -> ValueError from int() + ("a", 0, float("inf")), # non-finite bound -> OverflowError from int() + None, # not iterable at all -> TypeError +], ids=["short", "long", "bad-bound", "inf-bound", "not-a-span"]) +def test_cell_group_ranges_malformed_span_is_wrapped(bad): + """A malformed SPAN is the same class of failure as group_spans() itself + changing shape, and must get the same explanatory RuntimeError rather than a + bare TypeError/ValueError escaping a list comprehension. + + Uses a raw list rather than _FakeStore. GroupSpan is a NamedTuple, so its + annotations reject nothing -- `GroupSpan("a", 0, "five")` constructs fine -- + but the arity cases cannot be built through it at all, so the fake would + raise in its own constructor rather than in the code under test. The guard is + for a cellstream whose contract has moved, not for one that is behaving. + + `inf-bound` is the case that motivated widening the except clause: `int()` + raises OverflowError, not ValueError, on a non-finite float, so before that + widening this input escaped the wrapper entirely. (codex.)""" from gpudge._cell_stream import _cell_group_ranges - with pytest.raises(RuntimeError, match="_load_groups"): - _cell_group_ranges(_FakeStore([bad], None, 5)) + + class _BadSpans: + path = "" + n_obs = 5 + manifest = {"reference": None} + + def group_spans(self): + return [bad] + + with pytest.raises(RuntimeError, match="group_spans"): + _cell_group_ranges(_BadSpans()) def test_plan_batches_groups_are_never_split(): @@ -509,9 +603,9 @@ def _write_shard(adata, path, *, group_by, reference): def twin_mode1(tmp_path): """The SAME AnnData written both ways, with DEFAULT ordering. - Default ordering was load-bearing under the old shardad v0.7.1 pin: its cell + Default ordering was load-bearing under the pre-0.8.0 writer: its cell writer converted categorical sort keys with .to_numpy() (losing category - order) where the shard writer used .values (shardad #252), so a categorical + order) where the shard writer used .values (cellstream #252), so a categorical sort_within_group ordered rows differently between layouts and moved the float64 group means. Fixed upstream in 0.8.0, below the >=0.9.0 floor. The fixture keeps default ordering anyway: it is what the twin-mode exactness diff --git a/tests/test_shard_stream.py b/tests/test_shard_stream.py index 5924050..744dd6d 100644 --- a/tests/test_shard_stream.py +++ b/tests/test_shard_stream.py @@ -731,18 +731,6 @@ def test_stream_empty_targets_respects_output_columns(tmp_path): assert df.schema["p"] == pl.Float64 -def test_missing_cellstream_import_message(monkeypatch): - import builtins - from gpudge._stream_backend import _import_cellstream - real_import = builtins.__import__ - def fake(name, *a, **k): - if name == "cellstream": - raise ImportError("no cellstream") - return real_import(name, *a, **k) - monkeypatch.setattr(builtins, "__import__", fake) - with pytest.raises(ImportError, match="streaming"): - _import_cellstream() - def test_resolve_mode1_reference_read_none_raises(archive_mode1, monkeypatch): # Manifest declares a reference shard but read_reference() returns None @@ -1019,7 +1007,7 @@ def test_stream_empty_targets_with_lfc_grid(archive_mode1, tmp_path): # --- tau_star over the streaming paths ----------------------------------- # CI is CPU-only and does not even COLLECT this module (it needs cellstream), so -# these run only on a GPU host with shardad installed. That is precisely why they +# these run only on a GPU host with cellstream installed. That is precisely why they # exist: the tau* accumulators are threaded differently on each path # (`__init__.py` carries a reference row and drops it via target_indices; # `_refpool.py` does not have one at all), and a shape or indexing slip there