feat: rebuildpy parity gate + Phase 1 loop module + schicluster-rs CLI - #2
Merged
Conversation
Pre-Phase-0 design under the rebuildpy protocol: extend schicluster_rs with native ports of every remaining numerical hot path in scHiCluster (loop calling, domain calling incl. a native TopDom dropping the rpy2/R dependency, compartment, embedding-matrix, cell-to-group merge), integrated as monkey-patches that leave upstream Python code unchanged. Records the dual-env parity flow (schicluster py3.6 reference, rebuild-rust py3.10 target, rpy env for the TopDom R ground truth), the per-output pre-registered parity gates, the Rust PyO3 surface, acceleration admissibility plan, and the rebuildpy artefact set the repo will grow. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…rectly The schicluster conda env has working R + rpy2 + the Matrix R package once activated (conda activate sets R_HOME). Confirmed: `hicluster domain` already runs end-to-end there, including the rpy2 → TopDom.R path. So the upstream TopDom reference comes from $PYTHON_REF_ENV like every other reference output — no separate rpy/Rscript env is needed. Drops the third env, gen_topdom_ref.R, and the three-env run_parity.sh. The cross-env dump-and-compare remains (schicluster_rs is abi3-py39 and the schicluster env is py 3.6, so a single-process import-both is still impossible) — just across two envs now. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ion) First of N execution plans for the loop / domain / compartment / embedding / merge port. Phase 0 only: provision rebuild-rust, add rebuildpy artefacts, commit the read-only data/manifest.yaml, ship convolve2d_mirror + parity gate, refactor sparse/banded helpers out of lib.rs into utils.rs. Loop / domain / compartment / embedding plans follow once Phase 0 lands. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…AUDIT/RECONSTRUCTION_REPORT) Pre-Phase-0 scaffolding for the loop / domain / compartment / embedding / merge port. DISCOVERY.md records the partial-port status and dep mapping; the other four are skeletons filled in later phases. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
One outputs[] block per ported function from the design spec §5, covering all phases (loop / domain / compartment / embedding). The threshold column is locked here and will not be tightened or loosened during the agent loop, per rebuildpy PROTOCOL.md §4. Phase 0 implements conv.convolved; later phases turn additional outputs green as their py_reference_driver.py / _run_candidate.py code lands. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
64x64 input + 7x7 donut kernel + scipy.ndimage.convolve(mode='mirror') reference, all packed into one .npz keyed by 'input', 'kernel', 'convolved'. The .npz is regenerated on demand and stays out of git; the synthesiser script is the canonical artefact. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
conftest.py adds REBUILDPY_DIR (default /large_storage/zhoulab/shengmao /rebuildpy) to sys.path so engine.parity_metrics is importable. parity_harness.py loads data/manifest.yaml and dispatches each outputs[] block to compute_parity / is_pass with the class-aware metric. test_exact_match.py parametrises one pytest case per output; missing reference/candidate dumps mark the case as skipped instead of failing, so the gate progressively turns green as phases land. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Run under $PYTHON_REF_ENV (schicluster env, py3.6). For now only emits the 'convolved' key, which echoes scipy.ndimage.convolve(mode='mirror') as already stored in the conv_small fixture. Later phases add one block per data/manifest.yaml output. Avoids 3.7+ syntax (`from __future__ import annotations`, PEP 585 subscripted generics) so the script runs in the schicluster env's Python 3.6. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Invoked under $RUST_TEST_ENV after `maturin develop --release`. Calls schicluster_rs.convolve2d_mirror on the conv_small fixture and dumps the result with the same JSON key the reference driver uses. Note: schicluster_rs.convolve2d_mirror is implemented in a later task of this plan; this commit only lands the driver scaffold. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Chains conda-run invocations: synthesizer + candidate dump + build in $RUST_TEST_ENV (default 'rebuild-rust', py3.10), py3.6-safe reference dump in $PYTHON_REF_ENV (default 'schicluster'), then the manifest-driven pytest gate. The synthesizer runs in $RUST_TEST_ENV rather than $PYTHON_REF_ENV because schicluster's py3.6 cannot parse `from __future__ import annotations`. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Behavior-preserving move of triplets_to_csr, csr_to_dense, mirror_index, band_w, band_idx, csr_to_banded, type Csr out of lib.rs into a new utils.rs module. Existing random_walk_cpu parity tests still pass unchanged (11 passed); this just makes room for the new conv / loop / domain modules without making lib.rs grow further. Renames _mirror_index -> mirror_index (vestigial leading underscore). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
General 2-D convolution matching scipy.ndimage.convolve(mode='mirror'), including the convolve (not correlate) kernel flip and origin=0 anchor at ((kh-1)//2, (kw-1)//2) of the flipped kernel. Mirror = reflect without repeating the edge (period 2*(n-1)) — shares mirror_index with the existing gaussian_filter_2d. Parallel across output rows (each row's reduction is fixed-order, so this is (E) exact vs a serial outer loop). PyO3 binding takes zero-copy PyReadonlyArray2<f32> and releases the GIL during compute. This is the shared primitive loop_bkg.py + loop_calling.py::scan_kernel will reuse — 5 convolutions per chrom in the loop pipeline. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Re-exports the Rust py_convolve2d_mirror as schicluster_rs.convolve2d_mirror with the standard f32-ascontiguous dance. Falls back to scipy.ndimage.convolve(mode='mirror') if the Rust extension wasn't built. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
(E) exact translation; row-parallel via rayon, fixed-order within row. Phase 0 parity gate clears (1 passed, 16 skipped in tests/test_exact_match.py). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Phase 0 of the loop / domain / compartment / embedding / merge port is in place: rebuildpy artefacts + read-only manifest + shared 2-D mirror convolution + cross-env parity gate (1 passed, 16 skipped, 0 failed). Adds pyyaml / cooler / h5py / maturin to the dev extra for the parity harness and reference / candidate drivers. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Ports the per-chrom numerical hot paths of scHiCluster's loop module:
- calculate_chrom_background_normalization -> rust/src/loop_bkg.rs
- merge_cells_for_single_chromosome -> rust/src/merge.rs
- loop_background -> rust/src/scan_kernels.rs
- find_summit -> rust/src/find_summit.rs
All five convolutions reuse Phase 0's convolve2d_mirror primitive.
merge accumulates in f64 BTreeMap for deterministic row-major emission.
The find_summit max-heap uses ascending-idx tie-break.
Python wrappers in python/schicluster_rs/loop.py keep cooler / .npz /
HDF5 I/O in Python; patch_schicluster() rebinds the upstream loop module
functions so every caller (CLI, snakemake, hicluster.domain) gets the
Rust kernels transparently. The shuffle=True path of loop_bkg falls back
to upstream Python (RNG; out of parity scope).
Parity gate (data/manifest.yaml read-only, 11 of 17 outputs now green):
conv.convolved deterministic-bounded 1e-6 (Phase 0)
loop_bkg.E deterministic-bounded 1e-6
loop_bkg.T deterministic-bounded 1e-6
merge.e_sum deterministic-bounded 1e-6
merge.e2_sum deterministic-bounded 1e-6
scan_kernels.bl/donut/h/v deterministic-bounded 1e-6
find_summit.idx ranked Jaccard ≥ 0.99
find_summit.sizes classification = 1.0 (idx-intersection aligned)
Skips (6): insulation + topdom × 2 (Phase 2), compartment × 2 (Phase 3),
embedding × 1 (Phase 4).
Other fixes:
- parity_harness: densify CSR-triplet JSON dicts; intersect find_summit
idx before aligning sizes; pad-to-common shape on mismatched densified
matrices.
- run_parity.sh: PYTHONNOUSERSITE=1 in the pytest step to avoid the
~/.local scipy shadow that bit Phase 0.
- reference driver: separate temp dirs for loop_bkg and merge so
loop_small.E.npz isn't picked up by the merge glob.
Version 0.2.0.dev0 -> 0.2.0.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…tefacts - README trimmed to install / tutorial / multi-process tuning / snakemake pointer / citation / license. Speed, accuracy, parity-gate status, layout and algorithm notes moved into docs/PERFORMANCE.md. - All rebuildpy protocol artefacts (DISCOVERY, MATH, ITERATION_LOG, AUDIT, RECONSTRUCTION_REPORT) moved from repo root into docs/. Git tracks them as renames so history is preserved. - python/schicluster_rs/loop.py promoted to a package (loop/__init__.py) and now ships a snakemake_template_loop.txt modelled on the upstream loop template, with patch_schicluster() prepended so every fanned-out rule transparently runs the Rust kernels. Drop-in replacement for upstream's schicluster/loop/snakemake_template_loop.txt. Parity gate unchanged: 11 passed, 6 skipped on test_exact_match. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Adds a thin entry-point script that pre-applies patch_schicluster()
and delegates argv to upstream's argparse. Users can swap `hicluster`
-> `schicluster-rs` in their pipelines (e.g. the workflow in
ProstateCancer/example_notebook/imputation.md) to route the per-chrom
numerical kernels through Rust transparently.
Two subcommands are explicitly whitelisted; everything else is rejected
with a pointer to upstream `hicluster`:
schicluster-rs filter-contact pure-IO blacklist filter (single
binary surface, no Rust speedup)
schicluster-rs prepare-impute snakemake jobs generator; fanned-out
hic-internal rules still need
schicluster_rs importable in each
worker to actually hit Rust
Adds [project.scripts] in pyproject.toml so maturin / pip install ships
the entry point. README gains a CLI section with the imputation.md
example calls. Parity gate unchanged (11 passed, 6 skipped).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…self-skip PyO3/maturin-action's `develop` subcommand calls `maturin develop --release`, which requires an active VIRTUAL_ENV / CONDA_PREFIX / .venv to install the extension into. setup-python@v5 doesn't create a venv, so the action errored with "Couldn't find a virtualenv or conda environment". Switch to `maturin build --release --out dist` (no env required) then `pip install --find-links dist schicluster-rs` into the setup-python interpreter. After the build fix, the next failure was tests/conftest.py raising SystemExit because REBUILDPY_DIR doesn't exist on CI runners (the rebuildpy kit isn't checked out). Make rebuildpy optional: conftest.py only injects the path when the dir exists, and test_exact_match.py guards `engine.parity_metrics` with pytest.importorskip so the manifest gate collects-and-skips as a single "skipped" module on rebuildpy-less envs (CI, fresh contributor machines). The legacy test_parity.py still runs in full. Verified locally: - REBUILDPY_DIR=/nonexistent pytest tests/ -> 1 passed, 11 skipped - rebuildpy on path pytest tests/ -> 12 passed, 16 skipped Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Brings the port from impute-only to loop calling + impute + CLI, under the rebuildpy protocol (pre-registered manifest, cross-env parity gate, iteration log).
data/manifest.yamlpre-registers gates for all 17 outputs across loop / domain / compartment / embedding / merge (read-only after this PR lands). Cross-env dump-and-compare harness: `schicluster` env (py3.6) runs the upstream reference; `rebuild-rust` env (py3.10) runs the Rust candidate; pytest in the candidate env asserts `engine.parity_metrics` per output. New shared 2-D primitive `convolve2d_mirror` matches `scipy.ndimage.convolve(mode='mirror')` semantics (kernel flip + reflect-no-repeat) — max abs err `1.79e-7` vs `1e-6` threshold.Parity gate
`bash tests/run_parity.sh` → 11 passed, 6 skipped
Test plan
Scope notes
🤖 Generated with Claude Code