Repository navigation
perf: stop the banded search running out of memory MAPPINGS, and make pooling a byte copy - #109
Merged
Merged
Conversation
…cross runs `run-experiment` refused `groups.window_groups > 1`, so a grouped search of several files meant one `mumdia run` per file and assembling the pooled rescore, the match between runs and the cross-run quantities by hand. It now routes each run's middle through `run_groups`: the run's competed table and chromatograms come back under the names the pooled stages already read, so the experiment-wide rescore, the per-source `run_psm_q`, MBR, per-run quant and the cross-run report are unchanged. `experiment.rt_library_scope` carries over. A grouped run adapts its bands rather than one library table, so the first run's `groups/` directory is what the rest reuse: each band takes that run's table for its own band, keeping the ids and row order that `fragment_offset` and the per-band seed views key on, and still fits its own per-run LOESS on top. Under `per_run` every run adapts its own bands as before. A run whose shared bands do not match its plan fails with the missing path named rather than searching a mismatched band. `run_groups` takes its manifest as an `Option`, because `run-experiment` keeps one experiment-level manifest rather than one per run. Measured on the fixture: a two-file grouped experiment plans three bands per run, pools each run to 288 competed rows and rescores 576 PSMs experiment-wide to 151 peptides at 1%, matching the ungrouped fixture experiment. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…lel) A grouped run searched its bands one after another, so a large machine sat idle: measured on a seven-file experiment with four runs in flight, basilisk was 97.6% idle with one process using 12 of 256 cores, because four bands (one per run) were all that was ever in flight, each using about three. Both per-band phases now run `groups.parallel` bands at a time: the slice-and-seed phase and the windows-extract-features-compete phase. Chunked rather than a free-running pool, because the chunk is exactly how many extraction working sets are resident, which is what banding exists to bound. Each band's manifest records are collected and merged in band order, so the manifest is what the sequential run wrote, and the artifacts are pooled in band order regardless of which band finishes first. The default stays 1, which is the previous behaviour exactly. Raise it after checking one band's peak RSS: on the 203M-precursor library at 63 bands the largest band took 39 GB and the median far less, so the low-m/z bands, which are both the largest and the slowest, set the ceiling. Byte-identical output: on the fixture, three bands searched three at a time reproduce the sequential grouped run's `psms_extracted`, `chromatograms`, `features`, `psms_competed`, `psms_scored` and `peptides.tsv` exactly. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ccumulator `plan` closed a band once its running total reached the target share and then reset the accumulator. Every band therefore overshoots a little, the overshoot compounds along the m/z axis, and the windows left at the end fall into the last band. Measured on the 8-12-mer immunopeptidomics library, whose first 14 isolation windows select nothing because the library starts at m/z 326: 63 bands asked for came back as 36, ranging from 3.23M to 37.89M precursors against a 3.2M target, one of them holding 28 of the 114 windows. That band alone drove a 233 GB extract peak with only six bands in flight, which is what made the experiment unschedulable. Cuts are now taken where the running total crosses each k/n share of the whole, so they cannot compound: each is measured against the same running total rather than a reset one. A regression test covers the shape that broke it, a leading run of empty windows followed by equal ones, and asserts every band asked for comes back, within 3x of each other, with every window placed exactly once. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A grouped run of the 8-12-mer library writes about 380 GB per file and the disk does 188 MB/s, so on a seven-file experiment the wall clock is partly the disk. Three changes, each measurable on its own: 1. A band's stages emit library-wide candidate ids. `extract` loads one band as a library whose rows are numbered 0..n and used to write those local numbers, so every downstream table only meant something next to the band's offset and pooling had to rewrite every id. Outputs now carry `local + Library::global_offset`, so pooling is a concatenation and a band's table can be read on its own. This is also what makes 2 and 3 simple. 2. The pooled `features.parquet` is not written. Nothing reads it: the competed table carries the feature columns, and the run-level copy was 55 GB per run. The per-band views of the pooled seed go with it, because features can now read the pooled seed directly, its ids matching the band's tables. 3. `MUMDIA_PARQUET_COMPRESSION=zstd` writes every artifact with zstd instead of snappy (`uncompressed` is also accepted). Both are read transparently whatever wrote them. Snappy stays the default, because it is what released artifacts use and what the sidecars' pyarrow reads without configuration, and because the setting changes every artifact's bytes: two runs compared by content hash must agree on it. On the CI fixture zstd is 3-11% smaller, which is too small a table to predict the float-heavy chromatograms of a real run. Unchanged output: the CI smoke passes with its TSV hashes intact, and a grouped fixture run reproduces the previous grouped run's `peptides.tsv` exactly, with and without zstd. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
# Conflicts: # rust/mumdia/crates/mumdia/src/stages/run_groups.rs
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…s that are sorted Two costs that only appear when several bands of a run are searched at once. The candidate-range fan-out took `rayon::current_num_threads()`, which is the whole pool, and every band in flight computed it independently: 24 bands each fanning out to twice the pool gave thousands of live narrowed-bin caches of 1-2 MB, allocated in lockstep, for pure scratch. `ExtractParams::sibling_bands` now says how many bands share the pool, and the fan-out is this band's share of it. The grouped orchestrator passes `groups.parallel`; every other caller passes 1, which is the previous behaviour exactly. Probing walks a window's scans in ascending index, so a candidate's hits are already rt-ascending in the ordinary case, and the sort then allocated scratch as large as the vector itself. Equal-rt hits collapse to a per-(rt, fragment) maximum afterwards whichever order they arrive in, so skipping a sort that would not move anything is exact. Those vectors are in the 64 KB to 1 MB class that dominates the process's memory mappings (measured on a live run: 92,030 mappings of 64-256 KB and 36,368 of 256 KB-1 MB out of 129,393 at 180 GB resident), so halving their churn matters beyond the copy itself. Output unchanged: the CI smoke passes with its hashes intact, and a grouped fixture run reproduces the previous run's `psms_extracted`, `chromatograms`, `psms_competed` and `peptides.tsv` byte for byte. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…columns The engine dies at ~290 GB resident with ~1.7 TB free because it is out of per-process memory MAPPINGS (1,048,576), not out of memory. Five changes in the library loader and the seed, all of which reduce live blocks or live bytes, plus one bug that has been printing a warning about a problem that does not exist. 1. `label` is read as one boolean per row instead of one `String` per row. The column is two-valued and its only consumer is `Candidate::is_decoy`, so a `Vec<String>` was 203M heap blocks and about 6.5 GB on the full 203M-precursor library to produce one bit each. `read_is_decoy` streams the column in 64k-row batches; the validity rule and the message stay in `fdr::validate_labels`, which now sees only the offending value, the one string ever allocated. 2. `candidate_id` is verified the same way and never held. The ids are a precondition, never data -- every use is the row position -- and the column was 4 B per precursor (812 MB) live across the whole fragment load and index build. 3. `Library::prec_mz` is the MOVED `precursor_mz` column rather than a second copy of it: same values, same order, row c is candidate c. 8 B per candidate, 1.6 GB and one large block off the load's peak. (The other two copies of this value, `Candidate::precursor_mz` and `FragIndex::prec_mz`, are read from extract.rs and cloned in fragindex.rs, so they are outside this change's file set and stay.) 4. `Library::release_fragment_payload` frees `frag_int`, `frag_name_id` and the name dictionary, and search-seed calls it once its fragindex exists. The seed reads neither predicted intensity nor fragment name: the hyperscore is count plus observed intensity, and the mass recalibration needs only `frag_mz`, which is kept and is now reached through `cand_frag_mz`. 6 B per library fragment, held for the whole search, roughly 7 GB and two large mappings on a 1.2G-fragment library. `cand_frags` panics after a release rather than handing back a short slice; the bucketed path keeps the payload. 5. The seed's output columns are reserved once at the known row count, the `label` column is built FROM the `is_decoy` boolean at write time instead of the boolean being rebuilt by re-scanning the column, and the per-scan `touched()` copy (up to one candidate window wide, per scan) is a borrow. BUG, unrelated to memory: the seed's window-width survey mapped every window group of the run, including the ones outside the loaded band. An out-of-band group resolves to an empty candidate range and contributed a width of 1, so with 63 bands the median was 1 on every band -- the scratch was sized to a single slot and grown on first use -- and the `widest > 8 * median` guard fired on all 63, telling the user to check for a zero-width or missing isolation window in a run that has none. `window_width_stats` now excludes groups with `hi <= lo`, the same test the worker already uses to skip a group, so both the sizing and the warning describe this search. Output equality: the boolean is exactly `label[c] == "decoy"` and an unknown label is still refused; the id check is the same comparison against the same file row; `prec_mz` is the same array; the released columns are unread on that path; the seed's `label` column is the same text and `is_dec` the same booleans; the scratch size never affects a result (SeedScratch grows on demand and new slots are unstamped), only the allocation. Tests added for each claim, including before/after equality of `cand_frag_mz` across a release and the old/new identity of the label round trip. Not done, blocked by the file set: a seed-only `FragIndex` build omitting `post_int`/`post_frag` (fragindex.rs), and moving `Candidate`'s two owned Strings to a flat buffer plus a dictionary id (extract.rs, fragindex.rs). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…mpeted row group Four terms, all of them live heap BLOCKS rather than bytes, which is what the engine actually dies on: a grouped search holds several bands in one process and hits the kernel's per-process mapping limit (1,048,576) at ~290 GB resident with 1.7 TB free (measured on a live run: 129,393 mappings at 180 GB, 92,030 of them 64-256 KB and 36,368 of them 256 KB-1 MB). The grouping was a `HashMap<key, Vec<usize>>` with one `Vec` per competition group. Under the shipped `group_by = peptidoform_charge` nearly every group is a singleton, so that is about one small heap block per PSM, plus a 48-byte-per-entry table at up to 87.5% load. It is now a `Vec<(key, row)>`, sorted once: 32 bytes per PSM in ONE allocation, so 100 MB and one mapping for the six-run Astral pool's 3.13 M PSMs. The resolver walks contiguous runs of equal keys instead of a sorted key list. It visits the same groups in the same order with members in the same ascending row order, and the winner rule was already order-independent with an index tiebreak, so the kept set and the removal pairs are unchanged; a test pins that against the previous HashMap resolver over all five modes on a population with ties, a NaN prelim and a signed-zero pair. `copy_kept_rows` built a take index and called `take` on all ~398 columns of every batch even when the batch had lost nothing, which under the default grouping is the normal case: the widest artifact of the run was copied through `take` to reproduce itself. An f64 column of a 16,384-row batch is exactly 128 KB, i.e. the 64-256 KB band above, 398 of them per batch and 191 batches for that pool. When as many rows survive as the batch holds they ARE the batch, in order, so the column is now passed through as an Arc clone. The synthesised all-zero `peak_rank` has no source array and is still built. The peptidoform column was read as a `Vec<String>` for the whole table and, unless the competition audit is on, only used to number dense ids; the label column was read as `Vec<String>` to be compared against two literals. Both are now streamed: the label as one byte per row, the peptidoform into first-appearance ids whose map holds one String per DISTINCT peptidoform rather than one per row. `base_peptide_id` is read only for the groupings that key on it and `candidate_id` only for the audit sidecar, which is the only consumer of either as a value; both columns are still required and type-checked by the pass-through copy. At 3.13 M rows that is ~6.2 M fewer String blocks and ~175 MB less. The competed table is written in 131,072-row groups, the cap the rescore handoff already uses and the one every other wide writer in the engine sets. This file took parquet's default 1,048,576, which at ~387 f64 feature columns is ~3.2 GB decoded per group: that is what the writer buffers before each flush, and rescore is the reader that decodes a whole group before slicing batches out of it. 131,072 rows is ~405 MB. Output equality: the competed parquet, its `.schema.json`, its report's row/kept/removed stats and the audit sidecar are byte-identical to the previous implementation over seven configurations (all three groupings, all four modes, audit on and off) on a 500-row crafted features table, and value- and order-identical on a 300,000-row one, where the cap binds and splits one row group into three. Only row-group boundaries move, so the competed table's `content_hash` changes for any run above 131,072 kept rows; nothing downstream reads that hash as an equality assertion. The one behavioural drift is error text, not values: a features table missing or mistyping `candidate_id` or `base_peptide_id` now fails in the pass-through copy's column check rather than in the typed getter. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…le once Three changes to the grouped middle, none of which changes a value. 1. `psms_extracted.parquet` is pooled only when something reads it. Its one consumer is the candidate audit (`extract.emit_candidate_audit`, off by default and, per CLAUDE.md, reading a per-candidate audit table extract does not yet write). Pooling it read every band's extracted rows and wrote them again into the run's second-widest artifact, for a file nothing opened. The band tables are written either way, so the audit path is unchanged and the run says in the log when it skips the pool. 2. Each pooled artifact is hashed once. `record_artifact` hashed the file for the manifest record and `ArtifactReport` hashed it again for the report beside it; on a real run those are tens of GB each, and blake3_file reads all of it. `record_artifact_with_hash` takes the hash the caller already has. 3. Under `experiment.rt_library_scope = first_run_only` the runs after the first reuse the first run's band precursor slices. A slice is a deterministic function of (library, row span) and a shared-band run searches the same library under the same plan, so rewriting it produced the same bytes; only the seed reads it, and the adapted table replaces it everywhere else. On a 203M-precursor library that is the whole precursor table not written, per run after the first. The reuse is guarded on the file existing with the row count the plan expects, and falls back to writing the slice. Two more, outside the grouped path: - `split_by_source` caps its row groups at 131,072 rows, as the rescore handoff does. The scored table is ~390 float columns wide, so parquet's default 1,048,576-row group is about 3 GB decoded per group for every reader of the per-run tables. Row-group boundaries move; rows, order and values do not. - `load_ms1` copies each peak list once instead of twice. Both lists were copied whole and then copied again truncated to the shorter of the two, which on MS1 scans of tens of thousands of peaks is two allocations per scan per column that are freed immediately. New unit test covers the truncation and the RT ordering, neither of which had one. Measured on the running seven-file experiment (63 bands, 8 in flight): extract 218 CPU-min per run, features 156, search-seed 99, rt-im-train 1.2. The RT calibration is not worth hoisting; the writes are. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… columns that already exist Five allocation-count reductions in the rescore stage. Output is unchanged: the crafted features -> compete -> rescore pipeline writes a scored parquet with the same blake3 (e535d04b2821a47d4fc8f9bc0a7ba57461b701b607496ffef0700a0352cd135a) before and after, and the two kernels that were rewritten are checked against transcriptions of their previous selves at bit level. 1. `percolator_lite` built its standardised training slice as a `Vec<Vec<f32>>`, one heap block per training row, inside the parallel fold map, so every fold's copy is live at once. That is the exact layout `FeatureMatrix`'s own comment records was removed from the matrix for this reason, surviving one level down: at 11.6M PSMs x 3 folds it is ~23M live blocks, now 3 flat buffers sliced by `chunks_exact`. The held-out fold is scored through the standardizer without materialising a row at all. The f32 narrowing is kept at both sites, because the score consumed the standardised value AFTER it was rounded to f32. 2. The competed-input merge cloned every metadata string (label, peptidoform, protein) into a second vector while the reader's stayed alive: twice the string bytes and 3 extra live allocations per PSM at peak. `merge_col` moves the first input's column and reserves the footer row count, so a single-input rescore copies nothing and a multi-input one reallocates once. 3. `target_decoy_q` sorted an index permutation with a comparator that dereferenced a separate key column (two random loads per comparison, two more per row in the tie walk) and allocated four n-length vectors. It now sorts one 16-byte `(key, row, is_decoy)` record in parallel and merges the monotonization into the block walk, so `fdr_at` is gone: 32 -> 24 bytes per row, 4 -> 2 allocations, and no indirection. The tie-block handling is unchanged, and the permutation is the same one the stable sort produced (`key` desc, row asc). 4. The non-finite validation was a serial `n_rows x n_features` scan over hundreds of GB. Both scans are parallel `find_first` now, and their answers are combined so the row reported is still the first offending row, features before scalars within a row, exactly as the interleaved serial loop reported it. 5. `psms_scored` has three duplicate columns by construction (`protein_group` is `protein`; `global_q_value` and `experiment_psm_q` are the pooled `q_value`). Through `write_table` each had to arrive as its own `Vec`, so the stage carried a second full copy of the protein strings and two more of the q column. They share one refcounted Arrow array now. A test asserts the parquet is byte-for-byte what `write_table` wrote from separate equal vectors. Risk to output equality: (1) and (3) are arithmetic-identical by construction and covered by reference-implementation tests; (5) is byte-tested; (2) and (4) move or parallelise without touching values. The one behaviour that is not identical is which error a malformed input reports when it has both a non-finite feature and a non-finite prelim_score in the SAME row -- it is still the feature, and still the first offending row, so this is preserved too. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…n chunks Four changes to the Arrow/Parquet table layer, all of them about how many live heap blocks and how much repeated work one band of a grouped search costs. No artifact changes: the scalar write path is asserted byte-identical to the one it replaces, and every new reader is asserted equal to the reader it stands beside. 1. `TableFile` holds its parsed footer (`ArrowReaderMetadata`) and every reader it builds reuses it. `ParquetRecordBatchReaderBuilder::try_new` re-reads and re-parses the footer on every call, and it was being called once per `open`, once per `row_group_stats` and once per typed getter, so reading nine columns of one band cost ten parses. Measured on a 13,000-row-group, six-column, 1.3-million-row file (the shape of a library fragment table): 26.6 ms per parse against 118 ms to read a whole f64 column, so each getter paid about a fifth of its own cost again for a footer it already had. `row_group_stats` now touches no file at all. `TableFile::span(first_row, n_rows)` is the span read against already-parsed metadata: `open_rows` is now `open` + `span`, and a caller that cuts one library into many bands can open once and span per band instead of re-parsing per band per stage. Spans do not nest, so a span's rows are always file rows. `span_of_an_open_handle_matches_open_rows` asserts a span equals `open_rows` column for column and equals the whole table's slice, over an empty span, the whole file, inside one row group, across several and the last row. 2. `str_eq(name, value)` on both read paths: a string column as a boolean equality test. `str` returns one `String` per row, which for `label` on a 203-million-row precursor table is 203 million live allocations and 4.9 GB of `String` spine to carry one bit per row. No caller changed; the test asserts `str_eq` is exactly `str(...).iter().map(|s| s == value)`, error for error, including the NULL refusal. `str_flat` is the same idea for the high-cardinality columns where an equality test will not do (`peptidoform`, `protein`): one buffer plus offsets, the counterpart of the `list_f32_flat` that is already here. 3. `write_table` encodes in 65,536-row chunks instead of building one `RecordBatch` for the whole table. The full batch was a second complete Arrow copy of every column beside the source vectors, one very large heap block per column; mimalloc maps those individually, which is what the per-process mapping limit counts. The chunk size divides the writer's default 1,048,576-row row group and is a multiple of its 1,024-value write batch, so the row groups and the pages fall where they fell: `write_table_matches_one_batch_byte_for_byte_on_scalars` compares the new output with the old code path (`cols_to_batch` + `write_batches`) byte for byte over 196,615 rows, and again at 0, 1 and 1,000 rows. List columns hold a variable number of leaf values per row, so their pages are the encoder's business; there the test asserts the rows and the row-group count (measured byte-identical too, at 0-6 and 0-136 values per row, but not asserted). Validation moved into `validate_cols` so the whole column set is still checked once, before anything is written, with the messages it always had. Risk to output equality: the scalar write path is byte-asserted; the list write path is row-asserted and was measured byte-identical. The read changes cannot alter values -- the metadata is the same metadata, and `str_eq` / `str_flat` / `span` are new entry points with equality tests against the existing ones. One behavioural difference worth naming: a `TableFile` is now a snapshot of the footer as of `open`, where before each getter re-read it. Readers still open the file by path per call, so a file replaced under an open handle was already inconsistent between `row_group_stats` and a getter; a run does not rewrite an artifact it is reading. cargo fmt, cargo clippy -p mumdia-io --all-targets -D warnings, cargo test --workspace all pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tation quant was the last consumer of the chromatogram table still holding it as `Vec<Vec<f32>>` plus a `String` per row: three heap blocks per chromatogram row, on top of four to six more per candidate. That is the shape that kills a grouped search at ~290 GB resident with 1.7 TB free, because the process runs out of per-process memory MAPPINGS (1,048,576) rather than out of memory, and `--out-peak-bounds` bypasses the accepted-candidate filter so the whole table is held that way. What changed, in the order of the findings: 1. `ChromStore` gives the table the flat treatment `features` already gives it: one buffer for intensities, one for RT axes with the axis shared between the rows of a candidate that sample the same grid (every row under extract's window grid), interned fragment names, and flat per-row index arrays. The MS1 isotope pseudo-traces are dropped at load rather than stored and skipped later, after the rt/intensity length check, which stays a guard on the table. Live heap blocks per row go 3 -> 0. Derived from the layout at a 60-sample trace and 8 fragments per candidate: ~624 B/row -> ~314 B/row, before allocator per-block overhead, which the figure does not count. 2. `CandIndex` replaces `BTreeMap<u32, Vec<usize>>` (a node and a `Vec` per candidate) with one grouped row array, and `win` / `applied_win` / the areas become flat arrays indexed by candidate position. The per-candidate `Vec<f64>` of areas, the `(area, predicted)` pairs and the `(name, area)` pairs are gone: rayon writes the areas into disjoint sub-slices of one buffer, so the positional pairing the serial fold used to assert is now structural. Blocks per candidate go 4-6 -> 0; what remains transient is bounded by the thread count, not the candidate count. 3. `peak_window` built the union of the RT axes by inserting every sample into TWO BTreeMaps keyed identically, one tree walk per sample each. It now either accumulates directly into the shared axis (the common case) or sorts the samples once by RT bit pattern. Both reduce each RT's f64 sum in the same order the tree did -- row order, then sample order -- and the sort is stable for that reason. 4. The fixed-window indices were computed twice per fragment row, once for the area and once to report the bounds; they are computed once. The nearest-sample search inside them linear-scanned the whole trace, which is O(trace) where the integration is O(window); it binary-searches when the store says every axis is non-decreasing and NaN-free, and falls back to the scan otherwise. 5. `select_fragment_areas` collected and SORTED a candidate's areas once per scored row mapping to it; memoised per candidate. 6. The cross-run combine cloned every feature vector into the protein matrix and again into each of the two sibling levels. `lfq_profile` now takes anything that borrows as `[Option<f64>]`, so all three borrow from one owner. The protein-group key is no longer allocated per row in either the rollup or the combine. Risk to output equality: the arithmetic and every reduction order are unchanged, and the equality claims are tested rather than argued. `peak_window_both` runs every existing `peak_window` test through BOTH union constructions and asserts they are bit-identical; `fixed_window_indices_both` runs the window tests through both nearest-sample searches; `nearest_index_binary_search_matches_the_scan` covers the tie-breaks the scan's strict `<` produced (duplicate RTs below, at and above the target, exact midpoints, targets off both ends); `a_repeated_rt_sample_merges_into_one_profile_point` pins the merge the BTreeMap did; `the_candidate_index_groups_exactly_as_the_map_it_replaces` compares the grouping against the map for a grouped, an interleaved and a single-row table; `borrowed_rows_give_the_same_profile_as_owned_rows` pins the LFQ core. The residual risk is the shared-axis shortcut in `peak_window`, which is taken only for an axis checked strictly increasing and non-negative when it is stored, and the binary search, which is taken only when every stored axis is non-decreasing and non-negative; either property failing sends the code back to the merge and the scan. Not run end to end on a real chromatogram artifact. Not done: streaming quant per candidate rather than materialising the run's chromatograms (finding 7). It needs each candidate's rows to be contiguous in the table, which this stage does not currently require, and it doubles the parquet read on the normal path, where the accepted-candidate filter already keeps only a few percent of the table. Reported rather than attempted. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…instead of materialising them Under `rescore.strict` with a PIN sidecar the engine's own `FeatureMatrix` is never read. The handoff file is written FROM it, it is released immediately after, and strict turns a sidecar failure into an error rather than a fall back to `native_tda`, so the native path that would read it is unreachable. It was still materialised in full first -- one allocation of rows x features x 4 bytes, about 250 GB at experiment scale (203M-precursor library, ~11.6M PSMs x 387 features and up) -- purely so that it could be copied to disk column by column. The competed feature columns now go straight into the handoff. What stays resident is one staging block of HANDOFF_BATCH_ROWS rows, about 390 MB at 387 features, plus its transposed columns while a block is encoded. That is the largest single allocation in the stage removed outright, and with it the largest contributor to the mapping count the OOM at ~290 GB resident was traced to. How it is arranged: - the load is two passes. Pass 1 reads the per-PSM metadata columns of every input; pass 2 reads the feature values, either into the matrix (`load_feature_matrix`) or through `for_each_feature_row` into the handoff. Splitting them is what makes the feature pass skippable, and it keeps the order of the population checks: labels and the target/decoy population are still validated before anything reads a feature value. It costs one extra parquet footer read per input. - `HandoffWriter` writes either encoding a row at a time, so the same writer serves the matrix path and the streamed path. The parquet blocks are the same 250k-row blocks, the row groups the same 131,072 rows, the values narrowed through the same `as f32` (including `f64::NAN as f32` for a null cell), so the file the worker reads is unchanged. A test writes both ways over two inputs with a batch small enough to force several blocks and compares the bytes. - the non-finite feature validation moves into the streaming pass, inline with the narrowing while the row is still in cache. `bail_non_finite` then picks between it and the scalar scan on the row index, so the error is still the first offending row, feature before scalar, with the same message. - `sidecar_paths` names the per-invocation files once; `run_pin_sidecar` takes the already-written handoff when there is one. A guard refuses a handoff whose row count disagrees with the metadata: the two passes cannot disagree, but if they ever did the handoff would be row-misaligned with every label. Equality: the streamed and matrix-built handoffs are byte-identical (tested, both encodings); the scored artifact of the crafted features -> compete -> rescore pipeline keeps the same blake3 (e535d04b2821a47d4fc8f9bc0a7ba57461b701b607496ffef0700a0352cd135a) as before this branch. `rescore.max_feature_matrix_gib` is deliberately left applying to the streamed path too: the worker still builds a matrix of exactly that size, so the ceiling still describes a real allocation. Residual risk: the streamed path is covered at unit level (row values equal to the matrix rows, handoff bytes equal, and `run` driven end to end with an unspawnable interpreter so the handoff is written and then strict fails), but no test in this repository runs a real Python sidecar, so the worker's side of the contract is unexercised here. It reads the same bytes it read before. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… thread, and a pruned confident-bounds pass Six changes to the features stage, all output-equal. The stage's problem is the NUMBER of live medium heap blocks, not bytes: mimalloc maps blocks of that size individually and a grouped search dies at ~290 GB resident against the kernel's 1,048,576 per-process mapping limit, with 92,030 mappings of 64-256 KB and 36,368 of 256 KB-1 MB measured on a live process at 180 GB. - The per-PSM extended values were one `Vec<f64>` each inside a `Vec<PerPsm>`: 337 f64 (2,696 B) per block, 70k-175k live from the collect() to the drop at the end of every chunk. They are one flat rows x n_ext buffer now, filled by par_chunks_mut. `FragFeatures` (no heap) stays a Vec. - The cross-charge reduction was a `HashMap<&str, HashSet<i32>>` plus a `HashMap<&str, f64>`: one HashSet block per distinct peptidoform, about 1M live for the whole run. Peptidoforms get a dense id (compete.rs's `pform_id` pattern) and the reductions become a `Vec<u32>` charge bitmask and a `Vec<f64>` of sums. The bitmask is exact for charges in 0..32 and falls back to the set-per-peptidoform build otherwise; the sums accumulate in the same row order, so the f64 addition order is unchanged. - The serial tail hashed a feature NAME per value per row, ~390 lookups per row in the Extended set, to resolve a permutation fixed before the loop. `ColIx` resolves it once. Its field names ARE the feature names (macro-generated), and a test asserts the fields of each set account for every active column exactly once, so a typo cannot silently drop a column. - The value matrix's columns are owned, so the writer takes them by move instead of `column(c).to_vec()`. That copy is why the matrix existed twice at the write and why `memlog` understated the peak by exactly 2x; the report now names the in-flight peak as well. - The parquet encode moved to a writer thread mirroring the loader thread, order preserved. Both channels are rendezvous (depth 0): `sync_channel(1)` held THREE chunks, not the two its comment claimed. - `confident_global_bounds` streamed the entire chromatogram table (up to 88 GB) to learn two scalars from a few thousand candidates, on by default. It now reads only the row groups whose `candidate_id` statistics can contain a confident candidate; a candidate's rows are contiguous, so one that spans two groups is inside both ranges and neither is skipped. Chunk boundaries inside a span stay on the same ABSOLUTE row grid, so a candidate straddling one is split where it was split before and the percentiles are identical. On the test fixture (100 row groups, every fifth candidate an anchor) it reads 40% of the rows. - Hoisted allocations, all bit-identical: the per-scan `obs_k` in the pCos loop and the per-fragment `others` in the contrast loop (features.rs); the summed-positive gather in nonzero.rs, which was two Vecs per fragment PAIR for a per-fragment quantity, plus scratch reuse for the third; the k x k `corr` (coelution.rs) and m x m Gram matrix (interference.rs) are one flat buffer each instead of a Vec per row, and power_top's per-iteration `nv` is reused. The trace alignment was built twice per PSM, once in `fragment_features` and once in `build_evidence`, each fragment through its own `HashMap<u32, f32>` keyed on the RT bit pattern. It is built once per PSM and handed to both (Evidence takes ownership), and it has a fast path: the chunk store shares ONE axis across a candidate's rows, so when every row with a trace samples that same strictly ascending slice the union axis IS that slice and every map is the identity. The two paths' bounding behaviour (`fragment_features` honours `bound_features`, `build_evidence` does not) is untouched. Two hazards the threading uncovered, both fixed rather than inherited: - the loader's receiver lived in the enclosing frame, so an error return from the scope closure left the loader blocked in `send` on a channel nobody would receive from again while the scope waited to join it: a hang instead of an error. Depth 0 makes that more reachable, so both channels are created inside the scope and their receivers drop with it. - with the write on its own thread, an error would drop the sender, end the writer's loop normally and PUBLISH a truncated features table over the previous good one. A `None` commit marker is sent only after the last chunk; without it the writer drops unpublished and `AtomicPath` removes its temp file. Output equality: eight configurations (Extended/Rich/Minimal x bound_from_confident on/off x chunk sizes 64, 4096 and 2^20, with and without a seed table) over a 97-candidate fixture produce byte-identical feature values and PIN bytes before and after, compared as raw f64 bit patterns per column. New tests assert the new code against the old: the shared-axis alignment against the union-and-map build (and that it refuses an unsorted or repeated axis), the charge bitmask against the set build, the pruned confident-bounds pass against the whole-table scan at four chunk sizes, ColIx against the name lookup, and the whole chunked Extended pass for chunk invariance, which the existing `features_chunking_is_value_preserving` never reached because it runs the default Minimal set. Residual risk: the confident-bounds pass already depended on the chunk size before this change (a candidate straddling a chunk boundary is bounded from each part separately), which the alignment preserves but does not fix. cargo fmt, cargo clippy -p mumdia --all-targets -D warnings and cargo test --workspace all pass. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Measured on the running seven-file experiment, at the moment it reached the pool: the stage was writing 3.3 MB/s and reading 3.9 MB/s at 110% CPU, on a disk that reads 221 MB/s (dd, direct). One run's chromatograms are 63 bands x 1.09 GB, so at that rate pooling one run would have taken about five hours, and the experiment pools seven of them. The rows were being decoded from parquet, filtered, and encoded again for no reason: the bands already write library-wide ids, so the pooled table is the bands' own rows in the bands' own order. `SpliceWriter` (mumdia-io) appends a source file's encoded column chunks into the output with the parquet writer's `append_column`, which is what that API exists for. The output takes the template's parquet schema AND its Arrow metadata, so the `LargeList` chromatogram columns read back as `LargeList` and not `List` -- that distinction lives only in the key-value metadata, and losing it would have changed the type of the run's largest artifact. A test asserts it. Only the row groups that can hold a candidate the overlap dedup drops are decoded: the statistics on `candidate_id` say which, the rest are copied. A dropped candidate comes from a window that two bands share, so those row groups are at the two ends of a band. The rewritten group is encoded to a temporary file beside the band and spliced in at its own position, so the pooled row order is unchanged whether a group was copied or rewritten. Also here: `groups.parallel` is clamped to one less than the thread count. A band in flight parks a rayon worker on its extraction's accumulation channel while the probing tasks run on the others, so as many bands as threads leaves nothing to feed them and the run deadlocks. Reproduced on the fixture with `parallel = 2, --threads 2`: the process sat at 0.1 s of CPU with no error and no output. The production runs are far from the bound (8 bands, 128 threads), which is why this was never hit. Tests: the three pool tests cover a clean splice, a band whose losers sit in one of ten row groups (mixed copy and rewrite, asserting the pooled ids are the bands' rows in order), and the skipped `psms_extracted`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…non-nullable The pass-through copy reads fewer columns through the typed getters than the typed path did -- `candidate_id` only for the audit, `base_peptide_id` only for the groupings that key on it -- and those getters are where nulls were refused. Without a check the copy would carry a null into the competed table and the failure would surface somewhere downstream, or nowhere. Raised by the adversarial review of the compete change. `Array::null_count` is a counter on the array, not a scan, so this is one comparison per column per batch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The adversarial review of the chunked `write_table` argued that its byte-equality claim does not hold for an entirely NULL optional column, because the definition levels are then RLE-run encoded and the writer takes its internal mini-batches in a different size, so a row-chunk boundary no longer lands on a mini-batch boundary. That is correct, and it is measured here rather than argued: 2,287,903 bytes against 2,287,919 at 196,615 rows, a 16-byte difference in page framing, with identical rows. It matters because every real run writes such a column: `apex_im` on `psms_extracted` and the three ion-mobility columns on `run_windows` have exactly one push site each and it pushes `None`. Those two artifacts therefore have different content hashes from the ones a pre-chunking build wrote. The rows, their order and their values are unchanged, which is what this layer promises; the byte change is of the same kind as the row-group caps elsewhere in this series. Two tests, neither of which existed: an all-null table compared row for row and row-group for row-group, and a table past the writer's 1,048,576-row row-group maximum, where the chunked and single-batch files come out byte-identical with the groups in the same places. The second is the property the 65,536-row chunk size was chosen for and nothing covered it: every other chunking test is inside one row group. Also: the all-null test gets its own temporary directory. The shared one is wiped with `remove_dir_all` by a test in another module while these run in parallel threads. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…andidate The grouped search dies at ~290 GB resident with "memory allocation of N bytes failed" while 1.7 TB of RAM is free. Measured on the live process, the limit it reaches is the per-process memory MAPPING table (vm.max_map_count = 1,048,576): at 180 GB it held 129,393 mappings, 92,030 of them 64-256 KB and 36,368 of them 256 KB-1 MB. Those sizes are one-heap-block-per-item structures, and the item here is a candidate: every candidate with evidence owned a `Vec<Hit>` (24 B per hit) created on first collision and grown by doubling, in the task-local map inside `probe_range`, in the merge into the accumulator, and in the drain. A candidate with no `run_windows` row gets infinite RT bounds and so collects hits across the whole gradient, 10^4-10^5 of them, i.e. 240 KB-2.4 MB in one block, which mimalloc maps individually. Those hits now live in a CSR layout (`HitStore`: ascending `cids`, per-candidate `offs`, one flat `hits` buffer), so a band of 1.9-5.8M precursors holds three allocations per probing task instead of one per candidate with evidence. Live medium/large blocks drop from O(candidates with evidence) to O(tasks), which is 2x threads. Mechanics, all chosen so the bytes never double: - `probe_range` collects flat `(cid, hit)` pairs in probe order and groups them at the end of the task with a STABLE counting sort (`group_hits_by_candidate`). The permutation is applied in place by cycle following rather than scattered into a second buffer, and the `cid` array is reused as the slot array, so the extra is 4 B per hit plus one u32 per candidate of the sub-range, not another 24 B per hit. - A window's sub-range tasks cover disjoint ascending candidate spans, so their stores concatenate into one ascending run (`HitRun`) with no per-candidate merge at all. The consumer appends runs in window order; `gather_chunk` resolves the concatenation lazily at flush, one `CAND_CHUNK` of candidates at a time through a reused buffer. - `per_candidate` takes `&mut [Hit]` into that buffer instead of an owned `Vec<Hit>`; it sorted a slice either way. - The eager (non-fragindex, two-pass, allowlist-free-of-fragindex) paths still build the whole-run `HashMap<u32, Vec<Hit>>` and are now moved into the CSR buffer a chunk at a time, freeing the map's vectors as they are consumed, so the flat copy never coexists with the whole map. Also hoists the per-peak query m/z out of the sub-range tasks: `peak.mz / mass_off.factor_at(peak.mz)` is a division, and a binary search into the masscal grid when one is present, and it was recomputed once per task per peak, up to 2x threads redundantly. It is now computed once per window into a flat buffer the window's tasks share, built only when the window is actually split. The bin computation (a `ln()` inside `probe_peak_win`) is the other half of that hoist and is NOT done here: it needs a `FragIndex` entry point taking a precomputed bin, which is outside this file. Risk to output equality: none intended, and the ordering is the whole argument. A candidate's hits must arrive in ascending scan order within a window and with windows concatenated in window order, because every float reduction downstream (rt sort, apex sum, wsum, chromatogram grids) is order-sensitive. The counting sort is stable, so within a task the order is arrival order; the sub-range spans are disjoint, so a candidate is produced by exactly one task of a window; and the gather walks runs in window order, appending each candidate's segments. PSM row order, chromatogram row order and the CAND_CHUNK partition (hence parquet row-group boundaries) are unchanged: the flush is the same candidate partition in the same sequence. Tests: `candidate_range_split_reproduces_the_unsplit_accumulation` now materialises the accumulator through the production gather and `slices_mut` and still asserts 2/8/16 threads agree hit for hit; `grouping_reproduces_per_candidate_push_order` asserts the counting sort reproduces the naive "push onto this candidate's Vec" accumulation it replaced, including untouched candidates inside the span; `gather_concatenates_a_candidate_across_runs_in_window_order`, `gather_stops_at_the_bound_and_keeps_the_rest` and `gather_chunking_partitions_the_accumulator` cover the flush. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… boundary From the adversarial review of the library change. Reading the label column in 65,536-row batches means the loop must push exactly one value per row; the `other` arm called `fdr::validate_labels`, which refuses anything but target/decoy, and pushed nothing. That is correct only because that rule is absolute today: if it ever widened, the column would come out shorter than the table and every later row would read its neighbour's label. The arm now ends in an explicit error naming the row and the file. The batch boundary itself had no test: every other fixture in index.rs is six rows. The new one is 65,541, and covers the label values either side of the boundary and in the short last batch, the `ncand` cap, a wrong candidate_id in the second batch (which must be named by its absolute row, not its row within the batch), and a NULL label there. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`run` pools at the end of a grouped search. Standalone the same stage is for the case where the search finished and the run did not: the band directories hold every artifact, and pooling them is now a byte copy of their parquet row groups, so a killed run costs a pool rather than a re-search. It also makes the pool testable at real scale without a search, which is how the splice was measured. `--psms` opts into pooling psms_extracted, which only the candidate audit reads. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`window_groups` drops the windows that select no candidate of the band, so a band of a 63-band grouped plan overlaps only 1-3 windows: the whole band is one batch, the flush bound is `u32::MAX`, and nothing was ever flushed early. The accumulator held the entire band, which is 1.9-5.8M precursors' worth of hits, for the whole stage. `windows_in_flight` cannot help, because there is only one batch. The sub-ranges that already split each window's candidate range are now a GLOBAL grid shared by every window of the batch, and a sub-range is flushed as soon as every window that intersects it has reported. Per-window sub-ranges parallelise just as well but cannot be flushed: a candidate is final once every window of the batch has passed it, and that is a statement about a boundary the windows share. Tasks are one per (sub-range, window) pair that actually intersects, k-major, so the pool starts on what will be flushed first. The width is taken from the MEAN window span divided by `tasks_per_window` rather than from the global span divided by a task count, so each window still splits into about `tasks_per_window` pieces and the batch still runs about 2x threads of them. No extra probing, no extra `WindowNarrow` caches: these are the tasks that existed before, on a shared grid. On a one-window band at 32 threads the flush unit becomes 1/64 of the band; on a 16-window batch of a whole-library search it becomes roughly one window's span instead of sixteen. The end-of-batch drain stays, because a batch's windows can reach past the last window's range when the windows differ in width, and the sub-range grid stops at the widest window's end. Risk to output equality: PSM rows and values unchanged; chromatogram parquet row-group boundaries may move. Sub-ranges are flushed in ascending index and candidates ascending within, and the sub-range grid partitions the candidate axis, so the flushed sequence is the same ascending candidate sequence as before. A candidate's hits are still gathered run by run in window order, which is what every float reduction downstream depends on. What does change is where a flush ends: the ChromChunk partition now follows sub-range boundaries as well as `CAND_CHUNK`, so the chromatogram writer's row groups can fall differently. That is the same class of change `windows_in_flight` already documents as acceptable, and no row and no value moves with it. Also hoists the per-window query-m/z buffer out of the window probe and into the batch plan, since the per-window task count is now known there. Test: `candidate_range_split_reproduces_the_unsplit_accumulation` now runs the fixture twice, once with `bound = 0` (nothing flushes, the accumulation is compared as before across 2/8/16 threads) and once with `bound = u32::MAX`, and asserts the per-sub-range flush delivers exactly the same candidates, strictly ascending, with the same hits in the same order, and leaves nothing open. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An adversarial review of the feature-streaming change found five issues in `stages/rescore.rs`. Four are real and fixed here; one is a documentation claim, corrected in place rather than by changing code. Fixed: - The non-finite validation ran AFTER the whole handoff was written and published. It used to run before `run_pin_sidecar` was entered, so a malformed competed table cost nothing; deferring it made the same failure pay a full handoff write, hundreds of GB at experiment scale, for a run that was always going to abort. The scan now aborts the stream on the first offending row. The message is unchanged: `bad_scalar` is known before the stream starts, so `bail_non_finite` still makes the same choice between the two answers on the row index, and the stream stops at the scalar row once no earlier feature row can win the tie-break. A parquet handoff never reaches its final name (`AtomicPath`), but the PIN encoding writes it directly, so an aborted stream now removes it. - The row-count guard existed on the streamed path only. The matrix path gets the same guard, and both now go through one `refuse_row_disagreement` helper, as does a third check the review flagged as a silently dropped invariant: `merge_col` appends whatever the reader returned, while `source` is still sized from the footer, so every concatenated metadata column is now measured against the footer row count at the point `n` is taken from it. - An empty feature set is refused by name. The old code neither errored nor worked: `iter_rows` is `chunks_exact(n_features.max(1))` over an empty buffer, so the validation loop ran zero times, which also skipped the prelim_score/precursor_mz check for every row, and the run trained on no features. An explicitly empty `rescore.features` list was already an error, so this makes the two consistent. - `a_shared_column_array_writes_the_same_parquet_as_two_copies` ran at 2,000 rows, below `write_table`'s 65,536-row chunk, where the two writers cannot diverge. It now runs at 3 chunks plus a short last one. The row-group seam is pinned in mumdia-io at 1,048,583 rows. Rejected: - "`write_scored_table`'s memory claim inverts on the merge target": the byte output is unaffected and the mechanism is right, but the review is correct that on `perf/integrate` chunked `write_table` holds one chunk of Arrow copies while one batch re-packs each string column in full. The doc comment now says that instead of claiming an unqualified saving; the code is unchanged. - `assert!(n <= u32::MAX)` in `fdr::target_decoy_q` stays an assert. It is unreachable at any allocatable scale, the alternative it guards is silent truncation scattering q onto the wrong rows, and an `anyhow` error would change a signature used by search-seed and others, outside this file set. - `rescore.max_feature_matrix_gib` stays checked on the streamed path. The ceiling is `rows x features x 4`, which is exactly the matrix the Python worker builds from the handoff, so it still describes a real allocation; making it path-dependent would let a strict `nn_torch` pool that will OOM the worker start anyway. The default is 0.0 (off). - The commit-message residency figure for the staging block: corrected in the code comment (the staged f32 block plus what `flush_block` transposes out of it, roughly twice the stated 390 MB). Tests: the two abort tests fail against the pre-fix ordering (the handoff was written and published before the error), the tie-break tests pass before and after, which is the point. Not fixed, outside the file set: `cargo fmt --check` already fails on `stages/pool.rs` and `mumdia-io/src/table.rs` at this branch tip. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…her than enforced Two adversarial reviews of the quant chromatogram store and the seed's window-width survey. Both found real defects; each fix is pinned by a test that fails when the fix is reverted, verified one fix at a time. quant.rs - `nearest_index`'s comment claimed any non-finite target "makes every predicate false and lands on 0". True for NaN and -inf, false for +inf: `r < inf` holds at every sample, so `partition_point` returned `rt.len()` and the LAST index came back where the scan returns 0. The precondition that actually holds is "finite", so it is now a guard returning the scan's answer, not a comment. Unreachable today (both call sites filter the apex hint on `is_finite`, and convert drops a spectrum whose scan start time is not finite), so no artifact moves. - `ChromStore::axis_for` deduplicated axes with slice `==`, and `-0.0 == 0.0` under f32's `PartialEq`, while `peak_window`'s merge keys the union on `to_bits`. A row whose axis differed from a stored one only in the sign of a zero was given the stored axis, folding two union points into one: different profile, apex, window and quantity. The comparison is now bitwise, so interning can only substitute an axis identical to the last bit. - `peak_window`'s `axis_sorted` guard asked whether the merged axis's last sample was `>= 0.0`, which `-0.0` satisfies although its bits sort after every positive f32. An axis ending in `-0.0` was declared ascending while descending at its last step, and `nearest_index` then binary-searched it, moving `ai` (the apex index that anchors `peak_bounds`). It now checks the order itself, one pass over an axis the sort already paid O(n log n) for. Neither -0.0 path is reachable from extract, whose grid is positive mzML scan times; both are reachable through `mumdia quant --chromatograms`, which is why the neighbouring rt/intensity length check exists. - `axis_for` could in principle mint the `NO_AXIS` (`u32::MAX`) sentinel, which every consumer reads as "no RT axis" and silently integrates to 0. Checked where the id is minted (one comparison per distinct axis); `ChromStore::push` and the loader now return `Result`. - `peak_window_both`'s claim to run every case through BOTH union constructions did not hold: `peak_window` takes the shared-axis fast path whenever the rows resolve to one strict axis id, which a single-row call always does whatever candidate id it was pushed under, so three of eight call sites compared the shared path against itself. The merged fixture now clears `axis_strict`, which withdraws the fast path without touching a stored value, and the helper asserts that routing. Reverting the fixture fails all six `peak_window_both` callers. search_seed.rs - ACCEPTED, the guard was weakened. Restricting the width survey to the in-band groups removed a 63x false positive and introduced a false negative: under `groups.window_groups` a full-range window resolves to the whole loaded band, and so does the band's own isolation window, so `widest > 8 * median` can never fire on a band. The malformed scan is now detected from the WINDOW instead: convert substitutes (0.0, 1e6) for a zero-width or missing isolation window, which reads the same on every band. A mixture of full-range and narrow windows warns; all full-range is a legitimate all-ion acquisition and stays silent. The width ratio is kept as the second, weaker diagnostic, with a comment saying what it can see under a grouped search. - ACCEPTED, the eager scratch sizing was a memory regression. "It grows on demand to the widest window it processes anyway" holds only for a leaf that processes a served group. `map_init` runs once per rayon LEAF, before the leaf knows which groups it drew, so in a banded search where 62 of 63 groups select nothing, nearly every leaf would allocate 16 B x band width and return empty: about 14 MB per worker on the profiled 54.8M-candidate library over 63 bands, roughly 445 MB across 32 workers, almost all never touched. `initial_scratch_width` now takes the served median only when a majority of groups is served; the leaf that does draw work still reaches the same size through `SeedScratch::ensure`, in one amortised realloc. Rejected - The reviews' remaining findings are all in `index.rs`, which is out of this change's scope and already addressed on the integration branch. - Not acted on, recorded instead: `peak_window`'s shared fast path indexes `prof` (sized from the AXIS) by the INTENSITY position, so its non-panicking behaviour rests on the loader's per-row length check. Making the loop `zip` would replace a panic with silent truncation, which is worse, so the invariant is asserted at `ChromStore::push` (debug only; the loader keeps the named, candidate-identifying error). - Neither review's output-equality reasoning is backed by an end-to-end artifact diff, and neither is this change. Every claim here is crafted-fixture evidence. The byte-identical contract still needs a before/after `blake3_file` of the quant tables and `seed_psms.parquet` on a real run, including one banded `groups.window_groups` run. cargo clippy -p mumdia --all-targets -D warnings clean; 283 lib + 5 bin + 2 + 3 integration tests pass. `cargo fmt --check` still reports pre-existing diffs in `pool.rs` and `mumdia-io/table.rs`, untouched here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An adversarial review of 1ac1b44 raised seven findings against the features stage. Five are real and fixed here, one is rejected on arithmetic, and one is answered with the evidence it said was missing. The stage's outputs are unchanged: a golden captured from 1ac1b44^ now proves that in the suite. FIXED - The confident-bounds pass re-opened the chromatogram file and re-parsed its whole footer once per span (`TableFile::open_rows`), and `ChromStream::open` opened it a second time for `column_names(path)`. Both are gone: `confident_global_bounds` takes the caller's already-open `&TableFile` and uses `TableFile::span` (an Arc clone of the parsed footer, and the call that method exists for and had no callers), and the stream answers "does this table carry frag_obs_mz?" from the handle's schema, which a span carries in full. The cost used to grow with the saving: the pruning's whole purpose is to produce many spans, and each one cost two footer parses. `the_chrom_stream_reads_its_optional_column_from_the_handle_it_is_given` pins the old answer across has-column x whole-file/span. - The PIN and the features table were committed in the opposite order. The in-line write ran `pin.finish()` then `writer.close()`, so a PIN that failed at the flush left the previous features table untouched; moving `close` onto the writer thread silently reversed it, publishing the table and then erroring. The PIN is now flushed inside the scope, before the commit marker, so a failure returns without the marker and the writer drops unpublished. `a_failed_pin_finish_does_not_publish_the_features_table` pins the old ordering through a `PinFinish` parameter (cfg(test) variant, not a global, so it cannot interfere with another test in the binary). - The pruning fixture wrote a fixed 6 rows per candidate into 12-row row groups, so no candidate ever straddled a boundary and the safety argument the pruning rests on was never exercised. The fixture now varies the fragment count (4..9); the test asserts that candidates do straddle boundaries, that a skipped group sits between two kept ones, and that more than one span comes back, and it adds a chunk size larger than a row group. - The claimed benefit of the pruning is a property of that fixture. Measured: extract writes 1<<16-row groups, so one group spans thousands of candidates while the confident set (~21,856 on AIF) is spread over the id range, and essentially every group holds an anchor. `row_group_pruning_saves_nothing_on_a _production_shaped_table` builds that distribution and asserts the pruning returns ONE span covering the whole table. The doc comment now says so instead of quoting the fixture's 40%, and says what it does buy (a clustered confident set reads 3 of 200 groups). Kept, not dropped: it is exact, it now costs no extra file open, and the sparse case is real. - `in_flight_peak` asserted `2 * max_matrix_bytes`, which was the in-line write's peak (one matrix plus its copy). With the writer thread there is no copy but the previous chunk's columns are held while the next chunk's matrix AND its extended buffer are live, so the overlap is nearer 2.9x at Extended -- above what it replaced. The report measures the three buffers now. `the_in_flight_peak_is_not_two_matrices` pins the arithmetic. ALSO, from the review's output-equality section - `align_shared_axis` decided "same axis" by `ptr::eq` plus a length check, so the fast path's correctness rested on a `ChromChunk::axis_for` invariant nothing in the function could see. It compares bit patterns now (pointer equality kept as the cheap accept), which keeps -0.0 and +0.0 distinct and so accepts exactly the set the union build reproduces. One existing assertion that pinned the old rejection was updated; the equality assertion it carried is unchanged. - `confident_row_spans` refuses statistics that cannot describe a u32 column (negative or inverted bounds) rather than skipping a group on them, which would drop an anchor and shift the half-widths that bound every candidate. - `charge_states_per_row` asserts the charge column covers every PSM row. Both builds zip, and a zip truncates where the `charge[i]` it replaced panicked. REJECTED - "The per-column `ValueMatrix` multiplies the live block count." The arithmetic does not support it. 387 columns of one chunk are 387 blocks; at most two matrices are in flight, so 774 -- 0.07% of the 1,048,576 mappings a process gets, against the 92,030 + 36,368 medium mappings measured on the process that hit the limit. What the layout removes is a whole-matrix copy at every write, hundreds of MB. The trade is lopsided in favour of the columns. The arithmetic is now in the type's doc comment and `value_matrix_columns_are_one_block_each` pins the count so a future widening cannot creep past it. The sub-point that `payload_bytes` became a formula rather than a measurement is granted: it sums the columns' capacities now, so a moved-out column counts as the zero it is. - The finding that `extended_features_are_chunk_invariant` compares new against new, and that `colix_covers_every_active_column` cannot see a transposed pair, is correct and is answered rather than argued with. Route taken: the golden, not the cheaper ColIx-name settlement (both are here). `extended_features_match_the_pre_permutation_build` pins a digest of every column's bit patterns, and the PIN hash, captured by running this same fixture against 1ac1b44^ -- the last commit whose assembly keyed each value by a feature-name string literal. Verified non-vacuous: swapping `ff.frag_corr` and `ff.frag_cosine` between their two columns fails it, while chunk invariance still passes, which is exactly the gap the review named. It also covers the uniform-shift case and the `align_shared_axis` widening above. `colix_resolves_each_field_to_the_column_its_own_name_denotes` is the second layer: `stringify!` from the same macro that builds the struct, so each field is pinned to the column its own name denotes without listing 387 names. Not touched, reported instead: `cargo fmt` reformats `mumdia-io/src/table.rs` and `stages/pool.rs`, which are committed unformatted on this branch and are outside this change's file set. cargo clippy -p mumdia --all-targets -D warnings and cargo test --workspace pass; rustfmt --check is clean on the files touched here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
136 GB of one run's band artifacts: 3 min 12 s spliced at 2.6 GB resident, against 3.5 MB/s decoding and re-encoding on one core, which had reached 42.7 GB in 2 h 50 min when it was stopped. The disk does 221 MB/s. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…wo byte changes Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e window scan Four scratch structures in the per-candidate pass, plus a linear scan over every isolation window. - The distinct-fragment set was `hits.iter().map(|h| h.frag).collect::<Vec<u16>>()` followed by a sort and a dedup: one `u16` PER HIT, and a candidate with no `run_windows` row collects 10^4-10^5 hits, so that was a 20-200 KB allocation and an n log n sort to recover at most a few dozen distinct ordinals. It is now a bitmask over the candidate's local fragment ordinals (`FragSet`, 512 inline, spilling to a sorted vector above that, which no real library reaches). The sorted vector is materialised only where the co-elution score asks for it, which the default gate mode does not. - The grid alignment built a `HashMap<u64, usize>` of the scan grid per candidate. Both sides are ascending, so it is a merge; the match is still on the exact bit pattern, so a group whose RT is not a grid RT is dropped exactly as before. - The chromatogram emission built a `HashMap<u16, Vec<(f64, f32)>>` over the whole candidate and then a `HashMap<u64, f32>` per fragment to sample it onto the grid. In grid mode `groups` IS the grid -- it was rebuilt on it above, one entry per grid RT in the same order -- so a fragment's trace is its value in each group, read directly. The RT axis is the grid itself, the same for every fragment and for the MS1 XICs, so it is converted once instead of once per fragment. In sparse mode the trace is the fragment's entries in group order, which is what the per-fragment vector held before its already-sorted stable sort. - `wsum` (per-fragment intensity-weighted observed m/z) was a `HashMap<u16, ...>` over ordinals that are `0..n_frag` by construction; it is now indexed. An untouched entry keeps `sum_w == 0`, which takes the same theoretical-m/z fallback the absent map entry took. - The scan grid scanned all ~150 windows per candidate to find the covering ones. `windows` is sorted by lower m/z, and a covering window has `lower_mz <= pm` and `lower_mz >= upper_mz - max_width >= pm - max_width`, so the span is two binary searches; the membership test inside it is unchanged. With one covering window the slice is already ascending, so the sort is skipped (the dedup still runs). No speed claim from the fixture available here. On the AIF benchmark (1,691,048 candidates, 465,806 scans, 152 windows, 728,814 candidates with evidence, 41,677 accepted) the extract compute phase is 2.12-2.16 s with and without these changes over five warm runs each, i.e. flat within noise. That fixture cannot resolve them: it is I/O-dominated (4.0 s total, ~1.8 s of it loading a 309 MB spectra table), its `run_windows` covers every candidate so the 10^4-10^5-hit population these target never occurs, and it averages about two hits per candidate. The first numbers measured for this change were cold-cache and are not real. Risk to output equality: none intended. Verified end to end against the base binary on that fixture, both `--release` from the same tree: `psms_extracted.parquet` is BYTE-IDENTICAL, and `chromatograms.parquet` is equal as a table (620,169 rows, every column, same order, `pyarrow` equality) -- its bytes differ only because the preceding commit moves the row-group boundaries, which the flush-unit change documents. Tests: `FragSet` is asserted against sort+dedup over several ordinal patterns, including the spill past the inline mask; the covering-window span is asserted to select exactly the windows a full scan selects, over uneven overlapping windows, a gap, and 2,000 precursor m/z probes plus every window edge. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… trusting a stale in-tree binary The committed CLI reference was missing the whole `pool` subcommand: the generator prefers `rust/mumdia/target/release/mumdia` over the target directory cargo actually reports, and on a machine whose `build.target-dir` is redirected off the synced tree that path holds whatever was built before the redirect. Here it was a binary from 2026-09-09. CI has no such directory, which is the only reason it noticed. The generator now asks cargo first, from inside the workspace, the way ci/smoke.sh already does. Also: the third-party licence inventory picks up the zstd crates, and a public doc comment in mumdia-io no longer links to a private item, which cargo doc refuses under -D warnings. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ory and buys nothing Item (3) of the brief was to hoist the per-peak setup recomputed once per sub-range task: `peak.mz / mass_off.factor_at(peak.mz)` and the bin computation (a `ln()`). Only the first half is reachable from this file -- the bin is computed inside `FragIndex::probe_peak_win` and using a precomputed one needs a new entry point on `FragIndex`, which is outside `extract.rs`. This reverts the first half as well, because it measured as a loss. The buffer is 8 bytes per peak of every window in flight. Measured peak RSS on the AIF fixture (39.6M MS2 peaks, 152 windows), two runs per arm on the same host: with the whole run in one batch (`windows_in_flight: 400`, which is the shape a grouped band search has, since `window_groups` leaves a band 1-3 windows and one batch) the buffer covers every peak of the run, 317 MB, and the process went to 2,913 and 2,921 MB against 2,693 and 2,719 MB for the baseline -- the flat accumulator's saving swallowed and then some. Under the default `windows_in_flight` it is a sixteenth of that, but still not free. It buys no measurable time. The extract compute phase is 2.12-2.16 s with the hoist and 2.12-2.16 s without, five warm runs each; the redundant work it removes is one f64 division per peak per task (the masscal grid, where the binary search would also be saved, is absent on this run: `mz_cal_grid=0`). Memory is the whole point of this branch, so a structure that is hundreds of MB in the shape that is failing in production and flat in time does not belong in it. Both halves are recorded in a comment at the call site so the next reader does not re-derive them. Also records, at the counting sort, that giving back the hit buffer's doubling slack with `shrink_to_fit` was tried and reverted too: 2,520-2,560 MB against 2,456-2,505 MB, i.e. the copy costs more than the slack it returns, and the slack is the same slack the per-candidate vectors carried before. Output equality unchanged: re-verified on the same fixture with the final binary, `psms_extracted.parquet` byte-identical to the baseline's and `chromatograms.parquet` equal as a table (620,169 rows, every column, same order). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e values are The digest the review asked for was captured on Windows and fails on Linux. It is not nondeterminism: the same fixture gives 0x4e43..a1cc on Windows and 0x6137..c42a on Linux, stably, two runs each on the same binary. The extended battery reaches `ln`, `exp` and `powf`, which are the platform's libm and agree only to within the last bit, so the engine's f64 feature values are not bit-identical across operating systems. Worth knowing on its own: any comparison of feature values, or of any artifact hash downstream of them, has to be made on one operating system. The PIN is unaffected and its hash is the same on both, because it formats to fixed decimals. The test now carries a digest per platform and asserts the one for the platform it runs on; a platform with no entry checks the shape and the PIN and says so, rather than failing on a constant captured somewhere else. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… the allowlist Two items from the brief, both scheduling/lookup only, neither touching a value. (7) The consumer runs on the thread that called `accumulate_groups`, and under `groups.parallel` that thread is itself a rayon worker, so a plain blocking `recv` parks one worker per band in flight: 24 bands parked 24 of 32 threads and the pool ran on what was left. It now calls `rayon::yield_now` while waiting, which executes one queued task instead of parking -- usually one of this batch's own probes -- and falls back to a 1 ms blocking wait when the pool reports `Idle`, so an empty pool does not become a spin. The results are still consumed in arrival order and reordered by sub-range, so nothing about the output depends on it. The per-candidate merge this thread used to do is gone with the flat accumulator; what is left is the gather and the flush. (6) The `--restrict-candidates` allowlist was a `HashSet<u32>` probed once per VERIFIED POSTING, inside the probe callback: a hash and a bucket load for every fragment match of every peak of every scan, on all six probe sites. Candidate ids are dense `0..ncand`, so it is now a bitset (`CandMask`): a shift and a load, in one allocation of `ncand / 8` bytes instead of a hash table -- smaller as soon as the allowlist holds more than about a sixty-fourth of the library, which is the case the allowlist exists for (a gate-on pass over a 35M-83M candidate immunopeptidomics library). Ids outside `0..ncand` are absent from the mask, which is not a behaviour change: the probe only ever asks about ids it produced. The other half of (6), slicing and zipping the posting arrays in the verify loop to drop bounds checks, is NOT done: that loop is `FragIndex::emit_range` in `matchers/fragindex.rs`, outside the one file this work may touch. The ppm predicate is untouched either way, as the brief requires. Verified on the AIF fixture (1,691,048 candidates, 465,806 scans, 152 windows) against the base binary, both `--release`, on the plain path AND on the allowlist path (`--restrict-candidates` over the plain run's 41,677 accepted candidates, which the mask counts identically): `psms_extracted.parquet` is BYTE-IDENTICAL in both, and `chromatograms.parquet` is equal as a table in both (620,169 rows, every column, same order), its bytes differing only through the row-group boundaries the sub-range flush moves. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The flat CSR hit accumulator, branched from main, meets the fan-out divisor and the library-wide ids this branch already carried. The accumulator's own two hunks are taken from the rewrite; `sibling_bands` is reapplied on top, because under `groups.parallel` every band in flight sizes its probing fan-out from the whole pool, and 24 bands each asking for twice the pool is thousands of live narrowed-bin caches. The in-file test's call site passes 1. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The reference records the environment reads whose name is not a literal by source line; the CSR accumulator moved three of them. Co-Authored-By: Claude Opus 5 (1M context) <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
1 participant
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.
Six subsystems, one owner each, every change adversarially reviewed and the review acted on.
The engine now searches a 203M-precursor library in isolation-window bands without dying, and
the stage that was about to cost 40 hours on the running experiment costs 3 minutes.
Why
A grouped search of the immuno library dies at ~290 GB resident with 1.7 TB free, reporting
memory allocation of 3072 bytes failed. It is not out of memory: it is out of per-processmemory MAPPINGS. Sampled on the live process at 180 GB:
Those sizes are one-heap-block-per-item structures under mimalloc, which maps large blocks
individually. So the work here is about the NUMBER of live medium blocks, not about bytes.
What each area did
labelcolumn read as one bit per row instead of oneString(203M blocks, 6.5 GB),
candidate_idverified streaming and never held,prec_mzmovedrather than copied, the fragment payload released once the seed's index exists (7 GB), and
a bug: the window-width survey counted the run's windows rather than the band's, so on 63
bands the median was always 1 and a warning about a malformed isolation window fired 63
times per run for a run that has none.
HashMap<key, Vec<usize>>grouping replaced by one sortedVec<(key, row)>(one small block per PSM removed, 100 MB in one allocation for a 3.1M-PSM pool), a
pass-through instead of
takeon all 398 columns when a batch loses nothing (the normalcase under the shipped grouping), the peptidoform and label columns streamed, and the
competed table's row groups capped at 131,072 rows, which is what rescore decodes.
Vec<f64>per PSM, dense peptidoform ids and acharge bitmask instead of a
HashSetper peptidoform, the column permutation precomputedonce instead of ~390 string-hash lookups per row, the encode moved to a writer thread, and
the trace alignment built once per PSM with a fast path when the fragments share an axis.
heap blocks per row, one union map instead of two, the fixed window computed once, and the
fragment areas memoised per candidate rather than per PSM row.
materialising the engine's own
FeatureMatrixfirst, which underrescore.strictwith asidecar classifier is ~250 GB at experiment scale that need not exist; the per-fold training
matrix flattened from one block per training row (320M live blocks at scale).
table was parsed ten times to read nine columns), and
write_tablechunked so a table isnot built as one Arrow copy before a byte is written.
The accumulator, which is the change the mapping census asked for
extractgave every candidate with evidence its ownVec<Hit>, grown by doubling. Acandidate with no
run_windowsrow gets infinite retention-time bounds and collects hitsacross the whole gradient, which is 10^4 to 10^5 hits, 240 KB to 2.4 MB in one block: those
are the 64 KB-1 MB mappings the census counted. The accumulator is now CSR -- ascending
candidate ids, offsets, one flat hit buffer. A sub-range task collects flat pairs and groups
them with a stable counting sort applied in place by cycle following, so the hit order is
what it was: ascending scan order within a task, tasks concatenated in window order. The
window batch is no longer the flush unit either; a sub-range flushes once every window that
intersects it has passed, which is what a band of a 63-band plan needs, since it overlaps so
few windows that the whole band used to be one batch.
Live medium blocks go from one per candidate with evidence to one per task, which is twice
the thread count. On the author's fixture the instrument reads 728,814 candidates with
evidence before and
largest_open_payload0.367 GiB before against 0.000 after with thewhole run in one batch, the shape a band has.
Measured on a real 2.87M-candidate band, same accepted rows and same chromatogram rows in
both arms: 25 GB resident against 29, and 2:48 against 3:04. The mapping count on that band
is unchanged at ~265, because its candidates all have retention-time windows, so no
candidate ever reaches the hit count that produces a large block; the census that found
129,393 and later 434,243 mappings was taken on the whole grouped run, with eight bands in
one process. That measurement is the one still outstanding, and it needs a grouped run
rather than a band.
The pool, which was the emergency
Pooling a grouped run's bands decoded every row group and re-encoded it. Measured on the
running experiment: 3.5 MB/s at 110% CPU, 42.7 GB in 2 h 50 min, on a disk that reads
221 MB/s. Seven runs of that is about 40 hours of pure re-encoding.
A band's rows are already in the pooled table's order and already encoded, so
poolnowsplices the parquet column chunks into the output as bytes (
SpliceWriter), decoding only therow groups whose
candidate_idstatistics say they hold a candidate the overlap dedup drops.Same 63 bands, same 136 GB:
mumdia pool --groups-dir <run>/groupsexposes the stage, so a grouped search that finishedwhile the run did not costs a pool rather than a re-search.
A deadlock nobody had hit
groups.parallel >= --threadsdeadlocks: each band in flight parks a rayon worker on itsextraction's accumulation channel, so with as many bands as threads there is no worker left to
feed them. Reproduced on the fixture at
parallel = 2, --threads 2, where the process sat at0.1 s of CPU indefinitely with no error and no output. The value is now clamped to
threads - 1with a warning. Production runs 8 bands on 128 threads, which is why this hadnever appeared.
Evidence
previous build. The three that differ are the two pooled tables, which are spliced rather
than re-encoded (same rows, same values, same schema, verified with pyarrow), and the pooled
psms_extracted, which is no longer written because its only reader is the candidate audit.PSMs,
chromatograms.parquetbyte-identical,psms_extracted.parquetidentical in everyvalue and 2,797 bytes different in page framing, which is the documented consequence of the
chunked write on a column that is entirely NULL (
apex_im).previous binary's, 40 of 40.
relaunched on this branch and its band artifacts compared with the previous binary's, band
for band.
chromatograms.parquetandfeatures.parquetare byte-identical; the two thatdiffer hold identical values (814,000 rows, 398 columns, verified with pyarrow) and differ
only where this branch intends:
psms_competedmoves from 1 row group to 7 under the131,072-row cap, and
psms_extractedkeeps its single row group with 2,797 bytes ofdifferent page framing.
not bit-identical between Windows and Linux. The same fixture digests to 0x4e43..a1cc on
Windows and 0x6137..c42a on Linux, stably on both, because the battery reaches
ln,expand
powfand those are the platform's libm. The PIN is unaffected, being formatted tofixed decimals. Any comparison of feature values, or of an artifact hash downstream of
them, has to be made on one operating system.
The reviews
Every change was reviewed by an agent told to refute it, and every review is acted on in a
follow-up commit that says what was fixed and what was rejected and why. What that caught,
among others: a null gate silently dropped in compete's pass-through; a byte-equality claim
for the chunked write that does not hold for an entirely-null column (measured: 16 bytes of
page framing, rows identical, now pinned by a test that says so); the rescore handoff being
written in full before the non-finite check that used to precede it; a span read that
re-parsed the whole parquet footer twice per span; and a column permutation whose test proved
totality rather than correctness, now pinned against a golden captured from the pre-change
code, with a deliberate transposition shown to fail it.
Not every finding survived. Three were rejected in writing with the arithmetic: the per-column
value matrix is 774 mappings against a million-mapping limit, not a regression; the seed
scratch sizing needed a conditional rather than a revert, because
map_initruns per rayonleaf before the leaf knows whether it drew a served group; and a
u32::MAXassertion isunreachable at any allocatable scale.
🤖 Generated with Claude Code