common_ancestors + children_of: the dense-output batch pair (issue #156 phase 3) - #164
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #164 +/- ##
==========================================
+ Coverage 95.27% 95.31% +0.04%
==========================================
Files 12 12
Lines 1587 1601 +14
==========================================
+ Hits 1512 1526 +14
Misses 75 75
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
| .ok_or_else(|| format!("{} parents x {width} children each overflows", words.len()))?; | ||
| // Allocated once at the exact final size: no growth realloc, so no | ||
| // old-plus-new transient in the peak (see the module's memory posture). | ||
| let mut out = vec![0u64; total]; |
There was a problem hiding this comment.
🤖 from Claude (review)
[high] children_of aborts the whole process on a large-d allocation, where the scalar loop it replaces raises a catchable MemoryError — and the checked_mul guard never fires below that point.
Measured on this branch (rebuilt with maturin develop --release, macOS/arm64, 64 GiB, 10 cores), each row a fresh subprocess:
| call | scalar np.stack([generate_morton_children(...)]) |
batch children_of |
|---|---|---|
| 1 order-0 parent → order 29 (2.00 EiB) | MemoryError: Unable to allocate 2.00 EiB ..., exit 0 |
memory allocation of 2305843009213693952 bytes failed → SIGABRT, exit 134 |
| 12 order-0 parents → order 25 (96 PiB) | MemoryError: Unable to allocate 8.00 PiB ..., exit 0 |
SIGABRT, exit 134 |
| 1M order-6 parents → order 14 (d=8, the shipped zagg shape, 488 GiB) | — | SIGKILL, exit 137 |
vec![0u64; total] on this line goes through Rust's infallible allocator, so an allocation failure is handle_alloc_error → abort(). There is no Python exception, no traceback, and no except that can see it — the interpreter is gone. On a Lambda worker that is a silent worker death rather than an error a caller can catch and retry at a coarser order.
The checked_mul above is decorative in practice: it only trips at n * 4**d >= 2**64 elements, i.e. ≥64 parents at d=29. I confirmed that case does return cleanly —
ValueError: 64 parents x 288230376151711744 children each overflows
— but every reachable blowup (2 EiB, 96 PiB, 488 GiB) sits below the guard and aborts instead.
This also inverts the premise of PR question (2), option (a): "adding one would refuse calls the Python loop it replaces completes". The Python loop does not complete them — np.arange(span, dtype=np.uint64) in generate_morton_children raises a catchable MemoryError at exactly the sizes where the batch aborts. A guard here would therefore move the batch closer to the scalar's observable behaviour, not away from it. That is the same reasoning #108 used to make moc_to_order's ceiling a catchable ValueError, and the same reasoning PR #160 used to fence a PanicException the batch could not otherwise surface.
The budget question (a/b/c) is espg's to rule, but the abort is independent of it: even with no max_cells, replacing vec![0u64; total] with a try_reserve-based allocation (or a total * 8 ceiling check returning Err(String)) turns this into a ValueError and needs no new constant.
Repro:
import numpy as np, mortie
from mortie import _rustie
w = np.asarray(_rustie.rust_nested2mort(
np.arange(12, dtype=np.uint64), np.zeros(12, np.uint8)), np.uint64)
mortie.children_of(w, 25) # -> SIGABRT, exit 134, no Python exceptionThere was a problem hiding this comment.
🤖 from Claude
Fixed in 48e230d. vec![0u64; total] is now try_reserve_exact + resize, so the failure comes back as Err(String) and the binding turns it into ValueError. Verified on this branch, each row a fresh subprocess, rebuilt with maturin develop --release:
| call | before | after |
|---|---|---|
| 1 order-0 parent → order 29 (2.00 EiB) | memory allocation of 2305843009213693952 bytes failed, exit 134 |
ValueError: 1 parents x 288230376151711744 children each needs 2305843009213693952 bytes; allocation failed, exit 0 |
| 12 order-0 parents → order 25 (96 PiB) | exit 134 | ValueError: ... needs 108086391056891904 bytes; allocation failed, exit 0 |
| 64 order-0 parents → order 29 | ValueError: ... children each overflows |
unchanged |
| 100k order-6 parents → order 11 (819 MB) | works | works — the guard is not a de facto budget |
Three tests, all in 48e230d:
test_children_oversized_result_raises_instead_of_aborting— asserts a plainexcept ValueErrorsees it (the PR Batch MOC densify: mocs_to_orders + the mortie/moc.py extraction (issue #156) #160 pattern you cite), and that thechecked_muloverflow message is still separately reachable.test_children_oversized_result_leaves_the_process_alive— the one that is actually the point:pytest.raisescannot observe anabort(), so this runs the repro in a subprocess and asserts exit 0 plusALIVEon stdout.children_oversized_result_errors_instead_of_abortingon the Rust side, pinning the byte count in the message.
Your point about PR question (2)'s premise is taken, and the question is rewritten. It now leads with the correction — the loop does not complete those calls, np.arange raises catchably — and I moved my own lean to (b) max_cells=None on the strength of it.
One thing your table's third row does not fix, and I want it on the record rather than implied away. The 1M order-6 → order 14 case (d=8, 488 GiB) is still exit 137. try_reserve returns Ok there — macOS accepts the reservation and the kernel kills the process later, while the block is being written. That is not a batch-vs-scalar gap: I measured numpy doing the same thing, which is the honest comparison your — left open:
np.empty((1_000_000, 65_536), np.uint64) -> ALLOCATED ok, nbytes 524288000000
np.empty((12, 1125899906842624), np.uint64) -> MemoryError: Unable to allocate 96.0 PiB ...
So after this fix the batch and the scalar agree at every size measured: both raise catchably where the allocator refuses, both are killed where the OS overcommits and then cannot back it. Closing the second class needs a policy ceiling, which is exactly what question (2) now asks you to rule on — and I've said in the PR body that the abort fix was independent of and prior to that ruling, as you framed it.
Cost, disclosed rather than buried. try_reserve_exact + resize gives up vec![0; n]'s alloc_zeroed, so the block is memset once before the parallel pass overwrites every word of it. Median of 5 runs, same box, before → after: d=4 14.4x → 12.9x, d=8 8.6x → 7.4x; no measurable change at d ≤ 2. It is fully recoverable by writing into spare_capacity_mut() as [MaybeUninit<u64>] with a single set_len after the last chunk succeeds, but that is one unsafe line in a module that has none — unsafe in this crate is confined to arrow_ffi.rs and the C ABI export today — so I did not expand a correctness fold into it. Raised as new PR question (6) for espg.
| //! `n` words it is `g / n` of the input copy — for the t-digest consumer | ||
| //! (groups of 2-3 centroids) about a third of it — and the chunk term is | ||
| //! 2048 x 32 B = 64 KiB regardless of batch size. The input copy is | ||
| //! therefore the peak, always. Measured over 5M groups of 3 order-9 words: |
There was a problem hiding this comment.
🤖 from Claude (review)
[medium] The common_ancestors memory model omits the scalar kernel's own per-group scratch, so "the input copy is therefore the peak, always" is false for skewed groups — measured 3.00x the stated model.
common_ancestor (decimal_morton.rs:429) builds let mut others: Vec<(u8, u64)> = Vec::with_capacity(rest.len()) — 16 B per non-first word in the group — and holds it for the whole reduction, once per group in flight. The model documented here (input copy + result + one 64 KiB chunk) has no term for it.
Independently measured on this branch, every input loaded from disk so the input copy is not masked by freed heap. The caveat four lines below is real and I reproduced it to 0.1 MiB — the 5M x 3 case reads 77.2 MiB built in-process versus 192.0 MiB loaded from disk, against the 77.3 / 191.9 stated here — so the load-from-disk numbers below are the comparable ones:
| shape | input | documented model | measured Δpeak | ratio | excess |
|---|---|---|---|---|---|
| 5M groups x 3 words, order 9 (the case quoted here) | 152.6 MiB | 190.8 MiB | 191.5 MiB | 1.00x | 0.7 MiB |
| 40 groups x 1M words, order 9 | 305.2 MiB | 305.2 MiB | 458.4 MiB | 1.50x | 153.2 MiB |
| 1 group x 20M words, order 9 | 152.6 MiB | 152.7 MiB | 458.6 MiB | 3.00x | 305.9 MiB |
The excess is exactly the missing term, to within a MiB:
- single 20M-word group:
16 B x 19,999,999= 305.2 MiB (measured excess 305.9) - 40 groups of 1M:
10 cores x 16 B x 1,000,000= 152.6 MiB (measured excess 153.2)
So the bound is min(threads, n_groups) * 16 B * max_group_size, not a flat 64 KiB — it scales with the largest group, which is the one dimension the documented case (groups of 3) holds constant.
The quoted measurement is accurate and for the t-digest consumer this term is noise. The problem is the generalisation: "always" is what a caller sizing a worker will read, and #162 exists because a memory claim was carried past the shape it was measured on. Suggest scoping the sentence to small groups and stating the scratch term alongside the chunk term; the same sentence is mirrored in mortie/moc.py's docstring ("the input copy is the peak").
There was a problem hiding this comment.
🤖 from Claude
Fixed in 6555542. "The input copy is therefore the peak, always" is gone; both docstrings (decimal_morton/batch.rs module header and mortie/moc.py) now state a four-term model with the scratch named and bounded:
peak is input copy + result + one 64 KiB chunk + the reduction's own scratch, where the scratch is
common_ancestor'sothersbuffer — 16 B per non-first word in the group it is reducing, held for that whole reduction, one per group in flight — so the term ismin(threads, groups) * 16 B * max_group_size. It scales with the largest single group, not withn.
That last sentence is the part I think matters most and it is your finding, not mine: the term is invisible in the consumer shape and dominant in a skewed one precisely because it keys off the one dimension the quoted case holds constant.
Re-measured independently on this branch, every input loaded from disk, same instrument across rows:
| shape | input | documented model | measured Δpeak | ratio | excess | formula predicts |
|---|---|---|---|---|---|---|
| 5M groups × 3 words, order 9 | 152.6 MiB | 190.8 MiB | 191.9 MiB | 1.01x | ~0 | < 1 KiB |
| 40 groups × 1M words, order 9 | 305.2 MiB | 305.2 MiB | 458.6 MiB | 1.50x | 153.4 MiB | 152.6 MiB |
| 1 group × 20M words, order 9 | 152.6 MiB | 152.7 MiB | 460.0 MiB | 3.01x | 307.3 MiB | 305.2 MiB |
Within 2 MiB of your 458.4 / 458.6 on both skewed rows, and the excess matches the formula to within 2 MiB in both — so the term is identified, not just observed. The PR body carries the same table.
Both docstrings now close on the two regimes rather than one rule: size a worker off input + result for many small groups, and off the largest group when groups are large.
I did not change the kernel to eliminate the buffer (it is a real second pass over the group's decoded words, so removing it is a rewrite of common_ancestor, not a batch-layer change) — that would be a scalar-side optimisation and is out of this diff's scope. Say if you want it filed.
| //! * [`common_ancestors`] validates only the *layout* up front and leaves the | ||
| //! per-group failures (an empty group, an undecodable word, words spanning | ||
| //! more than one base cell) to the parallel pass. Hoisting them into the | ||
| //! pre-pass would break the lowest-index rule, not preserve it: the cheap |
There was a problem hiding this comment.
🤖 from Claude (review)
[low] The lowest-index guarantee holds across domain classes (verified) but not across the layout/domain split — and the reason given here for the asymmetry only holds for a partial hoist.
Three things, all about this paragraph.
(1) The domain-class behaviour is correct and I could not break it. With offenders straddling the CHUNK seam and 200 repeats per configuration, the lowest index always wins across differing failure classes:
| empty group at | mixed-base-cells at | reported |
|---|---|---|
| 4000 | 3 | group 3: inputs span multiple base cells ... |
| 3 | 4000 | group 3: empty input has no common ancestor |
| 2049 | 2047 | group 2047: ... multiple base cells |
| 2047 | 2049 | group 2047: empty input ... |
| 0 | 5000 | group 0: empty input ... |
| 5000 | 0 | group 0: ... multiple base cells |
200/200 runs of the first two rows each produced a single distinct answer. So the shipped behaviour is what this paragraph promises.
(2) Layout errors are in a different pass, so they outrank domain errors regardless of index — the same inversion this paragraph says it is avoiding, on the other axis:
v, o = ... # group 0 spans two base cells (domain error at index 0)
o[8] = 99 # group 7's offset is out of bounds (layout error at index 7)
mortie.common_ancestors(v, o)
# ValueError: group 7: offset 99 exceeds value array length 20moc::batch::validate_batch does not have this gap — its domain check (the per-MOC max_cells budget) is interleaved into the same index-order loop as the offsets checks, which is exactly what line 53 credits it with. Here validate_ragged has no domain check to interleave, so the guarantee really is weaker; but the module header ("naming the lowest-index offending item"), the common_ancestors doc comment, and mortie/moc.py's Raises section all list layout and domain failures under one undifferentiated lowest-index promise.
(3) The justification as written is not the load-bearing one. Hoisting all the per-group checks into a single index-order pre-pass would preserve lowest-index, not break it — that is precisely what validate_words does with its three checks 150 lines below, and what moc::batch does. Only a partial hoist (emptiness alone, as its own earlier pass) produces the "empty group 7 ahead of mixed-base-cell group 2" inversion described here. The real reason not to hoist is cost: the mixed-base-cell check is the reduction, so a pre-pass would run the whole kernel twice. That is a stronger argument than the one written, and worth correcting because later phases will copy this paragraph.
There was a problem hiding this comment.
🤖 from Claude
All three points taken; fixed in 826e793.
(3) — the justification. You are right that the written reason is false as stated, and right about why: a full index-order pre-pass preserves lowest-index, and validate_words 150 lines below is the counterexample in this same file. The module header now says the load-bearing reason:
The reason is cost, not ordering: the mixed-base-cell check is the reduction — it decodes every word in every group — so a pre-pass that ran it would run the whole kernel twice. Hoisting per se would not break the lowest-index rule: a single index-order pre-pass doing all the checks preserves it, which is exactly what
validate_wordsandcrate::moc::batch's validator do. Only a partial hoist would invert — emptiness alone, as its own earlier pass, would report an empty group 7 ahead of a mixed-base-cell group 2.
Your "later phases will copy this paragraph" is the reason I rewrote rather than deleted it. The partial-hoist inversion is kept as the parenthetical it always should have been.
(2) — the layout/domain inversion. Reproduced exactly:
domain only -> group 0: inputs span multiple base cells and have no common ancestor; ...
layout at 7 -> group 7: offset 99 exceeds value array length 20
Now documented as deliberate rather than left implicit, with the reason stated: a layout failure says the offsets array itself is wrong, and the group indices a domain error would be reported by are read out of that same array — so "your offsets are broken" is the failure a caller has to fix first. And I took your moc::batch contrast into the text: it has no such split only because its domain check (a per-MOC cell budget, computable without reducing) is cheap enough to interleave; this op's is not, which is the same cost argument as (3).
Documented and pinned, so it cannot drift silently now that it is a stated contract:
ancestors_layout_errors_outrank_domain_errors(Rust) — group 0 domain error on its own, then a group-2 layout error added, which outranks it.test_ancestors_layout_errors_outrank_domain_errors(Python) — your exact repro shape,o[8] = 99against a mixed-base-cell group 0.
The undifferentiated promise you flagged in three places is split in all three: the module header, common_ancestors's # Errors section, and mortie/moc.py's Raises now all say the lowest-index rule holds within a pass — layout first for the whole batch, then domain in index order — instead of implying it holds across both.
(1) — nothing to do, and I did not touch it. Your 200-repeat sweep is the evidence that the shipped behaviour is correct; I only corrected the comment in ancestors_errors_name_lowest_index_group, which claimed the mixed-base-cell-beats-empty case was "the whole reason emptiness is not hoisted into the pre-pass". Both are domain failures found in the same pass, so the lower index wins regardless of which check is cheaper — which is your point, restated where a future reader will hit it.
| of the scalar without it. | ||
|
|
||
| Note the result grows as ``4**d``: at large ``d`` this is a memory-bandwidth | ||
| measurement, not a boundary-cost one, and the speedup falls accordingly. |
There was a problem hiding this comment.
🤖 from Claude (review)
[low] The d=8 shape zagg actually ships measures 8.7x — below the table's 15.4x low end, not "nearer the low end" of it.
Measured on this branch with real order-6 shard keys (the in-tree Antarctic drainage-basin fixture coarsened with clip2order(6, ...), 644 distinct keys), refined to order 14 — the 65,536 cells/shard shape the PR body cites:
n=2000 parents, 6 -> 14 (d=8, 65,536 children/parent), result 1000 MiB
scalar np.stack loop : 2.79 s
batch children_of : 0.32 s
speedup : 8.71x
For calibration, my reruns of the published rows on the same box (10 cores) also come in under the table — common_ancestors 12.8x (100k x 3) and 13.4x (500k x 2); children_of 63.8x (d=1) and 13.5x (d=4) — so part of the gap is machine load. But the decay is monotone and continues past d=4 rather than flattening inside the published range, which is the substantive point: d=8 is the only row a zagg or moczarr reader will look for, and it is the one row that is predicted rather than measured. Either add the measured d=8 row to the table, or reword to "below the low end".
There was a problem hiding this comment.
🤖 from Claude
Taken on both counts; fixed in 41e4508. The optimistic set is gone — I did not keep it and explain the gap, because re-measuring showed there is nothing to explain except run-to-run variance, and the published numbers sat at the top of it.
Republished as medians of 5 runs with the observed range, same box (10 cores, macOS/arm64), post-fold build. Both benchmark scripts now carry this table in their module docstring, so it is reproducible from the file rather than only from the PR:
| case | result | median | range over 5 runs | was published |
|---|---|---|---|---|
common_ancestors, 100k groups of 3, order 9 |
— | 17.3x | 15.0–18.3 | 14.6x |
common_ancestors, 500k groups of 2, order 9 |
— | 19.6x | 18.1–19.9 | 18.7x |
children_of, 100k, 6 → 7 (d=1) |
3.1 MiB | 74.2x | 68.5–84.8 | 87.8x |
children_of, 100k, 6 → 8 (d=2) |
12.2 MiB | 50.3x | 48.1–55.9 | 61.7x |
children_of, 100k, 6 → 9 (d=3) |
48.8 MiB | 28.2x | 26.3–28.8 | — (new) |
children_of, 100k, 6 → 10 (d=4) |
195.3 MiB | 12.9x | 12.3–13.1 | 15.4x |
children_of, 2k, 6 → 14 (d=8) |
1000.0 MiB | 7.4x | 7.1–7.5 | predicted only |
The range column is the point: at small d the batch side is a few hundredths of a second, so one shot lands anywhere in a ±20% band — d=1 alone spanned 68.5–84.8 across five runs. That is where 87.8x came from, and why your reruns and mine both came in under it. The docstrings now say to quote the median and the range.
Your substantive point — the decay does not flatten inside the published range — is now measured, not reworded. I added the d=3 row so the curve is visible (74 → 50 → 28 → 12.9 → 7.4) and the d=8 row is measured rather than predicted. The "expect d=8 to sit nearer the low end" sentence is replaced by a statement that it sits below it, and measure_children_of.py's docstring now warns in its own words: a figure quoted at d ≤ 4 does not bound d = 8; measure the d you intend to run.
Your 8.71x on the real order-6 shard keys and my 7.4x on the random-parent corpus are the same measurement either side of the fold: 8.71 vs my 8.64 pre-fix median on the same shape, and the drop to 7.4 is the memset the try_reserve fix in 48e230d costs (disclosed on that thread and in the PR body). So the two corpora agree, which is worth having established — the random-parent bench is a fair stand-in for the shipped shape.
Merging this PR will degrade performance by 14.16%
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing Footnotes
|
|
🤖 from Claude espg ruling (in-session, 2026-08-08): question (2) resolves to option (b) — add Implementing now; Two things to carry into the implementation, recorded so they are not rediscovered:
|
|
🤖 from Claude espg ruling (in-session, 2026-08-08): question (3) resolves to the stated lean — keep Reasoning on the record: deriving the width from the data beats making callers repeat what the data already says, and the empty case is already special-cased consumer-side — moczarr's No code change. A one-line note is being added to the docstring recording why the width is 1 rather than derived, so the question is not re-litigated by the next reader. That closes (3). Still open on this PR: (1) the |
|
🤖 from Claude espg ruling (in-session, 2026-08-08): question (6) resolves NO — do not take the The memset is the price of making the allocation fallible: Why not buy the 14% back with
Question (6) closed. The measured cost stays documented in the benchmark table so the trade is visible rather than lost. |
|
🤖 from Claude Both rulings are landed and pushed — This PR is now genuinely ready. Every phase, review finding, and ruling is folded. Two things from the implementation worth a reviewer's eye: The The acceptance case is the one that motivated the ruling. 1M order-6 parents → order 14 — the shipped Unset is byte-identical, verified rather than asserted: the same SHA-256 over Standing for espg: (1) |
Refs #156 — phase 3 of many (not
Closes). Phase 1 (mocs_to_orders+ themortie/moc.pydomain split) merged as42db670; this is the dense-output pair.What this does
Adds the two dense-output batches from the issue #156 audit table. They are one phase because they share a property: their results are fixed-shape arrays, so neither needs an arrow offsets pair on the way out, and both are the cheapest high-value entries in the sweep.
common_ancestors(values, offsets) -> uint64[N]Segmented reduce of
common_ancestor/moc_min. Ragged in (the arrow list layoutpolygons_to_morton_mocsandmocs_to_ordersalready use), dense out — one word per group, because the reduction is many→one per group.The consumer is a per-worker inner loop, not a one-off: zagg's t-digest merge runs
for j in np.flatnonzero(~single):atsrc/zagg/stats/tdigest.py:198— once per multi-member centroid (δ=512), per cell, per build and per fold, on every Lambda worker, at 65,536 cells/shard in the shipped ATL03 config. That module's own docstring attdigest.py:177-179already names it "the same O(n) Python-loop shape issue #279 removed".One contract decision worth flagging: an empty group is an error, not an empty slot.
mocs_to_orderslets an empty item densify to an empty slice, but a many→one reduction over no words has no answer — the scalar refuses empty input, so the batch refuses that group by index.children_of(words, order, max_cells=None) -> uint64[N, 4**d]Batch of
generate_morton_children, whose wrapper coerces to a scalar parent (tools.py:1167). Every parent must sit at one orderp <= order, so each yields exactly4**dchildren ford = order - pand the result is a dense(n, 4**d)block.That shared-order requirement is not new — it is what the consumers already assume, because the loop this replaces ends in
np.stack, which raises on rows of unequal width. moczarr wrote the gap down in its own source:src/moczarr/dggs.py:310isnp.stack([generate_morton_children(int(w), level) for w in words]), with the comment atdggs.py:302-305saying "there is still no vectorized many-parent children kernel". zagg calls the scalar the same way per sub-chunk on every worker (src/zagg/grids/healpix.py:199) and per shard in the shardmap reprojection (src/zagg/catalog/shardmap.py:708).d == 0returns the parents verbatim, matching the scalar exactly — that is what preserves aKind::Pointword, which ato_nested/from_nestedround trip would re-pack as the order-29 area cell. Pinned by a test.The result block is allocated fallibly (
try_reserve_exact, notvec![0u64; total]), so anorderwhose(n, 4**d)block the allocator refuses is a catchableValueErrornaming the byte count rather than ahandle_alloc_error→abort()that kills the interpreter. An opt-inmax_cells=Nonebudget (espg's ruling on question (2)) refuses an over-budgetn * 4**dresult before anything is allocated, which is the only thing that fences an allocation an overcommitting OS accepts and then cannot back. See "Allocation posture" below.House pattern
Per #153/#154/#160, and unchanged here:
children_ofkeeps espg's named form from the audit.offsets[0] == 0,offsets[-1] == len(values), exact coverage, both endpoints checked and each naming which one failed.order), not per-item arrays.try_reduceshort-circuiting (rejected in Batch polygon coverage: polygons_to_morton_mocs (issue #153) #154 for nondeterminism).py.allow_threads, chunked atCHUNK = 2048.Rust module placement is the house pattern too, not a deviation.
src_rust/src/decimal_morton/batch.rssits exactly where the tree already puts batch submodules —coverage/batch.rs,moc/batch.rs, and PR #158'swkb/batch.rsare all<domain>/batch.rs. That is the axis espg ruled in the plan revision item 1 ("Rust side follows the same axis"), and both scalar kernels this phase wraps (common_ancestor, and theto_nested/from_nestedpair the child generator is built from) live indecimal_morton.rs. An earlier revision of this description called it a deviation; it is not.One place this phase genuinely does differ from its siblings:
children_ofvalidates entirely up front;common_ancestorsdoes not.children_ofmust know the row width before it can allocate, so decodability / target order / shared-order all run in the pre-pass.common_ancestorsvalidates only the layout up front and leaves per-group failures to the parallel pass. The reason is cost, not error ordering: the mixed-base-cell check is the reduction (it decodes every word in every group), so a pre-pass that ran it would run the whole kernel twice. Hoisting per se would not break the lowest-index rule — a single index-order pre-pass doing all the checks preserves it, which is exactly whatvalidate_wordsandmoc::batch's validator do. Only a partial hoist would invert (emptiness alone, as its own earlier pass, would report an empty group 7 ahead of a mixed-base-cell group 2). An earlier revision of this description gave the partial-hoist inversion as the reason for the whole design; that was wrong, and the module header now says cost.The one place the lowest-index promise is qualified
For
common_ancestorsthe rule holds within each pass, not across the two. Layout is a whole-batch serial pre-pass, so a bad offset at a high index outranks a domain failure at a low one:That ordering is deliberate and now documented and pinned by a test (
test_ancestors_layout_errors_outrank_domain_errors, plus a Rust twin): a layout failure says theoffsetsarray itself is wrong, and the group indices a domain error would be reported by are read out of that same array, so "your offsets are broken" is the failure a caller has to fix first.moc::batchhas no such split only because its domain check (a per-MOC cell budget, computable without reducing) is cheap enough to interleave into the index-order layout loop; this op's is not. Among the domain classes themselves the lowest index always wins — verified with offenders straddling theCHUNKseam in both directions, 200 repeats per configuration, one distinct answer every time.On reuse: there were no
pub(crate)helpers incoverage::batchonmainto reuse (all its helpers are private, and PR #158 is unmerged).validate_raggedhere is the offsets-only spelling of the same contract. It is deliberately not folded into a shared helper withcoverage::batch/moc::batch, because each of those interleaves a domain check (the 3-vertex ring minimum; the per-MOCmax_cellsbudget) inside the index-order loop, which is exactly what makes their errors lowest-index across both kinds of failure — a shared helper would have to take that as a callback, and this op has no such per-item pre-check. Raised as a question below.Allocation posture —
children_ofmust not abort the processvec![0u64; total]goes through Rust's infallible allocator, whose failure path ishandle_alloc_error→abort(). Measured on the pre-fix branch (macOS/arm64, 64 GiB, 10 cores), each row a fresh subprocess:np.stack([generate_morton_children(...)])MemoryError, exit 0memory allocation of 2305843009213693952 bytes failed, SIGABRT exit 134ValueError: 1 parents x 288230376151711744 children each needs 2305843009213693952 bytes; allocation failed, exit 0MemoryError, exit 0ValueError: ... needs 108086391056891904 bytes; allocation failed, exit 0ValueError: 64 parents x 288230376151711744 children each overflowsThe existing
checked_mulguard only trips at ≥2⁶⁴ elements — ≥64 parents atd=29— so every reachable blowup sat below it and aborted. The fix istry_reserve_exacton the result block; no new constant, no policy.What it does not fence. An allocation an overcommitting OS accepts and then cannot back still ends in a kill: 1M order-6 parents → order 14 (
d=8, 488 GiB) istry_reserve-clean and dies at exit 137 (SIGKILL) while the block is being written. That is not a batch-specific gap — numpy accepts the same request (np.empty((1_000_000, 65_536), np.uint64)allocates 488 GiB on this box without raising, while(12, 1125899906842624)raisesMemoryError), so the scalar loop dies at the same shape. After the fix the two agree everywhere measured: both raise catchably where the allocator refuses, both are killed where the OS overcommits. Refusing the second class needs a policy ceiling — that is what the opt-inmax_cellsadded in73b0681is, per espg's ruling on question (2) below.Cost of the fix.
try_reserve_exact+resizelosesvec![0; n]'salloc_zeroed, so the block is memset once before the parallel pass overwrites it. Median of 5 runs, same box, before → after:d=414.4x → 12.9x,d=88.6x → 7.4x (≈10–15% off the largest results; no measurable change atd ≤ 2, where the result is small). The zero-fill is recoverable by writing intospare_capacity_mut()asMaybeUninitand a singleset_len— oneunsafeline in a module that has none today, which is why it is not in this diff. Flagged as question (6).Memory posture — measured, not asserted
#162 exists because merged batches claimed "result + one chunk" while omitting the mandatory input copy (
to_vec()is required for a numpy-borrowed slice to crossallow_threads). Both docstrings here state the real model, with numbers.input + resultcommon_ancestors, 5M groups × 3 order-9 wordschildren_of, 1M order-6 parents → order 9Dense output makes the model short: input copy + result + one chunk of
Results, where the chunk term is a flat 2048 × 32 B = 64 KiB regardless of batch size.children_ofadditionally allocates its block once at the exact final size, so there is no growth-realloc transient and no ragged assembly copy.common_ancestorshas a fourth term, and it scales with the largest groupAn earlier revision of this description said "for
common_ancestorsthe input copy is the peak, always". That is false for skewed groups. The scalar kernel builds its own scratch —let mut others: Vec<(u8, u64)> = Vec::with_capacity(rest.len())(decimal_morton.rs:429), 16 B per non-first word — and holds it for the whole reduction, one per group in flight. So the term ismin(threads, n_groups) * 16 B * max_group_size: it grows with the largest single group, which is the one dimension the quoted case (groups of 3) holds constant.Measured on this branch, every input loaded from disk, same instrument and order across rows:
The excess is the missing term to within 2 MiB in both skewed rows. The docstrings in
decimal_morton/batch.rsandmortie/moc.pynow state the four-term model and both regimes: size a worker offinput + resultfor many small groups, and off the largest group when groups are large. For the t-digest consumer (groups of 2–3) the term is noise, which is why the original measurement was accurate and the generalisation was not.A measurement caveat worth recording, because it bit this PR: sampled RSS under-counts the input copy when the input is constructed in-process, since
to_vecis then served out of already-resident freed heap. The same 5M-group case reads as 77.3 MiB that way versus 191.9 MiB when the input is loaded from disk (independently reproduced to 0.1 MiB). The numbers above are the load-from-disk ones.Benchmarks — honest read
benchmarks/measure_common_ancestors.pyandbenchmarks/measure_children_of.py, 10 cores, macOS/arm64, ≥100k items except where noted. Thechildren_ofscalar side is timed as the consumer actually writes it,np.stackincluded, since a caller cannot get a dense block out of the scalar without it.These are medians of 5 runs, with the observed range. An earlier revision of this description published single-run figures (87.8x / 61.7x / 15.4x / 14.6x / 18.7x); re-measuring showed the batch side is a few hundredths of a second at small
d, so one shot lands anywhere in a ±20% band and those numbers sat at the top of it. The medians below are lower and reproducible; the ranges are printed so a re-run that disagrees by 10% is not a surprise. Both scripts now carry the table in their module docstring.common_ancestors, 100k groups of 3, order 9common_ancestors, 500k groups of 2, order 9children_of, 100k parents, 6 → 7 (d=1)children_of, 100k parents, 6 → 8 (d=2)children_of, 100k parents, 6 → 9 (d=3)children_of, 100k parents, 6 → 10 (d=4)children_of, 2k parents, 6 → 14 (d=8, the shipped zagg shape)These land high, unlike phase 1's
mocs_to_orders(2.96–5.80x), and the reason is the same reason phase 1 landed low: what dominates. Phase 1 was bandwidth-bound on writing a flat result far larger than its input, so removing the boundary cost bought little. Here the per-item work is tiny (a shared-prefix walk over 2-3 words; a contiguous nested run) and the result is small or written once at exact size, so the Python boundary is the wall — which is what a batch removes.The
children_ofdecay does not flatten inside the published range, andd=8is below its low end. 74x at d=1 falls to 12.9x at d=4 as the result turns it back into a bandwidth measurement, and keeps falling to 7.4x at d=8 — the 65,536 cells/shard shape zagg ships, and the only row a zagg or moczarr reader will actually look for. An earlier revision predicted d=8 would "sit nearer the low end" of a d≤4 table; it sits below it. The row is now measured, not predicted, and the benchmark docstring says so. (Independently measured at 8.71x pre-fix on real order-6 shard keys from the in-tree Antarctic drainage-basin fixture coarsened withclip2order(6, ...), 644 distinct keys — the random-parent corpus and the real-key corpus agree.)Testing
mortie/tests/test_dense_batch.py— 51 tests, all passing (42 in phase 3a/3b, 4 folding review, 5 folding themax_cellsruling).decimal_morton::batch;cargo test --libis 297 passed / 0 failed / 1 ignored.Coverage of the acceptance list:
Ant_Grounded_DrainageSystem_Polygons.txt) — forcommon_ancestorsgrouped bysplit_base_cells, forchildren_ofat real order-9 shard keys refined the way zagg's two-step does it. Plustest_children_stacked_scalar_loop_is_reproduced_exactly, which is moczarr'snp.stack([...])expression verbatim.(0,)and(0, 1)); a group of one word (returns that word);children_ofwithd = 0(identity, and the point-word case); a word finer thanorder(refused, index named); mixed parent orders (refused, index named); undecodable words (refused, index named).ValueErrora plainexcept ValueErrorcatches (the PR Batch MOC densify: mocs_to_orders + the mortie/moc.py extraction (issue #156) #160 lesson, where aPanicExceptionescaped evenexcept Exception) — and, becausepytest.raisescannot observe anabort(), a subprocess test asserts the interpreter survives it (exit 0). Plus a 819 MB result that must still go through, so the fallible allocation is not a de facto budget.test_ancestors_lowest_index_holds_across_chunk_boundariesand itschildren_oftwin are parametrized over offenders at indices0, 1, 7, CHUNK-1, CHUNK, CHUNK+1, 2*CHUNK+5, 3*CHUNK-1and repeat each 5x; the..._wins_over_later_offenderspair plants five simultaneous offenders and heals them lowest-first, asserting the named index each time (3x per step). Plus 10-run byte-equality checks on both ops.test_threading.test_gil_released_during_rust_compute.Gates
cargo fmt --checkclean ·cargo clippy --lib --benches— no new warnings (the remaining 7 are the pre-existingcoverage/tests.rs,geo2mort.rs,prefix_trie.rsset) ·cargo test --lib297 passed / 1 ignored ·pytest1119 passed / 12 skipped ·flake8 mortie --select=E9,F63,F7,F82clean ·flake8 --max-line-length=88clean on every touched file (the threetools.pyhits are pre-existing, at lines 958/975/986 — theon_antimeridianone is #151 item 4) ·numpydoc lintclean onmoc.py,tools.py,__init__.py· doctests pass on both touched modules.Phases
common_ancestors: Rust kernel + binding +mortie/moc.pywrapper, beside its scalar twin.children_of: Rust kernel + binding +mortie/tools.pywrapper, beside its scalar twin.USAGE.md,docs/api/moc.md,docs/api/tools.md,__init__exports.max_cells, and(0, 1)kept with the reasoning recorded.Landed as one commit because the two share a Rust module and a test file; splitting them would have been file-splitting, not phasing.
Questions for review
tools.pyis now 1,451 lines, against this repo's ~1,000-line aim (CLAUDE.md §4). It was already over at 1,354 before this PR. The instruction for this phase was explicit: putchildren_ofbeside its scalar twin and note that Split tools.py and geometry.py into domain modules mirroring the Rust tree #159 will move the hierarchy/order functions toorders.pylater, and do not start the Split tools.py and geometry.py into domain modules mirroring the Rust tree #159 split here (it isblocked). I followed that, but the rule says stop and raise when a file would cross, so raising it.The remedy has a deadline that is earlier than "after the sweep."
children_ofgenuinely belongs besidegenerate_morton_children, so 1,451 is the right place to land this phase — but every remaining phase lands in the same file too: phase 4'swords_to_decimals/hive_paths, phase 5'smort2polygons/mort2bboxes. That putstools.pynear ~1,700 by the end of the sweep. So the ask is to unblock Split tools.py and geometry.py into domain modules mirroring the Rust tree #159 before phase 4, not after phase 5, so the split absorbs the batch additions instead of chasing them.mortie/moc.pyis fine at 644;src_rust/src/decimal_morton/batch.rsis 702.Second, smaller point: mortie's CLAUDE.md has no 1,200-line raise — that is zagg's standing ruling (its issues #351/#358), not this repo's, and it should not be inherited silently. If you want the same allowance here it belongs in mortie's own CLAUDE.md as its own decision.
ShouldRULED — option (b), implemented inchildren_ofcarry amax_cellsbudget?73b0681. espg ruled the opt-in form: "it costs adopters nothing and gives a Lambda worker a way to fail catchably instead of being killed."children_of(words, order, max_cells=None). Unset — the default — behaviour is byte-identical to before: same SHA-256 overchildren_ofoutput across six shapes (d=0, 3, 5, 8 spans, order-0 and order-29 edges, plus the empty batch) on the pre- and post-change builds, and thetry_reservepath and its documented overcommit residual are untouched. Set, the result'sn * 4**dcell count is checked before anything is allocated and refused with aValueErrorinmoc_to_order's wording, so the two read alike:That is the case that motivated the ruling and it is the acceptance test: zagg's shipped
d=8shape at fleet scale (1M order-6 parents → order 14, 488 GiB) is the onetry_reservecannot refuse, because the OS accepts the reservation and kills the process mid-write. With a worker's own ceiling set it comes back catchable.Why the default is the opposite of
moc_to_order's, stated in the docstring so the API does not read as inconsistent:moc_to_orderdefaults its budget on because a densify explodes from a tiny input (Σ 4**(order - depth), pre-emptive flat-cover size guard #80) and the caller cannot cheaply predict the output;children_of's output is exactlyn * 4**d, computable from the arguments before the call, so a default guard would refuse calls the caller already knows are fine. Same parameter name, opposite default, for a stated reason. TheNonepolarity inverts with it — hereNonemeans no budget, not disable a default — and that is written down too.Follows Consolidated follow-ups from the June/July sweeps (#73, #88, #97) #108's ruled semantics where they apply:
ValueError(a caller-controlled argument condition, not a failed allocation), caller-settable, negative rejected, and clamped atu64::MAXthe waymocs_to_ordersclamps.One ordering decision, made deliberately and pinned: the budget is compared in
u128and checked ahead of thechecked_mulelement-count guard, so an explicit budget answers even for a request too large to represent as ausize. 64 order-0 parents at order 29 gives... children each overflowswith no budget and... exceeding max_cells=1048576with one; likewise the budget outranks theallocation failedrefusal. Rationale: an explicit argument condition is more actionable than an internal representability diagnostic the caller cannot act on (Consolidated follow-ups from the June/July sweeps (#73, #88, #97) #108's framing). Pinned inchildren_max_cells_outranks_the_overflow_guard(Rust) andtest_children_max_cells_outranks_the_overflow_guard(Python).The residual stands and is why (b) beat (a): an unset budget still cannot close the overcommit class, and the scalar loop has the identical gap (
np.empty((1_000_000, 65_536), np.uint64)allocates 488 GiB on this box without raising). A caller-set ceiling is the only thing that closes it, which is exactly what (b) provides without changing anyone's behaviour by default.RULED — keepchildren_ofon an empty batch returns shape(0, 1).(0, 1); noparent_order=parameter. No behaviour change; the reasoning is now recorded in the docstring (722ec03) so it is not re-opened. With no words there is no source order to derive4**(order - parent_order)from, so a derived width would require callers to passparent_order— repeating, in the empty case alone, what the data carries in every other case. Consumers that need a typed empty already special-case it: moczarr'sdggs.pybuilds(0, 4**(level - order))on its own empty branch precisely because it knows its source order at that point. So(0, 1)is the honest shape for "no rows, width unknown", and the width belongs in the consumer-side special case.The three ragged validators are now structurally parallel and not shared (
coverage::batch,moc::batch,decimal_morton::batch). I deliberately did not refactor: the other two interleave a domain check inside the index-order loop, so unifying them needs a callback, and touchingcoverage/batch.rsright now would collide with PR Rust WKB reader: backend-free geometry ingest, plus from_wkbs (issue #157) #158. Worth a follow-up issue after Rust WKB reader: backend-free geometry ingest, plus from_wkbs (issue #157) #158 merges, or leave the three as they are?Should
common_ancestorsgrow asplit_base_cells-style escape? Today a group spanning more than one base cell refuses the whole call by index, matching the scalar. That is the right default, but the real Antarctic fixtures do straddle base cells — the parity test has to runsplit_base_cellsfirst to build groups the reduction accepts. If a consumer wants "reduce each group, and partition the ones that straddle", that is a different op and I would rather it be filed than smuggled in here.Should
children_ofrecover the zero-fill the fallible allocation costs?try_reserve_exact+resizememsets the block before the parallel pass overwrites every word of it — measured at ~10–15% on the large-dshapes (d=414.4x → 12.9x,d=88.6x → 7.4x; no measurable change atd ≤ 2). Writing intospare_capacity_mut()as[MaybeUninit<u64>]and callingset_lenafter the last chunk succeeds removes it entirely and is provably fully-initialised (words.len()rows ×width=total, and an early return drops the vec at len 0). It costs oneunsafeline in a module that has none today —unsafein this crate is currently confined toarrow_ffi.rsand the C ABI export. I left it out rather than expand the scope of a correctness fold; say the word and it is a two-line follow-up.