From 7f47fafc14d530d2e5a80c3e55e613d6bab8220c Mon Sep 17 00:00:00 2001 From: SkyeAv Date: Mon, 27 Jul 2026 13:59:13 -0700 Subject: [PATCH 1/9] Swap build backend from setuptools to maturin MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the setuptools build backend with maturin so the project builds as a mixed Rust/Python package. This is the backend swap only — the Rust side is an empty placeholder module (src.rs, importable as `from src import rs`) that real functionality will fill in via a follow-up. - pyproject.toml: build-system -> maturin; drop [tool.setuptools.packages.find]; add [tool.maturin] (module-name = "src.rs", manifest-path = "rust/Cargo.toml"); add maturin to the dev dependency group. - rust/: new Cargo.toml + src/lib.rs stub (empty #[pymodule] `rs`, pyo3 abi3). Rust source lives under rust/ to avoid colliding with the Python src/ dir. - .gitignore: ignore rust/target/ and the maturin-develop .so/.pyd/.dylib. - CI/Docker: install a Rust toolchain (dtolnay/rust-toolchain in test.yml, apt cargo in Dockerfile) since every `uv sync` now compiles via cargo. - uv.lock + rust/Cargo.lock regenerated. The importable package stays `src`; nothing under src/ moves or renames, and all console scripts and subpackages are preserved (verified: 402 unit tests pass; wheel bundles src/* plus src/rs.abi3.so). --- .github/workflows/test.yml | 2 + .gitignore | 7 ++ Dockerfile | 4 + pyproject.toml | 17 ++-- rust/Cargo.lock | 180 +++++++++++++++++++++++++++++++++++++ rust/Cargo.toml | 11 +++ rust/src/lib.rs | 13 +++ uv.lock | 23 +++++ 8 files changed, 251 insertions(+), 6 deletions(-) create mode 100644 rust/Cargo.lock create mode 100644 rust/Cargo.toml create mode 100644 rust/src/lib.rs diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index da065f340..958a79102 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -17,6 +17,7 @@ jobs: - uses: astral-sh/setup-uv@v7 with: python-version: "3.11" + - uses: dtolnay/rust-toolchain@stable - run: uv sync --frozen - run: PYTHONPATH=. uv run pytest --collect-only -q - run: PYTHONPATH=. uv run pytest -m unit --no-cov -q @@ -34,5 +35,6 @@ jobs: - uses: astral-sh/setup-uv@v7 with: python-version: "3.11" + - uses: dtolnay/rust-toolchain@stable - run: uv sync --frozen - run: PYTHONPATH=. uv run pytest --network -m "unit or network or slow" --no-cov -q diff --git a/.gitignore b/.gitignore index 36e021879..adbc5b9c8 100644 --- a/.gitignore +++ b/.gitignore @@ -36,3 +36,10 @@ docs/sources/*/impact-report/modified-cliques.json .coverage .coverage.* htmlcov/ + +# maturin / Rust build artifacts +rust/target/ +# native extension dropped into src/ by `maturin develop` (don't commit compiled binaries) +src/rs.*.so +src/rs.*.pyd +src/rs.*.dylib diff --git a/Dockerfile b/Dockerfile index ae4fdd4ee..96732ba49 100644 --- a/Dockerfile +++ b/Dockerfile @@ -36,6 +36,10 @@ RUN adduser --home ${ROOT} --uid 1000 nru # Set up a $ROOT directory with the source code to work in. RUN mkdir -p ${ROOT} WORKDIR ${ROOT} + +# Rust toolchain — required because the build backend is maturin (compiles a native extension). +RUN apt-get update && apt-get install -y cargo + USER nru COPY --chown=nru . ${ROOT} diff --git a/pyproject.toml b/pyproject.toml index 63b17de88..7935dfcb0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -39,6 +39,7 @@ Issues = "https://github.com/NCATSTranslator/Babel/issues" [dependency-groups] dev = [ "ipykernel>=7.1.0", + "maturin>=1.0,<2.0", "py-spy>=0.4.2", "ruff>=0.14.14", "rumdl>=0.2.4", @@ -55,12 +56,16 @@ source-impact-report = "src.tools.source_impact_report.cli:main" package = true [build-system] -requires = ["setuptools>=83.0.0"] -build-backend = "setuptools.build_meta" - -[tool.setuptools.packages.find] -where = ["."] -include = ["src", "src.*"] +requires = ["maturin>=1.0,<2.0"] +build-backend = "maturin" + +[tool.maturin] +# The importable Python package is literally named `src` (flat layout). A dotted module-name +# places the compiled extension at `src/rs.*` and tells maturin the package to bundle is `src` +# (auto-resolved to the repo root since src-layout detection won't fire). +module-name = "src.rs" +# Rust source lives under rust/ so Cargo never collides with the Python `src/` dir. +manifest-path = "rust/Cargo.toml" # Linting/formatting configuration diff --git a/rust/Cargo.lock b/rust/Cargo.lock new file mode 100644 index 000000000..76ba98f35 --- /dev/null +++ b/rust/Cargo.lock @@ -0,0 +1,180 @@ +# This file is automatically @generated by Cargo. +# It is not intended for manual editing. +version = 4 + +[[package]] +name = "autocfg" +version = "1.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f2032f911046de80f0a198e0901378627c33f59ea0ac00e363d481118bd70a53" + +[[package]] +name = "babel-pipeline" +version = "1.17.0" +dependencies = [ + "pyo3", +] + +[[package]] +name = "cfg-if" +version = "1.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9330f8b2ff13f34540b44e946ef35111825727b38d33286ef986142615121801" + +[[package]] +name = "heck" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2304e00983f87ffb38b55b444b5e3b60a884b5d30c0fca7d82fe33449bbe55ea" + +[[package]] +name = "indoc" +version = "2.0.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "79cf5c93f93228cf8efb3ba362535fb11199ac548a09ce117c9b1adc3030d706" +dependencies = [ + "rustversion", +] + +[[package]] +name = "libc" +version = "0.2.189" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3eaf3ede3fee6db1a4c2ee091bf8a8b4dccdc6d17f656fb07896ee72867612f2" + +[[package]] +name = "memoffset" +version = "0.9.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "488016bfae457b036d996092f6cb448677611ce4449e970ceaf42695203f218a" +dependencies = [ + "autocfg", +] + +[[package]] +name = "once_cell" +version = "1.21.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9f7c3e4beb33f85d45ae3e3a1792185706c8e16d043238c593331cc7cd313b50" + +[[package]] +name = "portable-atomic" +version = "1.14.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3d20d5497ef88037a52ff98267d066e7f11fcc5e99bbfbd58a42336193aacec3" + +[[package]] +name = "proc-macro2" +version = "1.0.107" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "985e7ec9bb745e6ce6535b544d84d6cd6f7ad8bd711c398938ae983b91a766d9" +dependencies = [ + "unicode-ident", +] + +[[package]] +name = "pyo3" +version = "0.23.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7778bffd85cf38175ac1f545509665d0b9b92a198ca7941f131f85f7a4f9a872" +dependencies = [ + "cfg-if", + "indoc", + "libc", + "memoffset", + "once_cell", + "portable-atomic", + "pyo3-build-config", + "pyo3-ffi", + "pyo3-macros", + "unindent", +] + +[[package]] +name = "pyo3-build-config" +version = "0.23.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "94f6cbe86ef3bf18998d9df6e0f3fc1050a8c5efa409bf712e661a4366e010fb" +dependencies = [ + "once_cell", + "target-lexicon", +] + +[[package]] +name = "pyo3-ffi" +version = "0.23.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e9f1b4c431c0bb1c8fb0a338709859eed0d030ff6daa34368d3b152a63dfdd8d" +dependencies = [ + "libc", + "pyo3-build-config", +] + +[[package]] +name = "pyo3-macros" +version = "0.23.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fbc2201328f63c4710f68abdf653c89d8dbc2858b88c5d88b0ff38a75288a9da" +dependencies = [ + "proc-macro2", + "pyo3-macros-backend", + "quote", + "syn", +] + +[[package]] +name = "pyo3-macros-backend" +version = "0.23.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fca6726ad0f3da9c9de093d6f116a93c1a38e417ed73bf138472cf4064f72028" +dependencies = [ + "heck", + "proc-macro2", + "pyo3-build-config", + "quote", + "syn", +] + +[[package]] +name = "quote" +version = "1.0.47" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1fbf4db142a473a8d80c26bbf18454ed458bf8d26c8219c331daecfdbd079001" +dependencies = [ + "proc-macro2", +] + +[[package]] +name = "rustversion" +version = "1.0.23" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cf54715a573b99ac80df0bc206da022bcd442c974952c7b9720069370852e21f" + +[[package]] +name = "syn" +version = "2.0.119" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "872831b642d1a07999a962a351ed35b955ea2cfc8f3862091e2a240a84f17297" +dependencies = [ + "proc-macro2", + "quote", + "unicode-ident", +] + +[[package]] +name = "target-lexicon" +version = "0.12.16" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "61c41af27dd6d1e27b1b16b489db798443478cef1f06a660c96db617ba5de3b1" + +[[package]] +name = "unicode-ident" +version = "1.0.24" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e6e4313cd5fcd3dad5cafa179702e2b244f760991f45397d14d4ebf38247da75" + +[[package]] +name = "unindent" +version = "0.2.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7264e107f553ccae879d21fbea1d6724ac785e8c3bfc762137959b5802826ef3" diff --git a/rust/Cargo.toml b/rust/Cargo.toml new file mode 100644 index 000000000..426067dda --- /dev/null +++ b/rust/Cargo.toml @@ -0,0 +1,11 @@ +[package] +name = "babel-pipeline" +version = "1.17.0" # valid SemVer (3 parts); published version is "1.17" from root pyproject +edition = "2021" + +[lib] +name = "rs" # matches the #[pymodule] fn + module-name's last component +crate-type = ["cdylib"] + +[dependencies] +pyo3 = { version = "0.23", features = ["abi3-py311", "extension-module"] } diff --git a/rust/src/lib.rs b/rust/src/lib.rs new file mode 100644 index 000000000..0db123630 --- /dev/null +++ b/rust/src/lib.rs @@ -0,0 +1,13 @@ +//! Placeholder native extension for babel-pipeline. +//! +//! The build backend is maturin; this empty module exists only so the project builds as a +//! mixed Rust/Python package. Real functionality will land in a follow-up PR. + +use pyo3::prelude::*; + +/// Empty pyo3 module, importable as `from src import rs`. Name must match the last component +/// of `[tool.maturin] module-name` ("src.rs" → `rs`). +#[pymodule] +fn rs(_m: &Bound<'_, PyModule>) -> PyResult<()> { + Ok(()) +} diff --git a/uv.lock b/uv.lock index 1502a8e5c..de33fc912 100644 --- a/uv.lock +++ b/uv.lock @@ -211,6 +211,7 @@ dependencies = [ [package.dev-dependencies] dev = [ { name = "ipykernel" }, + { name = "maturin" }, { name = "py-spy" }, { name = "ruff" }, { name = "rumdl" }, @@ -247,6 +248,7 @@ requires-dist = [ [package.metadata.requires-dev] dev = [ { name = "ipykernel", specifier = ">=7.1.0" }, + { name = "maturin", specifier = ">=1.0,<2.0" }, { name = "py-spy", specifier = ">=0.4.2" }, { name = "ruff", specifier = ">=0.14.14" }, { name = "rumdl", specifier = ">=0.2.4" }, @@ -1239,6 +1241,27 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/41/09/5b161152e2d90f7b87f781c2e1267494aef9c32498df793f73ad0a0a494a/matplotlib_inline-0.2.2-py3-none-any.whl", hash = "sha256:3c821cf1c209f59fb2d2d64abbf5b23b67bcb2210d663f9918dd851c6da1fcf6", size = 9534, upload-time = "2026-05-08T17:33:32.055Z" }, ] +[[package]] +name = "maturin" +version = "1.14.1" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/e7/b3/addd877f871fb1860d46d3a4f206ecb10b946c85846805e6367631926fd3/maturin-1.14.1.tar.gz", hash = "sha256:9d6577a62cd08e0ceba7a0db06fb098e0c9b1b3429bad747a4f3a18215a1b3df", size = 369637, upload-time = "2026-06-19T05:19:49.774Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/f4/f0/97c5a5bd9c71653a066c0976a484eaaae50b9369557838a4176b7b0bdaa5/maturin-1.14.1-py3-none-linux_armv6l.whl", hash = "sha256:522292398945442cdafa9daeb2271b2340fbde57027b818f923f88eab04174f8", size = 10207496, upload-time = "2026-06-19T05:19:09.321Z" }, + { url = "https://files.pythonhosted.org/packages/fe/83/294bca639b0e052f1e2f65199b3db258780c7d4e31408b934c9c974a1379/maturin-1.14.1-py3-none-macosx_10_12_x86_64.macosx_11_0_arm64.macosx_10_12_universal2.whl", hash = "sha256:ffe5ad71f21d1e6603c4dd75f7fee34adf5ed5ebcebb692886549888ebb329ed", size = 19680113, upload-time = "2026-06-19T05:19:13.43Z" }, + { url = "https://files.pythonhosted.org/packages/43/b6/79c881410a3b1c187f7eb3d407aecae646c6a4433d630d72200359015e83/maturin-1.14.1-py3-none-macosx_10_12_x86_64.whl", hash = "sha256:f3306078070c1508fd715b9116070cbcaff5959024272a9f1e6f5cb29768b86c", size = 10169205, upload-time = "2026-06-19T05:19:16.615Z" }, + { url = "https://files.pythonhosted.org/packages/93/9d/44b6f26dcb7f7a04c5501ac2dbb6ca1490150682baa525ca5860504f9eab/maturin-1.14.1-py3-none-manylinux_2_12_i686.manylinux2010_i686.musllinux_1_1_i686.whl", hash = "sha256:cd457cd88961156e26379e1155bd287cc0ec1c8b2f1582b0660fb31b87c8842d", size = 10188098, upload-time = "2026-06-19T05:19:19.736Z" }, + { url = "https://files.pythonhosted.org/packages/1a/bd/9c0d5d6983905ce2c9edaa073a7e89355a9cf7f396988e05d32f1c37785d/maturin-1.14.1-py3-none-manylinux_2_12_x86_64.manylinux2010_x86_64.musllinux_1_1_x86_64.whl", hash = "sha256:dfc54ae32e6fcb18302193ab9a30b0b25eefffba994ae13238974805533ef75e", size = 10627576, upload-time = "2026-06-19T05:19:22.713Z" }, + { url = "https://files.pythonhosted.org/packages/e5/33/b096412bd6a7cb399652b260666f901adf88a687181a6dbd6a3f89f0a94e/maturin-1.14.1-py3-none-manylinux_2_17_aarch64.manylinux2014_aarch64.musllinux_1_1_aarch64.whl", hash = "sha256:a131d912b5267e640bc96d70f4914e10590aed64082ec9abacba7cea52004224", size = 10085181, upload-time = "2026-06-19T05:19:25.69Z" }, + { url = "https://files.pythonhosted.org/packages/56/8d/08c3bf469c38a23c9e6c877e338193001eb604d010fedc08341974e38528/maturin-1.14.1-py3-none-manylinux_2_17_armv7l.manylinux2014_armv7l.musllinux_1_1_armv7l.whl", hash = "sha256:be18fc568fb76884c0205456336892a75105ec398e6b667cd777c6268bd06d69", size = 10026363, upload-time = "2026-06-19T05:19:28.904Z" }, + { url = "https://files.pythonhosted.org/packages/3a/a4/c4d1a92839f8745ab4aab988a7db884a79d6d710bd3b286fcf9316dece1a/maturin-1.14.1-py3-none-manylinux_2_17_ppc64le.manylinux2014_ppc64le.musllinux_1_1_ppc64le.whl", hash = "sha256:994a0c8ba3ad8a92b3a9ee1b02645d200d610216b15cff5102b0fe65e8e08666", size = 13321347, upload-time = "2026-06-19T05:19:32.411Z" }, + { url = "https://files.pythonhosted.org/packages/b3/fa/170f04624d03fd07d2a8b1b67de83a127af93aef9eaa425839553347297b/maturin-1.14.1-py3-none-manylinux_2_17_s390x.manylinux2014_s390x.whl", hash = "sha256:be80866363e605d137991b491a741a84cde9ae350183c4c85f49690ca9aaaa65", size = 10877609, upload-time = "2026-06-19T05:19:35.448Z" }, + { url = "https://files.pythonhosted.org/packages/61/ad/1ae2e1d0ded282bf2c55ac13f0811d87deb425e200ae64a15785675dede9/maturin-1.14.1-py3-none-manylinux_2_31_riscv64.musllinux_1_1_riscv64.whl", hash = "sha256:5282dffd4b539d2be245f4e5b1a5ab6bc1033b58f4a4872f5833f9d43c954aa4", size = 10417316, upload-time = "2026-06-19T05:19:38.28Z" }, + { url = "https://files.pythonhosted.org/packages/fb/27/bf677183920718da49cd7982d6a3ffc440aad8919329f571d189f81b7bdf/maturin-1.14.1-py3-none-win32.whl", hash = "sha256:1a04de0a20188f95c721b5702eed18140bdcccb28c386797093eca3f62f4d4e0", size = 8931293, upload-time = "2026-06-19T05:19:41.183Z" }, + { url = "https://files.pythonhosted.org/packages/63/4b/585adeb9167b08d3cdff0032a938b0e72655c92003df4f52c3f696a1bcc2/maturin-1.14.1-py3-none-win_amd64.whl", hash = "sha256:3c9f94640ecc4895e94abaf834a0684430032c865b2748a36c12461fd9252fdd", size = 10314067, upload-time = "2026-06-19T05:19:44.389Z" }, + { url = "https://files.pythonhosted.org/packages/51/d4/dac8c0720ae246be1700afb6fbdbbea20fe35b13f6570b2f70faa005df77/maturin-1.14.1-py3-none-win_arm64.whl", hash = "sha256:15cea8fcb3ba47dd636f50092bb34baea8b04ac777392f23e6bf8a9a61efb894", size = 9718943, upload-time = "2026-06-19T05:19:47.49Z" }, +] + [[package]] name = "more-itertools" version = "11.1.0" From aafda28d5fda8d86f617837652d15b37cf673f81 Mon Sep 17 00:00:00 2001 From: SkyeAv Date: Tue, 28 Jul 2026 13:26:41 -0700 Subject: [PATCH 2/9] ci: stop ruff format from touching Markdown astral-sh/ruff-action@v3 floats to the latest ruff, now 0.16.0 (pyproject pins only a floor, >=0.14.14). Ruff 0.16.0 formats Python code fences inside *.md, which overlaps with the dedicated rumdl Markdown check and was failing 'Check Python formatting with ruff' on every open PR. Exclude *.md from ruff format so ruff stays scoped to Python and Markdown formatting CI is deterministic across ruff releases. --- pyproject.toml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/pyproject.toml b/pyproject.toml index 7935dfcb0..84d610865 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -89,6 +89,13 @@ ignore = [ fixable = ["ALL"] unfixable = [] +[tool.ruff.format] +# Ruff 0.16.0 began formatting Python code fences inside Markdown, which overlaps with the +# dedicated rumdl Markdown check and broke CI when astral-sh/ruff-action@v3 floated to 0.16.0 +# (pyproject pins only a floor, >=0.14.14). Keep ruff's formatter scoped to Python source and +# leave Markdown to rumdl so formatting CI stays deterministic across ruff releases. +exclude = ["*.md"] + [tool.snakefmt] line_length = 120 include = '\.snakefile$|^Snakefile' From 3ba024a5608f6cd00ca3d5a3f460e5024d132d44 Mon Sep 17 00:00:00 2001 From: Gaurav Vaidya Date: Wed, 29 Jul 2026 02:39:16 -0400 Subject: [PATCH 3/9] Name the extension _accel, and guard against a stale build Renames the module from `rs` to `_accel`: the name should say what it is (a compiled accelerator) rather than what it is written in, and while the module is empty this rename is as cheap as it will ever be. Adds `src/accel.py` as the only thing that imports the compiled module, because the two failure modes need opposite handling and neither is obvious at the call site: - A *missing* extension falls back to Python and logs at INFO. Every snakefile does a top-level `import src.foo` at DAG-parse time, so raising would take down all 245 rules for a contributor without a Rust toolchain, for a reviewer, and for a fork's CI -- not just the rules that would have used Rust. AGENTS.md's "a log warning is not a control" is about wrong output; a slower path emitting identical bytes is not that. - A *stale* extension raises. The extension is installed editable, so the compiled artifact sits in the checkout at src/_accel.*.so and a `git pull` does not rebuild it. ABI_VERSION in rust/src/lib.rs is compared against _REQUIRED_ABI_VERSION at import -- i.e. at DAG-parse time -- so a stale build fails in the first second rather than partway through a 12h rule. The check is a separate function so it can be tested against a stub; building a genuinely stale extension inside a unit test would mean compiling Rust. BABEL_DISABLE_RUST=1 forces the Python path, which is what makes an A/B measurement possible in one checkout. An environment variable rather than a config.yaml entry: which of two byte-identical implementations runs has no user-facing meaning, and config.yaml is threaded into Snakemake params and output paths where changing it could perturb a running DAG. Precedent is BABEL_DUCKDB_TEMP_DIR. Also: rust/Cargo.toml's version becomes 0.0.0 (maturin takes the distribution version from pyproject.toml -- the wheel is still babel_pipeline-1.17-cp311-abi3 -- and nothing publishes this crate, so a second version number is only a thing to forget to bump); rust-version declares pyo3 0.23's 1.63 floor; rust-toolchain.toml pins the channel; and `[tool.maturin] exclude` stops the four per-directory CLAUDE.md agent-instruction files shipping to anyone who installs the wheel. Verified by rebuilding: the wheel now holds src/_accel.abi3.so, src/_accel.pyi, 98 .py files and the 19 .snakefiles, and no CLAUDE.md. Co-Authored-By: Claude Opus 5 --- .gitignore | 8 +-- pyproject.toml | 10 ++-- rust-toolchain.toml | 5 ++ rust/Cargo.lock | 2 +- rust/Cargo.toml | 11 +++- rust/src/lib.rs | 35 ++++++++++--- src/_accel.pyi | 9 ++++ src/accel.py | 82 ++++++++++++++++++++++++++++++ tests/test_accel.py | 120 ++++++++++++++++++++++++++++++++++++++++++++ 9 files changed, 266 insertions(+), 16 deletions(-) create mode 100644 rust-toolchain.toml create mode 100644 src/_accel.pyi create mode 100644 src/accel.py create mode 100644 tests/test_accel.py diff --git a/.gitignore b/.gitignore index adbc5b9c8..f01bec5e3 100644 --- a/.gitignore +++ b/.gitignore @@ -39,7 +39,7 @@ htmlcov/ # maturin / Rust build artifacts rust/target/ -# native extension dropped into src/ by `maturin develop` (don't commit compiled binaries) -src/rs.*.so -src/rs.*.pyd -src/rs.*.dylib +# native extension dropped into src/ by an editable install (don't commit compiled binaries) +src/_accel*.so +src/_accel*.pyd +src/_accel*.dylib diff --git a/pyproject.toml b/pyproject.toml index 84d610865..b72bec371 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -61,11 +61,15 @@ build-backend = "maturin" [tool.maturin] # The importable Python package is literally named `src` (flat layout). A dotted module-name -# places the compiled extension at `src/rs.*` and tells maturin the package to bundle is `src` -# (auto-resolved to the repo root since src-layout detection won't fire). -module-name = "src.rs" +# places the compiled extension at `src/_accel.*` and tells maturin the package to bundle is `src` +# (auto-resolved to the repo root since src-layout detection won't fire). Import it through +# src/accel.py, never directly -- see docs/Rust.md. +module-name = "src._accel" # Rust source lives under rust/ so Cargo never collides with the Python `src/` dir. manifest-path = "rust/Cargo.toml" +# maturin bundles the whole `src` tree, which otherwise ships the per-directory agent instruction +# files to anyone who installs the wheel. +exclude = ["**/CLAUDE.md"] # Linting/formatting configuration diff --git a/rust-toolchain.toml b/rust-toolchain.toml new file mode 100644 index 000000000..75a71b56b --- /dev/null +++ b/rust-toolchain.toml @@ -0,0 +1,5 @@ +# Pin the channel, not an exact version: nothing here publishes a wheel, so the value of pinning is +# that everyone's cargo comes from the same place rather than from whatever a distro packages. +# The hard floor is `rust-version` in rust/Cargo.toml. +[toolchain] +channel = "stable" diff --git a/rust/Cargo.lock b/rust/Cargo.lock index 76ba98f35..0f465a8ff 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -10,7 +10,7 @@ checksum = "f2032f911046de80f0a198e0901378627c33f59ea0ac00e363d481118bd70a53" [[package]] name = "babel-pipeline" -version = "1.17.0" +version = "0.0.0" dependencies = [ "pyo3", ] diff --git a/rust/Cargo.toml b/rust/Cargo.toml index 426067dda..5b8fd82ba 100644 --- a/rust/Cargo.toml +++ b/rust/Cargo.toml @@ -1,10 +1,17 @@ [package] name = "babel-pipeline" -version = "1.17.0" # valid SemVer (3 parts); published version is "1.17" from root pyproject +# Not the distribution version: maturin takes that from the root pyproject.toml (the built wheel is +# babel_pipeline--cp311-abi3-*.whl), and nothing publishes this crate to +# crates.io. Kept at 0.0.0 so there is no second version number to remember to bump. +version = "0.0.0" edition = "2021" +# The floor pyo3 0.23 itself requires. Declared so a too-old cargo fails with a clear MSRV message +# rather than a syntax error from somewhere inside a dependency. Note this binds rustup-managed +# toolchains only -- see docs/Rust.md on why the Dockerfile installs rustup rather than apt cargo. +rust-version = "1.63" [lib] -name = "rs" # matches the #[pymodule] fn + module-name's last component +name = "_accel" # matches the #[pymodule] fn + module-name's last component crate-type = ["cdylib"] [dependencies] diff --git a/rust/src/lib.rs b/rust/src/lib.rs index 0db123630..fa53382a8 100644 --- a/rust/src/lib.rs +++ b/rust/src/lib.rs @@ -1,13 +1,36 @@ -//! Placeholder native extension for babel-pipeline. +//! Native accelerators for babel-pipeline, importable as `from src import _accel`. //! -//! The build backend is maturin; this empty module exists only so the project builds as a -//! mixed Rust/Python package. Real functionality will land in a follow-up PR. +//! Every function here must have a byte-identical Python implementation that stays in the tree as +//! the reference, and a test asserting the two agree (see `src/accel.py` and docs/Rust.md). Rust is +//! an optimisation, never the only way to produce a given output: `src/accel.py` falls back to the +//! Python implementation when this extension is missing, because every snakefile imports `src.*` at +//! DAG-parse time and a hard import failure would take down all 245 rules for anyone without a +//! Rust toolchain. +//! +//! **The FFI boundary is coarse, and that is a rule, not a preference.** A `#[pyfunction]` here +//! takes a file path and returns the whole parsed result; it never takes one row. Crossing pyo3 +//! once per CURIE costs more than the Python it would replace, so a per-row entry point would be +//! slower while looking like an optimisation. Functions open their own files so there is no entry +//! point that *could* accept a row. +//! +//! Real accelerators land once the Python `read_concord_file` they mirror is on main; this module +//! currently exports only the ABI version that guards against a stale build. use pyo3::prelude::*; -/// Empty pyo3 module, importable as `from src import rs`. Name must match the last component -/// of `[tool.maturin] module-name` ("src.rs" → `rs`). +/// Bumped by hand whenever a function's signature or semantics change, in the same commit. +/// +/// `src/accel.py` compares this against its own `_REQUIRED_ABI_VERSION` at import time and raises +/// if they disagree. That matters because the extension is installed editable: the compiled +/// artifact sits in the checkout at `src/_accel.*.so` and a `git pull` does not rebuild it, so +/// without this check a stale binary would be used silently for a 12-hour rule. +/// +// ponytail: a hand-bumped integer. Move to a build-time hash of this crate's sources if anyone +// forgets to bump it twice. +const ABI_VERSION: u32 = 1; + #[pymodule] -fn rs(_m: &Bound<'_, PyModule>) -> PyResult<()> { +fn _accel(m: &Bound<'_, PyModule>) -> PyResult<()> { + m.add("ABI_VERSION", ABI_VERSION)?; Ok(()) } diff --git a/src/_accel.pyi b/src/_accel.pyi new file mode 100644 index 000000000..2f543555d --- /dev/null +++ b/src/_accel.pyi @@ -0,0 +1,9 @@ +"""Type stub for the compiled Rust extension built from rust/src/lib.rs. + +Hand-written and hand-maintained: there is no mypy in this repo, so nothing enforces that this +matches the Rust. It exists so a reader who does not read Rust can see what the module offers. +Import through src/accel.py rather than importing this module directly. +""" + +# Bumped in lockstep with ABI_VERSION in rust/src/lib.rs; see src/accel.py. +ABI_VERSION: int diff --git a/src/accel.py b/src/accel.py new file mode 100644 index 000000000..327880687 --- /dev/null +++ b/src/accel.py @@ -0,0 +1,82 @@ +"""Access to the optional Rust accelerators, with the Python implementations as the reference. + +Babel's expensive rules are CPU-bound single-threaded Python (`generate_pubmed_concords` alone is +20 h at 99.7% CPU in the babel-1.18 benchmarks), so some of them will get Rust implementations. +This module is the one place that decides whether a Rust implementation is available and safe to +use. Nothing else should import ``src._accel`` directly. + +Three rules govern how Rust is used here, and each exists for a specific reason: + +**Rust is an accelerator, never the only implementation.** Every accelerated function keeps its +Python version in the tree, and a unit test asserts the two produce identical output. That is what +makes a Rust port reviewable: the Python is the specification, and the test is the proof the Rust +matches it. It also means a differential test can be written before the Rust exists. + +**A missing extension falls back to Python; it does not raise.** Every snakefile does a top-level +``import src.foo`` at DAG-parse time, so an ImportError here would take down all 245 rules for a +contributor without a Rust toolchain, for a reviewer, and for a fork's CI. AGENTS.md's "a log +warning is not a control" is about *wrong output*; a slower path that produces identical bytes is +not that. The active implementation is logged once, at INFO, so it lands in the SLURM job log. + +**A stale extension raises.** That is a bug, not a configuration. The extension is installed +editable -- the compiled artifact lives in the checkout at ``src/_accel.*.so`` -- so ``git pull`` +does not rebuild it and a stale binary would otherwise be used silently. The check runs at import +of this module, i.e. at DAG-parse time, so a stale build fails in the first second of a run rather +than eleven hours into a 12-hour rule. + +Set ``BABEL_DISABLE_RUST=1`` to force the Python path (for an A/B measurement, or to rule the +extension out while debugging). This is an environment variable rather than a ``config.yaml`` +entry deliberately: which of two byte-identical implementations runs is an implementation detail +with no user-facing meaning, and ``config.yaml`` is threaded into Snakemake ``params`` and output +paths where changing it risks perturbing the DAG of a running build. Precedent: +``BABEL_DUCKDB_TEMP_DIR`` in ``src/exporters/duckdb_exporters.py``. +""" + +import os + +from src.util import get_logger + +logger = get_logger(__name__) + +# Must equal ABI_VERSION in rust/src/lib.rs. Both are bumped by hand, in the same commit, whenever +# an accelerated function's signature or semantics change. +_REQUIRED_ABI_VERSION = 1 + + +def check_abi_version(compiled, required=_REQUIRED_ABI_VERSION): + """Return ``compiled`` if its ABI_VERSION matches ``required``, else raise RuntimeError. + + Separate from the import below so it can be tested against a stub module: building a genuinely + stale extension to exercise this would mean compiling Rust inside a unit test. + """ + found = getattr(compiled, "ABI_VERSION", None) + if found != required: + raise RuntimeError( + f"The compiled Rust extension src/_accel is stale: it reports ABI_VERSION {found!r}, " + f"but this checkout needs {required!r}. The extension is installed editable, so " + f"pulling new Rust source does not rebuild it. Run " + f"`uv sync --reinstall-package babel-pipeline`, or set BABEL_DISABLE_RUST=1 to run " + f"the Python implementations instead." + ) + return compiled + + +# Set to the compiled module when it is present, importable and current; None otherwise. Callers +# should treat it as "use the Rust path if this is not None". +accel = None + +if os.environ.get("BABEL_DISABLE_RUST"): + logger.info("BABEL_DISABLE_RUST is set; using the Python implementations.") +else: + try: + from src import _accel as _compiled + except ImportError: + # Expected whenever the extension has not been built. Not an error: see the module + # docstring on why this must not take down the whole DAG. + logger.info( + "The Rust extension src/_accel is not built; using the Python implementations. " + "Build it with `uv sync` (see docs/Rust.md)." + ) + else: + accel = check_abi_version(_compiled) + logger.info("Using the Rust extension src/_accel (ABI_VERSION %s).", accel.ABI_VERSION) diff --git a/tests/test_accel.py b/tests/test_accel.py new file mode 100644 index 000000000..e1adff4fd --- /dev/null +++ b/tests/test_accel.py @@ -0,0 +1,120 @@ +"""Unit tests for src/accel.py, the gateway to the optional Rust extension. + +src/accel.py decides whether the compiled extension is available and safe to use. Its three +branches each have a consequence that is expensive to discover late: + +- a missing extension must fall back to Python, because every snakefile imports `src.*` at + DAG-parse time and raising would take down all 245 rules for anyone without a Rust toolchain; +- a stale extension must raise, because the extension is installed editable and a `git pull` does + not rebuild it, so a silent stale binary could run for a 12-hour rule; +- `BABEL_DISABLE_RUST` must force the Python path, which is what makes an A/B measurement possible. + +The module resolves availability at import time, so the fallback branches are tested by reloading +it under a patched environment. The staleness check is tested directly against stub modules -- +building a genuinely stale extension would mean compiling Rust inside a unit test. +""" + +import builtins +import importlib +import types + +import pytest + +import src.accel + + +def reload_accel(monkeypatch, *, disable=None, hide_extension=False): + """Re-import src.accel under a patched environment and return the fresh module.""" + monkeypatch.delenv("BABEL_DISABLE_RUST", raising=False) + if disable is not None: + monkeypatch.setenv("BABEL_DISABLE_RUST", disable) + + if hide_extension: + real_import = builtins.__import__ + + def fail_on_accel(name, globals=None, locals=None, fromlist=(), level=0): + if fromlist and "_accel" in fromlist: + raise ImportError("no compiled extension in this test") + return real_import(name, globals, locals, fromlist, level) + + monkeypatch.setattr(builtins, "__import__", fail_on_accel) + + return importlib.reload(src.accel) + + +@pytest.fixture(autouse=True) +def restore_accel(): + """Leave src.accel in its real state, so reloads here can't leak into other tests.""" + yield + importlib.reload(src.accel) + + +def stub_extension(version): + """A stand-in for the compiled module, carrying just the attribute the guard reads.""" + module = types.ModuleType("src._accel") + if version is not None: + module.ABI_VERSION = version + return module + + +# FALLING BACK TO PYTHON + + +@pytest.mark.unit +def test_a_missing_extension_falls_back_instead_of_raising(monkeypatch): + """Without a compiled extension, importing src.accel must succeed and expose accel = None. + + Raising here would break every Snakemake target for a contributor with no Rust toolchain, not + just the rules that would have used Rust. + """ + assert reload_accel(monkeypatch, hide_extension=True).accel is None + + +@pytest.mark.unit +def test_babel_disable_rust_forces_the_python_path(monkeypatch): + """BABEL_DISABLE_RUST=1 should select the Python implementations even when the extension is + built, so the two can be timed against each other in one checkout.""" + assert reload_accel(monkeypatch, disable="1").accel is None + + +# GUARDING AGAINST A STALE BUILD + + +@pytest.mark.unit +def test_a_matching_abi_version_is_accepted(): + """The guard returns the module untouched when the versions agree.""" + stub = stub_extension(src.accel._REQUIRED_ABI_VERSION) + assert src.accel.check_abi_version(stub) is stub + + +@pytest.mark.unit +@pytest.mark.parametrize( + "version,description", + [ + (src.accel._REQUIRED_ABI_VERSION + 1, "extension newer than the checkout"), + (src.accel._REQUIRED_ABI_VERSION - 1, "extension older than the checkout"), + (None, "extension predates ABI_VERSION existing at all"), + ], +) +def test_a_mismatched_abi_version_raises_with_a_rebuild_instruction(version, description): + """Any mismatch must raise, and the message must say how to fix it. + + The rebuild command is asserted because that is the whole point of the error: the person who + hits it is mid-run and needs the fix, not a diagnosis. + """ + assert description # names the case in the failure output + with pytest.raises(RuntimeError, match="uv sync --reinstall-package babel-pipeline"): + src.accel.check_abi_version(stub_extension(version)) + + +@pytest.mark.unit +def test_the_built_extension_matches_the_version_python_expects(): + """If the extension is built, its ABI_VERSION must be the one this checkout expects. + + Both constants are bumped by hand in the same commit. Failing in CI after a Rust change means + the bump was forgotten; failing locally means the checkout needs + `uv sync --reinstall-package babel-pipeline`. + """ + if src.accel.accel is None: + pytest.skip("compiled extension not built") + assert src.accel.accel.ABI_VERSION == src.accel._REQUIRED_ABI_VERSION From 27e448b23c2103dde1dc876524ecfd69d08268fa Mon Sep 17 00:00:00 2001 From: Gaurav Vaidya Date: Wed, 29 Jul 2026 02:39:36 -0400 Subject: [PATCH 4/9] Make the Rust toolchain a declared prerequisite, not an accident Since the maturin swap, `[tool.uv] package = true` means every `uv run` and `uv sync` builds the project, and building the project invokes cargo. Three places had not caught up: - check-formatting.yml's snakefmt job runs `uv run`, with no toolchain step. It would have passed by accident, because ubuntu-latest happens to ship Rust -- which is worse than failing, since it makes a lint job depend on the runner image. Fixed by not building at all: snakefmt does not need babel-pipeline installed, so `uv run --no-project --with snakefmt`. - The Dockerfile installed Debian bookworm's cargo, which is 1.63 -- exactly pyo3 0.23's minimum. It builds today and breaks on the next pyo3 bump with an error that reads as unrelated. Swapped for rustup, which also honours rust-toolchain.toml (apt's cargo ignores it). - Nothing in slurm/, kubernetes/ or docs/ mentioned Rust at all, while `uv sync --frozen` on the Hatteras login node now fails outright without a toolchain -- before Snakemake starts. slurm/README.md now says so, and notes that uv's own fallback downloads ~600 MB into a cache directory the UV_CACHE_DIR override in run-babel-on-slurm.sh does not cover. Adds a cargo fmt/clippy job mirroring the one-linter-per-job structure of the other three, so "all four linters checked in CI" does not quietly become four of five languages, plus Swatinem/rust-cache so every PR does not rebuild pyo3 from scratch. docs/Rust.md is the reference: why targets come from a run's benchmark: TSVs rather than from reading code, why the FFI boundary is one call per file and never one per row, the fallback and staleness contracts, how to build in each environment, and what PR #588's 19 standalone binaries got wrong. Co-Authored-By: Claude Opus 5 --- .github/workflows/check-formatting.yml | 19 +++- .github/workflows/test.yml | 6 ++ AGENTS.md | 13 ++- Dockerfile | 12 ++- docs/Rust.md | 139 +++++++++++++++++++++++++ slurm/README.md | 8 ++ 6 files changed, 193 insertions(+), 4 deletions(-) create mode 100644 docs/Rust.md diff --git a/.github/workflows/check-formatting.yml b/.github/workflows/check-formatting.yml index 60e093131..670184ad1 100644 --- a/.github/workflows/check-formatting.yml +++ b/.github/workflows/check-formatting.yml @@ -24,7 +24,24 @@ jobs: - uses: actions/checkout@v7 - name: Install the latest version of uv uses: astral-sh/setup-uv@v7 - - run: uv run snakefmt --check --compact-diff . + # --no-project: snakefmt does not need babel-pipeline installed, and a plain `uv run` here + # would sync the project, which since the maturin swap means invoking cargo. That left a lint + # job depending on the runner image happening to ship a Rust toolchain. + - run: uv run --no-project --with snakefmt snakefmt --check --compact-diff . + + check-formatting-with-cargo: + name: Check Rust formatting and lints with cargo + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + - uses: dtolnay/rust-toolchain@stable + with: + components: rustfmt, clippy + - uses: Swatinem/rust-cache@v2 + with: + workspaces: rust + - run: cargo fmt --manifest-path rust/Cargo.toml --check + - run: cargo clippy --manifest-path rust/Cargo.toml --all-targets -- -D warnings check-formatting-with-rumdl: name: Check Markdown formatting with rumdl diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 958a79102..65b073e15 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -18,6 +18,9 @@ jobs: with: python-version: "3.11" - uses: dtolnay/rust-toolchain@stable + - uses: Swatinem/rust-cache@v2 + with: + workspaces: rust - run: uv sync --frozen - run: PYTHONPATH=. uv run pytest --collect-only -q - run: PYTHONPATH=. uv run pytest -m unit --no-cov -q @@ -36,5 +39,8 @@ jobs: with: python-version: "3.11" - uses: dtolnay/rust-toolchain@stable + - uses: Swatinem/rust-cache@v2 + with: + workspaces: rust - run: uv sync --frozen - run: PYTHONPATH=. uv run pytest --network -m "unit or network or slow" --no-cov -q diff --git a/AGENTS.md b/AGENTS.md index 6be01db95..ae11e200e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -70,7 +70,7 @@ Memory-hungry tests also carry a parametrized `min_memory_gb(n)` guard (register - `docs/Testing.md` — testing strategy: cadence per environment (per-PR, nightly, weekly, pre-release), GitHub Actions vs HPC self-hosted runner trade-offs, and other strategies. -### Linting (all four checked in CI on PRs) +### Linting (all five checked in CI on PRs) ```bash uv run ruff check # Python lint @@ -81,6 +81,8 @@ uv run snakefmt --check --compact-diff . # Snakemake format check uv run snakefmt . # Snakemake auto-fix uv run rumdl check . # Markdown lint uv run rumdl fmt . # Markdown auto-fix +cargo fmt --manifest-path rust/Cargo.toml # Rust format +cargo clippy --manifest-path rust/Cargo.toml # Rust lint ``` ### Configuration @@ -260,6 +262,15 @@ ingest is in `docs/Development.md` ("Enhancing a data source ingest"); datahandl do and any non-obvious behavior. Name functions for what they do — `fetch_*` (not `get_*`) when the call hits the network. +- **Rust accelerators** — a `#[pyfunction]` takes a file path and returns the whole parsed result; + never export one that is called once per row, because crossing pyo3 per CURIE costs more than the + Python it replaces. Every accelerated function keeps its Python implementation as the reference, + with a test asserting the two agree, and is reached through `src/accel.py` (never `src._accel` + directly) so a missing extension falls back rather than breaking DAG parsing for all 245 rules. + Pick targets from a run's `benchmark:` TSVs, not by reading code — see + [`docs/Rust.md`](docs/Rust.md), which also records the three targets chosen that way that turned + out to be pure-Python bugs. + ## Debugging Prefer invoking this repo's existing download code over a fresh API lookup when investigating a diff --git a/Dockerfile b/Dockerfile index 96732ba49..2757a86f1 100644 --- a/Dockerfile +++ b/Dockerfile @@ -37,8 +37,16 @@ RUN adduser --home ${ROOT} --uid 1000 nru RUN mkdir -p ${ROOT} WORKDIR ${ROOT} -# Rust toolchain — required because the build backend is maturin (compiles a native extension). -RUN apt-get update && apt-get install -y cargo +# Rust toolchain — required because the build backend is maturin, which compiles a native +# extension on every `uv sync`. Installed via rustup rather than `apt install cargo`: Debian +# bookworm ships cargo 1.63, which is *exactly* pyo3 0.23's minimum, so the next pyo3 bump would +# break this image with an error that reads as unrelated. rustup also honours rust-toolchain.toml, +# which apt's cargo ignores. +ENV RUSTUP_HOME=/usr/local/rustup CARGO_HOME=/usr/local/cargo +ENV PATH=${CARGO_HOME}/bin:${PATH} +RUN curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs \ + | sh -s -- -y --no-modify-path --profile minimal --default-toolchain stable \ + && chmod -R a+rX ${RUSTUP_HOME} ${CARGO_HOME} USER nru COPY --chown=nru . ${ROOT} diff --git a/docs/Rust.md b/docs/Rust.md new file mode 100644 index 000000000..485ac2333 --- /dev/null +++ b/docs/Rust.md @@ -0,0 +1,139 @@ +# Rust in Babel + +Babel's expensive rules are CPU-bound single-threaded Python. From the `babel-1.18` Snakemake +`benchmark:` TSVs, the five costliest rules all run at ~100% of one core: + +| Rule | Wall | CPU | `mean_load` | `max_rss` | +|------|------|-----|-------------|-----------| +| `generate_pubmed_concords` | 71,947 s (20 h) | 71,753 s | 99.7% | 31 GB | +| `protein_compendia` | 19,876 s | 19,844 s | 99.8% | 246 GB | +| `chemical_compendia` | 19,643 s | 19,569 s | 99.5% | 335 GB | +| `geneprotein_conflated_synonyms` | 15,719 s | 15,702 s | 99.8% | 98 GB | +| `gene_compendia` | 15,400 s | 15,178 s | 98.5% | 179 GB | + +The build backend is therefore [maturin](https://www.maturin.rs/), and the project builds as a +mixed Rust/Python package: a `cdylib` crate under `rust/` compiled into `src/_accel`, alongside the +ordinary Python in `src/`. + +## Rules for adding a Rust function + +**Measure first, from the benchmark TSVs.** Not from reading code for quadratic-looking shapes. +Three candidate targets picked that way all turned out to be wrong: `SynonymFilter.should_suppress` +looks like an O(labels × entries) scan but `input_data/obsolete_synonyms.yaml` holds three entries; +concord parsing looks like the obvious string-manipulation target but runs 159,283 lines in +0.148 s; and each genuinely expensive rule examined had a pure-Python defect (an eagerly evaluated +f-string feeding a suppressed `logger.debug`, a missing `elem.clear()`) worth more than any port. +Check `mean_load` before assuming a slow rule is CPU-bound — `get_ensembl` is 6,665 s wall but only +257 s CPU, so Rust would do nothing for it. + +**The FFI boundary is coarse.** A `#[pyfunction]` takes a file path and returns the whole parsed +result. It never takes one row. Crossing pyo3 once per CURIE costs more than the Python it +replaces, so a per-row entry point would be *slower* while looking like an optimisation. This is +enforced structurally: Rust functions open their own files, so there is no entry point that could +accept a row. + +**Keep the Python implementation as the reference.** Every accelerated function keeps its Python +version in the tree, and a unit test asserts the two produce identical output over the same input. +The Python is the specification; the test is the proof the Rust matches it. This is also what makes +the fallback below safe, and what lets a differential test be written before the Rust exists. + +**Bump `ABI_VERSION` in the same commit** as any change to an accelerated function's signature or +semantics — in both `rust/src/lib.rs` and `_REQUIRED_ABI_VERSION` in `src/accel.py`. See +"Staleness" below. + +## Using it from Python + +Import through [`src/accel.py`](../src/accel.py), never `src._accel` directly: + +```python +from src.accel import accel + +def read_something(path): + if accel is not None: + return accel.read_something(path) + return _read_something_python(path) +``` + +`accel` is the compiled module when it is present, importable and current, and `None` otherwise. + +**A missing extension falls back to Python rather than raising.** Every snakefile does a top-level +`import src.foo` at DAG-parse time, so an `ImportError` would take down all 245 rules for a +contributor without a Rust toolchain, for a reviewer, and for a fork's CI — not just the rules that +would have used Rust. AGENTS.md's "a log warning is not a control" is about *wrong output*; a slower +path producing identical bytes is not that. The active implementation is logged once at INFO, which +lands in the SLURM job log that `babel-slurm-errors` reads. + +**`BABEL_DISABLE_RUST=1` forces the Python path**, which is how you A/B a port in one checkout. It +is an environment variable rather than a `config.yaml` entry deliberately: which of two +byte-identical implementations runs is an implementation detail with no user-facing meaning, and +`config.yaml` is threaded into Snakemake `params` and output paths, where changing it risks +perturbing the DAG of a running build. Precedent: `BABEL_DUCKDB_TEMP_DIR`. + +## Staleness + +The extension is installed **editable**: the compiled artifact lands in the checkout at +`src/_accel.*.so` (gitignored), and that is the copy `PYTHONPATH=.` finds. A `git pull` that brings +new Rust source does **not** rebuild it, so without a guard a stale binary would be used silently +— potentially for a 12-hour rule. + +So `src/accel.py` compares the extension's `ABI_VERSION` against its own `_REQUIRED_ABI_VERSION` +and raises if they disagree. The check runs at import, i.e. at DAG-parse time, so a stale build +fails in the first second of a run. To fix one: + +```bash +uv sync --reinstall-package babel-pipeline +``` + +What the guard does not catch is editing the Rust body without bumping the constant. That is what +the differential tests are for — they compare against the Python reference regardless of where the +`.so` came from, which also covers a `.so` served from a stale uv or cargo cache in CI. + +## Building + +A Rust toolchain is a build prerequisite for **every** environment now, because `[tool.uv] package += true` means every `uv run` and `uv sync` builds the project, and building the project invokes +cargo. + +- **Locally:** `uv sync`. If `cargo` is not on `$PATH`, uv bootstraps a toolchain itself (via + `puccinialin`) rather than failing — convenient, but it silently downloads ~600 MB into a + platform cache directory that `UV_CACHE_DIR` does **not** cover. Installing rustup yourself + (`brew install rustup && rustup default stable`, or ) avoids that. +- **CI:** `dtolnay/rust-toolchain@stable` plus `Swatinem/rust-cache@v2` in `test.yml`. The + formatting workflow deliberately runs snakefmt with `uv run --no-project` so that a lint job does + not build the project at all. +- **Docker:** the image installs rustup rather than `apt install cargo`. Debian bookworm ships + cargo 1.63, which is *exactly* pyo3 0.23's minimum, so the next pyo3 bump would break the image + with an error that reads as unrelated. rustup also honours `rust-toolchain.toml`, which apt's + cargo ignores. +- **Hatteras:** `uv sync` runs once on the login node and compute nodes reuse the `.venv` from + `/projects`, so compilation happens in one place. Make sure a toolchain is available there before + a build — `uv sync --frozen` fails outright without one, before Snakemake starts. See + [`slurm/README.md`](../slurm/README.md). + +`rust-toolchain.toml` pins the channel (`stable`) rather than an exact version: nothing here +publishes a wheel, so the value of pinning is that everyone's cargo comes from the same place +rather than from whatever a distro packages. The hard floor is `rust-version` in `rust/Cargo.toml`. + +## Layout + +| Path | What | +|------|------| +| `rust/Cargo.toml` | the crate. `version` is `0.0.0` on purpose — maturin takes the distribution version from the root `pyproject.toml`, and nothing publishes this crate | +| `rust/src/lib.rs` | the `#[pymodule]`: module doc, `ABI_VERSION`, registrations. Functions go in their own modules, split from the first one | +| `src/accel.py` | the only thing that imports the compiled module | +| `src/_accel.pyi` | hand-written stub. There is no mypy here, so it enforces nothing; it exists so a reader who does not read Rust can see what the module offers | +| `rust-toolchain.toml` | channel pin | + +## History + +[PR #588](https://github.com/NCATSTranslator/Babel/pull/588) took a different approach — 19 +standalone binaries under `babel_io/src/bin/` invoked from Snakemake `shell:` blocks — and is worth +reading as a cautionary tale. It targeted datacollect label/synonym rules totalling roughly 1.5 h of +CPU, under 2% of the pipeline, two of them download-bound where Rust does nothing; the one binary +aimed at an expensive rule (`build_compendia.rs`) is a 72-line stub. 3,252 lines across 44 files, +unmerged. Hence the "measure first" and "one path, deep" rules above. + +The in-process pyo3 model that replaced it fits the codebase better regardless: 245 of Babel's +Snakemake rules are `run:` blocks with no `script:` directives and no subprocess boundaries, so an +importable extension needs no rule rewriting, whereas standalone binaries would mean converting +rules to `shell:` one at a time. diff --git a/slurm/README.md b/slurm/README.md index 91d84e758..adc1fbc5c 100644 --- a/slurm/README.md +++ b/slurm/README.md @@ -12,6 +12,14 @@ running it in a low-memory low-CPU node (by running `sbatch run-babel-on-slurm.s [run-babel-on-slurm.sh](./run-babel-on-slurm.sh)) works fine, and ensures that you don't get complaints from your cluster manager about long-running login node processes. +`uv sync` now needs a Rust toolchain on the login node: the build backend is maturin, which +compiles a native extension (see [`docs/Rust.md`](../docs/Rust.md)). Without `cargo` on `$PATH`, uv +downloads a toolchain itself into a platform cache directory that the `UV_CACHE_DIR` override in +[`run-babel-on-slurm.sh`](./run-babel-on-slurm.sh) does **not** cover, which on a quota'd home +directory is its own problem — so install rustup (or `module load` a Rust toolchain) first. Only +the login node needs it: `uv sync` runs once there and the compute nodes reuse the resulting +`.venv` from `/projects`. + ```bash # Activate the environment uv sync From cc0192d788ead75fb6ea0bf85241a6c5f68e113a Mon Sep 17 00:00:00 2001 From: SkyeAv Date: Wed, 29 Jul 2026 10:27:06 -0700 Subject: [PATCH 5/9] ci: pin Rust toolchain to 1.95.0 and freeze Docker uv sync Address Copilot review feedback on PR #975: - Add a checked-in rust-toolchain.toml pinning channel = "1.95.0" (the toolchain used locally) so local builds, CI, and any rustup-based environment compile the maturin/PyO3 extension with the same compiler. - Pin the CI action ref dtolnay/rust-toolchain@stable -> @1.95.0 in both test jobs so CI no longer tracks the moving stable channel; the ref and rust-toolchain.toml are cross-referenced to be bumped together. - Dockerfile: uv sync -> uv sync --frozen so the image installs exactly the committed uv.lock (no re-resolution), matching CI and keeping builds reproducible now that the step compiles Rust via maturin/cargo. --- .github/workflows/test.yml | 4 ++-- Dockerfile | 5 +++-- rust-toolchain.toml | 6 ++++++ 3 files changed, 11 insertions(+), 4 deletions(-) create mode 100644 rust-toolchain.toml diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 958a79102..253726362 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -17,7 +17,7 @@ jobs: - uses: astral-sh/setup-uv@v7 with: python-version: "3.11" - - uses: dtolnay/rust-toolchain@stable + - uses: dtolnay/rust-toolchain@1.95.0 # pinned; keep in sync with rust-toolchain.toml - run: uv sync --frozen - run: PYTHONPATH=. uv run pytest --collect-only -q - run: PYTHONPATH=. uv run pytest -m unit --no-cov -q @@ -35,6 +35,6 @@ jobs: - uses: astral-sh/setup-uv@v7 with: python-version: "3.11" - - uses: dtolnay/rust-toolchain@stable + - uses: dtolnay/rust-toolchain@1.95.0 # pinned; keep in sync with rust-toolchain.toml - run: uv sync --frozen - run: PYTHONPATH=. uv run pytest --network -m "unit or network or slow" --no-cov -q diff --git a/Dockerfile b/Dockerfile index 96732ba49..4a5bcb806 100644 --- a/Dockerfile +++ b/Dockerfile @@ -43,10 +43,11 @@ RUN apt-get update && apt-get install -y cargo USER nru COPY --chown=nru . ${ROOT} -# Install and run `uv sync` to install packages. +# Install uv and sync packages. `--frozen` installs exactly the committed uv.lock +# (no re-resolution), keeping the image reproducible and matching CI's `uv sync --frozen`. RUN pipx install uv ENV PATH="${ROOT}/.local/bin:${PATH}" -RUN uv sync +RUN uv sync --frozen # Our default entrypoint is to start the Babel run. ENTRYPOINT ["bash", "-c", "uv run snakemake --cores ${CORES}"] diff --git a/rust-toolchain.toml b/rust-toolchain.toml new file mode 100644 index 000000000..c62df1bcc --- /dev/null +++ b/rust-toolchain.toml @@ -0,0 +1,6 @@ +# Pin the Rust toolchain so local builds, CI, and any rustup-based environment all +# compile the maturin/PyO3 extension with the same compiler version. The CI action ref +# (dtolnay/rust-toolchain in .github/workflows/test.yml) is pinned to match this; bump +# both together when upgrading Rust. +[toolchain] +channel = "1.95.0" From a5acbfb894e1008e3cbcb859604a3cc16bffa35a Mon Sep 17 00:00:00 2001 From: SkyeAv Date: Thu, 30 Jul 2026 10:44:57 -0700 Subject: [PATCH 6/9] Say 'every rule' rather than a count that rots The comments in rust/src/lib.rs and src/accel.py said a hard import failure would 'take down all 245 rules'. The exact count is brittle (verified 244 run: + 26 shell: on main) and adds nothing: a top-level import failure at DAG-parse time takes down the whole parse, i.e. every rule. Wording it as 'every rule' cannot go stale. --- rust/src/lib.rs | 2 +- src/accel.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/rust/src/lib.rs b/rust/src/lib.rs index fa53382a8..c4e8f1e96 100644 --- a/rust/src/lib.rs +++ b/rust/src/lib.rs @@ -4,7 +4,7 @@ //! the reference, and a test asserting the two agree (see `src/accel.py` and docs/Rust.md). Rust is //! an optimisation, never the only way to produce a given output: `src/accel.py` falls back to the //! Python implementation when this extension is missing, because every snakefile imports `src.*` at -//! DAG-parse time and a hard import failure would take down all 245 rules for anyone without a +//! DAG-parse time and a hard import failure would take down every rule for anyone without a //! Rust toolchain. //! //! **The FFI boundary is coarse, and that is a rule, not a preference.** A `#[pyfunction]` here diff --git a/src/accel.py b/src/accel.py index 327880687..84fc72a7a 100644 --- a/src/accel.py +++ b/src/accel.py @@ -13,7 +13,7 @@ matches it. It also means a differential test can be written before the Rust exists. **A missing extension falls back to Python; it does not raise.** Every snakefile does a top-level -``import src.foo`` at DAG-parse time, so an ImportError here would take down all 245 rules for a +``import src.foo`` at DAG-parse time, so an ImportError here would take down every rule for a contributor without a Rust toolchain, for a reviewer, and for a fork's CI. AGENTS.md's "a log warning is not a control" is about *wrong output*; a slower path that produces identical bytes is not that. The active implementation is logged once, at INFO, so it lands in the SLURM job log. From f769b4ac9f918ce18e10e228c4320944fb718982 Mon Sep 17 00:00:00 2001 From: SkyeAv Date: Thu, 30 Jul 2026 10:55:05 -0700 Subject: [PATCH 7/9] Note in releases/README that the wheel now ships the .snakefiles The maturin swap (this branch) bundles all of src/* into the wheel, which includes the 19 .snakefile files; main's setuptools wheel shipped zero non-Python files, so an installed babel-pipeline previously could not run the pipeline. Record this accidental fix as a one-line general release note so the next release-notes author picks it up. --- releases/README.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/releases/README.md b/releases/README.md index b51dc8c7d..4813893fe 100644 --- a/releases/README.md +++ b/releases/README.md @@ -2,7 +2,9 @@ ## General releases -- TODO +- The `babel-pipeline` wheel now bundles the pipeline `.snakefile`s, so an installed package can + actually run the pipeline (previously it shipped no non-Python files) — maturin build-backend swap, + [PR #975](https://github.com/NCATSTranslator/Babel/pull/975). ## Translator-specific releases From 77839d6b39642d2a4e129bf00ed68cc1a26a983d Mon Sep 17 00:00:00 2001 From: SkyeAv Date: Thu, 30 Jul 2026 10:58:41 -0700 Subject: [PATCH 8/9] Reflow the release note to satisfy rumdl MD013 (100-col) --- releases/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/releases/README.md b/releases/README.md index 4813893fe..66ab01fb1 100644 --- a/releases/README.md +++ b/releases/README.md @@ -3,8 +3,8 @@ ## General releases - The `babel-pipeline` wheel now bundles the pipeline `.snakefile`s, so an installed package can - actually run the pipeline (previously it shipped no non-Python files) — maturin build-backend swap, - [PR #975](https://github.com/NCATSTranslator/Babel/pull/975). + actually run the pipeline (previously it shipped no non-Python files) — maturin build-backend + swap, [PR #975](https://github.com/NCATSTranslator/Babel/pull/975). ## Translator-specific releases From c3e0803ec8c663244cd7ea404803e210c0eebf3d Mon Sep 17 00:00:00 2001 From: SkyeAv Date: Thu, 30 Jul 2026 11:00:48 -0700 Subject: [PATCH 9/9] Pin the cargo lint job to Rust 1.95.0 to match rust-toolchain.toml The cargo fmt/clippy job used dtolnay/rust-toolchain@stable, which installed rustfmt+clippy for 'stable'. But rust-toolchain.toml pins channel=1.95.0, so when cargo ran it switched to 1.95.0 -- a toolchain without those components -- and failed with "'cargo-fmt' is not installed for the toolchain '1.95.0'". Pin the action to 1.95.0 (as test.yml already is) so the components are installed for the toolchain cargo actually uses. --- .github/workflows/check-formatting.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/check-formatting.yml b/.github/workflows/check-formatting.yml index 670184ad1..b770e63e4 100644 --- a/.github/workflows/check-formatting.yml +++ b/.github/workflows/check-formatting.yml @@ -34,7 +34,7 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v7 - - uses: dtolnay/rust-toolchain@stable + - uses: dtolnay/rust-toolchain@1.95.0 # pinned; keep in sync with rust-toolchain.toml with: components: rustfmt, clippy - uses: Swatinem/rust-cache@v2