Skip to content

Commit 48fcc60

Browse files
authored
Merge pull request #12 from cellgeni/feat/nextflow-compatible-image
Nextflow-compatible image, faster streaming reads, release 0.5.1
2 parents 7f4df63 + 9a77450 commit 48fcc60

9 files changed

Lines changed: 257 additions & 27 deletions

File tree

‎.dockerignore‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
# Keep the build context to what `uv sync --locked` actually needs. In
2+
# particular .venv/ would otherwise ship a host-platform virtualenv into the
3+
# image, and .claude/worktrees/ holds full copies of this repo.
4+
#
5+
# Do NOT add README.md or LICENSE here: pyproject.toml references them via
6+
# `readme` and `license-files`, so the build fails without them.
7+
.venv/
8+
.git/
9+
.claude/
10+
.pytest_cache/
11+
__pycache__/
12+
*.py[cod]
13+
.coverage
14+
coverage.xml
15+
htmlcov/
16+
build/
17+
dist/
18+
*.egg-info/

‎CHANGELOG.md‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,48 @@
33
Notable changes to `adata-cli`. Versions are `MAJOR.MINOR.PATCH`; tags carry no
44
`v` prefix.
55

6+
## 0.5.1
7+
8+
Makes the container image usable from Nextflow, and stops `copy_dataset`
9+
reading one row at a time from row-chunked stores.
10+
11+
### Fixed
12+
13+
- **Copying a row-chunked store was dominated by read latency.** The read step
14+
was the source's chunk height verbatim, so a store chunked `(1, n_cols)` was
15+
copied one row per read. On a network filesystem (Lustre, NFS) each read is a
16+
round-trip, so a million-row copy spent nearly all of its time waiting. Reads
17+
are now sized to a 32 MiB budget, rounded down to a whole number of source
18+
chunks. A `(1_000_000, 30_000)` float32 store chunked `(1, 30_000)` goes from
19+
1 row per read to 279.
20+
- Read sizing no longer trusts `itemsize` for variable-length strings. h5py
21+
reports 8 there because the value is a pointer, which overestimated the row
22+
count by an order of magnitude and broke the memory bound.
23+
24+
### Container
25+
26+
- **The image could not be used from a Nextflow process.** Nextflow requires
27+
`/bin/bash` to be the container entrypoint, so `ENTRYPOINT ["adata"]` made
28+
every Docker- and Podman-backed task fail with `No such command
29+
'/bin/bash'`. Apptainer was unaffected, as `singularity exec` ignores the
30+
entrypoint.
31+
- **Task metrics were silently lost.** `procps` is absent from the base image,
32+
so Nextflow could not run `ps` to collect them. The required tool set
33+
(`bash`, `ps`, `awk`, `date`, `grep`, `sed`, `tail`, `tee`) is now installed
34+
and asserted at build time.
35+
- `PYTHONNOUSERSITE` is set, so a bind-mounted `$HOME` under Apptainer can no
36+
longer shadow the image's virtualenv with the user's `~/.local` packages.
37+
- `XDG_CACHE_HOME` points at `/tmp`, so the image tolerates being run under an
38+
arbitrary UID with no writable `$HOME`.
39+
- Added a `.dockerignore`. Local builds were copying the host's `.venv`,
40+
`.git` and `.pytest_cache` into the image.
41+
42+
### Changed
43+
44+
- **The image no longer sets an entrypoint, so the command must be named
45+
explicitly:** `docker run IMAGE adata view file.h5ad`, where `docker run
46+
IMAGE view file.h5ad` previously worked.
47+
648
## 0.5.0
749

850
Renamed from `h5ad` to `adata-cli`, restored compatibility with current

‎Dockerfile‎

Lines changed: 57 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,33 @@
11
# Base image: Python 3.12 + uv preinstalled (Debian slim)
22
FROM ghcr.io/astral-sh/uv:python3.12-bookworm-slim
33

4-
ENV UV_NO_DEV=1
4+
# PYTHONNOUSERSITE: Apptainer bind-mounts the host $HOME by default, so a user's
5+
# ~/.local/lib/python3.12/site-packages would otherwise shadow this venv.
6+
# XDG_CACHE_HOME: Nextflow is commonly configured with `-u $(id -u):$(id -g)`,
7+
# which leaves the container with no writable $HOME.
8+
# UV_COMPILE_BYTECODE: bake .pyc at build time, so nothing writes to a
9+
# read-only rootfs on first import.
10+
ENV UV_NO_DEV=1 \
11+
UV_COMPILE_BYTECODE=1 \
12+
PYTHONNOUSERSITE=1 \
13+
PYTHONDONTWRITEBYTECODE=1 \
14+
PYTHONUNBUFFERED=1 \
15+
XDG_CACHE_HOME=/tmp/.cache
516

6-
WORKDIR /cli
7-
8-
# Copy the project files (from the GitHub Actions checkout context)
9-
COPY . .
10-
11-
# --locked asserts that uv.lock is in sync with pyproject.toml, so an image
12-
# can never be built from a lockfile that drifted.
13-
RUN uv sync --locked
17+
# procps supplies `ps`, which Nextflow needs to collect per-task metrics.
18+
# curl, unzip and ca-certificates fetch duckdb below, and are left in place
19+
# rather than purged: pipeline scripts routinely reach for curl, and TLS
20+
# roots are worth having in any container that may touch the network.
21+
RUN apt-get update \
22+
&& apt-get install -y --no-install-recommends \
23+
ca-certificates curl procps mawk unzip \
24+
&& rm -rf /var/lib/apt/lists/*
1425

1526
# duckdb, for the filtering workflows in the docs: export obs to CSV, query it,
1627
# feed the names back to `adata subset --obs`. A single static binary, so it
1728
# needs no venv and cannot conflict with the project's dependencies.
1829
ARG DUCKDB_VERSION=v1.1.3
19-
RUN apt-get update \
20-
&& apt-get install -y --no-install-recommends ca-certificates curl unzip \
21-
&& ARCH="$(dpkg --print-architecture)" \
30+
RUN ARCH="$(dpkg --print-architecture)" \
2231
&& case "$ARCH" in \
2332
amd64) DUCKDB_ARCH=amd64 ;; \
2433
arm64) DUCKDB_ARCH=aarch64 ;; \
@@ -28,13 +37,43 @@ RUN apt-get update \
2837
"https://github.com/duckdb/duckdb/releases/download/${DUCKDB_VERSION}/duckdb_cli-linux-${DUCKDB_ARCH}.zip" \
2938
&& unzip -q /tmp/duckdb.zip -d /usr/local/bin \
3039
&& chmod +x /usr/local/bin/duckdb \
31-
&& rm /tmp/duckdb.zip \
32-
&& apt-get purge -y curl unzip \
33-
&& apt-get autoremove -y \
34-
&& rm -rf /var/lib/apt/lists/*
40+
&& rm /tmp/duckdb.zip
41+
42+
# Fail the build, rather than every Nextflow task, if the base image ever drops
43+
# one of the tools Nextflow requires in a task container.
44+
RUN set -eu; for t in bash ps awk date grep sed tail tee; do \
45+
command -v "$t" >/dev/null || { echo "missing required tool: $t" >&2; exit 1; }; \
46+
done
47+
48+
WORKDIR /cli
49+
50+
# Copy the project files (from the GitHub Actions checkout context)
51+
COPY . .
52+
53+
# --locked asserts that uv.lock is in sync with pyproject.toml, so an image
54+
# can never be built from a lockfile that drifted.
55+
#
56+
# uv honours XDG_CACHE_HOME, so the sync leaves a root-owned package cache at
57+
# /tmp/.cache -- which made the variable self-defeating, as a task running
58+
# under an arbitrary UID then could not write to the very path it advertises.
59+
# Clear it and leave an empty world-writable directory behind. Done in this
60+
# same layer because a later `rm` would mask the files without reclaiming
61+
# them; that reclaims about 9 MB, the cache being mostly hardlinks into the
62+
# venv rather than separate copies.
63+
RUN uv sync --locked \
64+
&& rm -rf /tmp/.cache /tmp/uv-*.lock \
65+
&& mkdir -p /tmp/.cache \
66+
&& chmod 1777 /tmp/.cache
3567

3668
# Put the project venv on PATH so `adata` is directly runnable
3769
ENV PATH="/cli/.venv/bin:${PATH}"
3870

39-
ENTRYPOINT ["adata"]
40-
CMD ["--help"]
71+
# No ENTRYPOINT on purpose: Nextflow requires /bin/bash to be the container
72+
# entrypoint, so the image must not set one of its own. This is why invocations
73+
# spell out the command: `docker run IMAGE adata view file.h5ad`.
74+
#
75+
# Deliberately NOT set here: OMP_NUM_THREADS / OPENBLAS_NUM_THREADS. NumPy's
76+
# BLAS sizes its thread pool to the whole host, which oversubscribes a shared
77+
# LSF node. This workload is streaming I/O, so capping it would cost nothing --
78+
# but it belongs in the pipeline's `env` scope, not baked into the image.
79+
CMD ["adata", "--help"]

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,5 +91,5 @@ adata concat per_sample/*.h5ad -o merged.h5ad --join outer --label sample
9191
A docker image is available on QUAY: `quay.io/cellgeni/adata-cli:latest`. Pull and run with:
9292

9393
```bash
94-
docker run --rm -it -v /path/to/data:/data quay.io/cellgeni/adata-cli:latest view /data/your_file.h5ad
94+
docker run --rm -it -v /path/to/data:/data quay.io/cellgeni/adata-cli:latest adata view /data/your_file.h5ad
9595
```

‎docs/index.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ Or run it without installing anything:
2222

2323
```bash
2424
docker run --rm -it -v /path/to/data:/data \
25-
quay.io/cellgeni/adata-cli:latest view /data/your_file.h5ad
25+
quay.io/cellgeni/adata-cli:latest adata view /data/your_file.h5ad
2626
```
2727

2828
## Documentation

‎pyproject.toml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
# so the distribution is published as `pyadata-cli`. The import package
44
# and the command are both still `adata`.
55
name = "pyadata-cli"
6-
version = "0.5.0"
6+
version = "0.5.1"
77
description = "Streaming CLI for exploring and editing large AnnData .h5ad and .zarr stores"
88
readme = "README.md"
99
requires-python = ">=3.12"

‎src/adata/storage/__init__.py‎

Lines changed: 62 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -440,12 +440,69 @@ def _is_string_src(src: Any) -> bool:
440440
return is_string_dtype(getattr(src, "dtype", None))
441441

442442

443-
def _chunk_step(shape: Sequence[int], chunks: Optional[Sequence[int]]) -> int:
444-
if chunks is not None and len(chunks) > 0 and chunks[0]:
445-
return int(chunks[0])
443+
#: Byte budget for a single read when streaming a dataset.
444+
#:
445+
#: Using a source's chunk height as the read size means inheriting whatever
446+
#: the writer chose, and a store chunked `(1, n_cols)` is then copied one row
447+
#: per read. On a local disk that is merely wasteful; on a network filesystem
448+
#: (Lustre, NFS) every read is a round-trip costing milliseconds, so a
449+
#: million-row copy spends nearly all of its time waiting. 32 MiB sits
450+
#: comfortably above a typical 1 MiB Lustre stripe while bounding peak memory.
451+
TARGET_READ_BYTES = 32 * 1024 * 1024
452+
453+
#: Assumed width of a variable-length string element, for read sizing only.
454+
#: h5py reports `itemsize` 8 for vlen strings because the value is a pointer,
455+
#: which would overestimate the row count by an order of magnitude and break
456+
#: the memory bound. Real cell and gene names sit well under this.
457+
VLEN_ELEMENT_BYTES = 64
458+
459+
460+
def _row_bytes(shape: Sequence[int], dtype: Any) -> int:
461+
"""In-memory size of one row along the first axis."""
462+
width = 1
463+
for dim in shape[1:]:
464+
width *= max(1, int(dim))
465+
466+
itemsize = int(getattr(dtype, "itemsize", 0) or 0)
467+
# 'O' is how h5py spells vlen str, 'T' is numpy StringDType as reported by
468+
# zarr-python 3. Neither itemsize reflects the bytes actually stored.
469+
if itemsize <= 0 or getattr(dtype, "kind", None) in ("O", "T"):
470+
itemsize = VLEN_ELEMENT_BYTES
471+
472+
return max(1, width * itemsize)
473+
474+
475+
def _chunk_step(
476+
shape: Sequence[int], chunks: Optional[Sequence[int]], dtype: Any
477+
) -> int:
478+
"""Rows to copy per read, sized for the filesystem rather than the source.
479+
480+
The step is grown to `TARGET_READ_BYTES`, then rounded down to a whole
481+
number of source chunks: reading part of a chunk still costs decompressing
482+
all of it, so a step that splits one wastes the remainder.
483+
"""
446484
if not shape:
447485
return 1
448-
return max(1, min(1024, int(shape[0])))
486+
487+
n_rows = int(shape[0])
488+
if n_rows <= 0:
489+
return 1
490+
491+
chunk_rows = 0
492+
if chunks is not None and len(chunks) > 0 and chunks[0]:
493+
chunk_rows = max(1, int(chunks[0]))
494+
495+
step = max(1, TARGET_READ_BYTES // _row_bytes(shape, dtype))
496+
if chunk_rows:
497+
# Never go below a single chunk: a partial read still decompresses the
498+
# whole thing, so a smaller step costs the same I/O for less data. That
499+
# makes TARGET_READ_BYTES a target rather than a cap -- a source whose
500+
# own chunk already exceeds the budget (say 1000 x 1e6 float32, chunked
501+
# whole) reads that chunk regardless. This matches the previous
502+
# behaviour, which used the chunk height verbatim.
503+
step = max(chunk_rows, (step // chunk_rows) * chunk_rows)
504+
505+
return min(step, n_rows)
449506

450507

451508
def copy_dataset(src: Any, dst_group: Any, name: str) -> Any:
@@ -475,7 +532,7 @@ def copy_dataset(src: Any, dst_group: Any, name: str) -> Any:
475532
ds[()] = src[()]
476533
return ds
477534

478-
step = _chunk_step(shape, getattr(src, "chunks", None))
535+
step = _chunk_step(shape, getattr(src, "chunks", None), src.dtype)
479536
for start in range(0, shape[0], step):
480537
end = min(start + step, shape[0])
481538
if len(shape) == 1:

‎tests/test_storage.py‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -393,3 +393,77 @@ def test_clamping_handles_one_dimensional_chunks():
393393

394394
assert _clamp_chunks({"chunks": (100,)}, 5)["chunks"] == (5,)
395395
assert _clamp_chunks({}, 5) == {}
396+
397+
398+
# ---------------------------------------------------------------------------
399+
# read sizing
400+
401+
402+
def test_read_step_grows_past_a_pathological_source_chunk():
403+
"""A `(1, n_cols)` source must not be copied one row per read.
404+
405+
Forwarding the source's chunk height meant inheriting whatever the writer
406+
chose. On a network filesystem each read is a round-trip, so a row-chunked
407+
million-cell store spent the whole copy waiting on latency.
408+
"""
409+
from adata.storage import TARGET_READ_BYTES, _chunk_step, _row_bytes
410+
411+
shape, chunks, dtype = (1_000_000, 30_000), (1, 30_000), np.dtype("float32")
412+
step = _chunk_step(shape, chunks, dtype)
413+
414+
assert step > 1, "the source chunk height is no longer used verbatim"
415+
assert step * _row_bytes(shape, dtype) >= TARGET_READ_BYTES // 2
416+
417+
418+
def test_read_step_stays_a_whole_number_of_source_chunks():
419+
"""Reading part of a chunk still costs decompressing all of it."""
420+
from adata.storage import _chunk_step
421+
422+
for chunk_rows in (1, 7, 100, 65_536):
423+
step = _chunk_step((10_000_000,), (chunk_rows,), np.dtype("int64"))
424+
assert step % chunk_rows == 0
425+
426+
427+
def test_read_step_never_exceeds_the_dataset():
428+
from adata.storage import _chunk_step
429+
430+
assert _chunk_step((10,), (1,), np.dtype("int64")) == 10
431+
assert _chunk_step((), None, np.dtype("int64")) == 1
432+
assert _chunk_step((0,), None, np.dtype("int64")) == 1
433+
434+
435+
def test_read_step_bounds_peak_memory():
436+
"""A wide unchunked source used to read 1024 rows however wide they were."""
437+
from adata.storage import TARGET_READ_BYTES, _chunk_step, _row_bytes
438+
439+
cases = [
440+
((1_000_000, 30_000), None, np.dtype("float32")),
441+
((1_000_000, 50), (1, 50), np.dtype("float32")),
442+
((200_000_000,), (65_536,), np.dtype("int64")),
443+
]
444+
for shape, chunks, dtype in cases:
445+
step = _chunk_step(shape, chunks, dtype)
446+
assert step * _row_bytes(shape, dtype) <= 2 * TARGET_READ_BYTES
447+
448+
449+
def test_row_bytes_does_not_trust_a_vlen_itemsize():
450+
"""h5py reports itemsize 8 for vlen str -- that is the pointer, not the text."""
451+
import h5py
452+
453+
from adata.storage import VLEN_ELEMENT_BYTES, _row_bytes
454+
455+
assert _row_bytes((10,), h5py.string_dtype(encoding="utf-8")) == VLEN_ELEMENT_BYTES
456+
assert _row_bytes((10,), np.dtype("O")) == VLEN_ELEMENT_BYTES
457+
assert _row_bytes((10, 4), np.dtype("float32")) == 16
458+
459+
460+
def test_copy_dataset_of_a_row_chunked_source_round_trips(new_store):
461+
"""The larger read step must not change what lands on disk."""
462+
path, opener = new_store()
463+
with opener("a") as root:
464+
from adata.storage import create_dataset
465+
466+
values = np.arange(400, dtype="float32").reshape(100, 4)
467+
create_dataset(root["uns"], "src", data=values, chunks=(1, 4))
468+
copy_dataset(root["uns"]["src"], root["uns"], "dst")
469+
assert np.array_equal(root["uns"]["dst"][...], values)

‎uv.lock‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)