Mixed-order support in the geo kernels (issue #116)

[espg]

Closes #116

Mixed-order support in the geo kernels, per the implementation plan on the issue (the "Implementation plan (espg-requested)" comment): python-side group-by-order dispatch, zero rust changes.

Phases (one commit each)

• Phase 1 — per-element introspection surface: public orders_of(words) -> uint8[n] and is_point(words) -> bool[n], pure-numpy suffix decodes per the spec page §1 suffix table (0..=27 order == suffix; 28..=47 order-28/29 area in parent-first preorder r = t28*5 + (t29 present ? t29+1 : 0), so (suffix-28) % 5 == 0 is the order-28 parent slot; 48..=63 order-29 point). Golden-tested against the table (every band suffix, both hemispheres) and cross-checked elementwise against the rust kernel's depth decode so the numpy table and the kernel cannot drift.
• Phase 2 — infer_order_from_morton semantics fix: uniform input still returns the scalar order (unchanged, kernel-validated); mixed input now raises ValueError naming the distinct orders and pointing at orders_of (previously the first element's order was returned silently).
• Phase 3 — group-by-order dispatch in mort2geo / mort2polygon / mort2bbox: orders_of → np.unique masks → the existing uniform kernel per group → scatter back to input positions. Points group with order 29 (a point's location is exactly what mort2geo returns); in mort2polygon / mort2bbox a point now yields the geometry of its containing order-29 cell — identical to the order-29 area word at the same location — so a single point has a well-defined box/polygon and a group of points covers a well-defined area element by element.
• Phase 4 — tests: the group-dispatch-equals-uniform oracle for all three kernels (orders {0, 6, 19, 24, 29} interleaved, both hemispheres, step=1/step=2 rings, singleton-group scatter posture), the point containing-cell postures (scalar / uniform / mixed, oracled against the same-location order-29 area word), and the orders_of → order2res resolution-ladder cross-check.
• Phase 5 — point containing-cell geometry (espg review): reconsidered Phase 3's point-raise in mort2bbox / mort2polygon per espg's review. A point word decodes to the identical HEALPix cell as the order-29 area word at the same location (verified: same nested id + depth 29), so points now flow through the order-29 group and yield that containing cell's bbox / ring instead of raising. Tests flipped from raise-assertions to containing-cell oracles.

Acceptance evidence

• Suite: full local suite is green apart from one inherited main failure (test_spec_page.py::TestDecimalParseTieBreak::test_order29_string_parses_to_area_word, issue main is red: test_spec_page tie-break test predates #120's p-suffix rendering #123 — decimal p-suffix parse, untouched by this PR): 706 passed / 11 skipped / 1 inherited failure. test_mixed_order.py is 23/23.
• Uniform-input behavior pinned: uniform arrays never enter the dispatch branch (np.unique(orders).size == 1 short-circuits to the pre-existing code path untouched).
• The oracle: TestGroupDispatchOracle asserts, per kernel, that the mixed-array result equals the per-order uniform kernel call scattered back to input positions — including a singleton-group case that exercises the length-1 bare-dict/bare-ring return posture. TestPointPostures oracles a point's bbox/ring against the same-location order-29 area word.
• Suffix goldens: TestOrdersOf pins every suffix in the 28..=47 preorder band and the 48..=63 point band onto real order-29 prefix+body words in both hemispheres, plus order==suffix for 0–27, plus elementwise agreement with _rustie.rust_mort2nested depths.

Notes on existing-test changes

Two test_coverage.py assertions called infer_order_from_morton(moc) on mixed-order MOCs — under the old first-element behavior these were asserting on whichever cell happened to be first. They now use orders_of(moc) and assert over the true per-element orders (strictly stronger). This is exactly the migration the new error message prescribes.

Questions for review

1. mort2norm keeps its mixed-orders raise — open question raised by espg (here); answered on the thread with concrete options. mort2norm returns a single scalar (norm, parent, order) by contract, so mixed input is structurally unanswerable there without a return-type change (unlike the geo kernels, which return per-element arrays). The mixed use-case is already served per-element by mort2geo + orders_of. Awaiting espg's pick between (a) keep the raise + point at mort2geo/orders_of [recommended], (b) a new per-element mort2norm_each, or (c) a flag — so not implemented here.
2. orders_of/is_point do not validate words (pure bit arithmetic; the empty sentinel 0 decodes as order 0). Docstrings say so and point at validate_morton. Validating would cost a kernel round-trip and diverge from the plan's "pure numpy bit ops".
3. Point words through mort2bbox/mort2polygon now yield their containing order-29 cell geometry (Phase 5, per espg's review). A point and the order-29 area word at the same location decode to the identical cell, so mort2bbox(point) == mort2bbox(area29) and likewise for mort2polygon; a group of points scatters element-by-element to each point's containing cell. This replaces the earlier raise — the containing box/polygon of a point is well defined, as espg noted.
4. Pre-existing ruff findings on main (e.g. F841 in tools.py::_normalize_antimeridian_polygon) are left untouched; the new/changed files are clean under the CI flags (--select=E,F,W,I --ignore=E501).

🤖 Generated with Claude Code

https://claude.ai/code/session_01N32Mx4rWhbTK8xXm7Mt2ax

[espg]

Point words through mort2bbox/mort2polygon previously returned the order-29 cell bbox/ring (an implicit area claim). The plan flips this to a raise; no existing test exercised that path, so nothing pinned the old behavior, but it is technically a behavior change for all-point (uniform order-29) input — flagging since the acceptance bar was "no behavior change for uniform inputs".

why? i understand that a single order 29 entry makes little sense... but it still makes some sense (what box / polygon contains that point)? furthermore, a group of points fed into mort2bbox/mort2polygon does cover and area and is well defined.

[codspeed-hq]

Merging this PR will not alter performance

✅ 67 untouched benchmarks
⏩ 1 skipped benchmark¹

---

_{Comparing claude/116-mixed-order (20cd212) with main (4b57a1f)}

Footnotes

1. 1 benchmark was skipped, so the baseline result was used instead. If it was deleted from the codebase, click here and archive it to remove it from the performance reports. ↩

[espg]

mort2norm keeps its mixed-orders raise

kinda feel like this maybe should be a flag? Do we have a function handles the mix use case-- and do we need one?

[espg]

🤖 from Claude (review)

Blocking (CI red — stale base, not a defect in the #116 code). test (3.10/3.11/3.12) fail on test_spec_page.py::TestDecimalParseTieBreak::test_order29_string_parses_to_area_word → ValueError: invalid literal for int() with base 10: 'p'. This PR does not touch decimal parsing, test_spec_page.py, or any rust — the diff is confined to tools.py (orders_of/is_point/dispatch/raise), __init__.py, and the two test files — so the failure is not caused by the mixed-order work.

Root cause: the branch is based on 8cf4c3f, but origin/main has advanced to 4b57a1f with 5 commits from issue #120 (8b0efc1 phase 1 of issue #120 + folds, incl. 27930f8 delegate point suffix to kernel) that implement the decimal p-suffix parse in morton_index.py + src_rust/. The failing test needs that parser; the branch base predates it. It passes locally only because the built rust extension in the working tree is newer than the branch source.

Evidence: git log HEAD..origin/main = those 5 #120 commits (touch morton_index.py, decimal_morton.rs, lib.rs, prefix_trie.rs — disjoint from this PR, so no rebase conflict expected); origin/main @ 4b57a1f CI is green.

Fix at fold time: rebase (or merge) onto current origin/main before marking ready — CI should go green, since the #116 changes themselves pass (23/23 in test_mixed_order.py, and the full suite is green on main).

[espg]

🤖 from Claude (review)

Downstream coordination note (non-blocking) — refines the "downstreams verified" claim in Q under the infer_order_from_morton raise. Two moczarr sites call infer_order_from_morton on a possibly-mixed array as the first step of their own mixed-order guard, relying on the old return-a-value behavior and then using a clip2order round-trip to raise their own bespoke message:

• moczarr/coverage.py::aoi_mask (cell_order = int(infer_order_from_morton(cells)) then if (clip2order(cell_order, cells) != cells).any(): raise ValueError("aoi_mask requires single-order cells…"))
• moczarr/join.py (level = int(infer_order_from_morton(words)) then the mixed-order raise)

With the new mixed→raise, that first line now raises mortie's message before the moczarr-specific guard message is reached. End-state is preserved (mixed input → ValueError either way), so there's no correctness regression here — but it's not quite "call only on single-order inputs": these sites feed it the guard input itself. When moczarr#8 adopts, those guards become redundant/dead and any moczarr tests asserting the specific message will need re-pointing. Flagging so the moczarr#8 sequencing accounts for it; nothing to change in this PR.

(Spot-checked the other callsites — dggs.py:263 uses var.data[:1] single-element; coverage.py:198/join.py:119 iterate single words; zagg only calls mort2polygon on shard-key/cell-id area words, never points — so the point→bbox/polygon raise has no blast radius in zagg/moczarr, and clip2order(24, <point>) was verified to produce a working order-24 area word, so the error-message guidance is actionable.)

[espg]

🤖 from Claude

Folded the two review findings.

Finding 1 (merge stale base) — done, but the red test is NOT fixed by the merge; root cause differs from the review's diagnosis. Merged origin/main (4b57a1f) into the branch with git merge --no-edit (no rebase, no force). Clean merge, no conflicts — the 5 #120 commits (morton_index.py, decimal_morton.rs, lib.rs, prefix_trie.rs, plus test_point_suffix.py) are disjoint from the #116 diff. Merge commit: d7fe5d2.

However, the merge does not turn test_spec_page.py::TestDecimalParseTieBreak::test_order29_string_parses_to_area_word green, because that failure is pre-existing on main itself, not a stale-base artifact:

• main @ 4b57a1f Tests workflow is red on this exact test (run 29787103143, both 3.10 and 3.12): ValueError: invalid literal for int() with base 10: 'p', identical to the branch failure. So the review's premise "origin/main @ 4b57a1f CI is green" does not hold.
• Cause: decimal repr kind suffix: emit p for point ids, accept on parse (spec §2/§4) #120's fold review: delegate point suffix to kernel made decimal_repr() append a p kind-suffix for point words (a point at 45,45 renders 141114111111111444444141444141p). The test at test_spec_page.py:87 still does int(dec[-1]), which now hits the p. The test/decimal_repr drift belongs to the v1.0 docs: specification & conventions page (bit-layout + resolution/Δ table + frozen-convention statement) #62/decimal repr kind suffix: emit p for point ids, accept on parse (spec §2/§4) #120 decimal-parse work; this PR (mort2geo (and friends) reject mixed-order input — vectorized mixed-order support #116) does not touch decimal parsing or test_spec_page.py (diff is confined to tools.py, __init__.py, test_mixed_order.py, test_tools.py).

Per repo convention I did not "fix" a pre-existing main failure that's unrelated to this PR's change — flagging instead. The #116 work itself is green: full local suite = 1 failed, 706 passed, 11 skipped where the single failure is this inherited spec-page test; test_mixed_order.py is 23/23 (100%). Lint mirror (ruff --select=E,F,W,I --ignore=E501) reports 10 findings, but they are byte-identical to origin/main (only tools.py's on_antimeridian line shifts 520→657 from #116 adding code above it) — all pre-existing, none introduced here.

Question for @espg: the spec-page tie-break test needs re-pinning for the p-suffix decimal_repr from #120 (update test_spec_page.py:87 to strip/handle the kind-suffix). That's a main-level fix owned by #62/#120, outside #116's scope — should it land as a separate small-fix on main, or would you prefer I pick it up here despite the scope crossover? Until then CI on this PR stays red on that one inherited test through no fault of the #116 diff.

Finding 2 (moczarr infer_order_from_morton downstream note) — acknowledged, recorded. No code change here (correct — end-state ValueError is preserved; the new raise merely pre-empts moczarr's bespoke message). Recorded on espg/moczarr#8's adoption inventory so the sequencing drops the now-redundant aoi_mask/join single-order guards and re-pins their error-message expectations to mortie's wording.

[espg]

🤖 from Claude

Two review points addressed.

On mort2bbox/mort2polygon point handling (your note) — implemented. You're right on both counts: a single point has a well-defined containing box/polygon, and a group of points covers a well-defined area. A point word decodes to the identical HEALPix cell as the order-29 area word at the same location (verified — same nested id, depth 29), so the fix is to stop raising and let points flow through the order-29 group: each point now yields the bbox/ring of its containing order-29 cell. mort2bbox(point) == mort2bbox(area29) and likewise for polygons; mixed point+area arrays scatter element-by-element. Landed as phase 5 (commit 20cd212); the raise-assertion tests are flipped to containing-cell oracles. mort2geo's point handling is unchanged (a point's location is exactly what it returns).

On mort2norm — should it be a flag? do we have / need a mixed-case function? (your note) — this one is a design decision I'd rather you make, because it turns on a return-type change to a frozen-surface function. The specifics:

• Why mort2norm can't just "handle the mix" the way the geo kernels do. mort2norm returns a single scalar (normed, parent, order) — one order for the whole call, by contract. The geo kernels return per-element arrays (lat[i], bbox[i], ring[i]), so group-by-order dispatch scatters cleanly there. mort2norm has nowhere to put per-element orders without changing its return shape, so a mixed array is structurally unanswerable element-wise as the function stands.
• Do we already have a function for the mixed case? Yes, for the parts that make sense per-element: mort2geo(words) now dispatches and returns per-element lat/lon for a mixed array, and orders_of(words) returns the per-element orders. Between them you can recover per-element location + order without mort2norm at all.
• Would a flag solve it? No — a flag can't change the scalar return shape, so mort2norm(mixed, flag=...) still has no place to return N distinct orders. A flag could only choose between raising and, say, returning the first element's order (the old silent-footgun behavior), which is what phase 2 deliberately removed.

Concrete options (pick one):

• (a) Keep the raise, point users at mort2geo + orders_of for the mixed case (my recommendation — no return-type change to a frozen-surface function; the mixed use-case is already served).
• (b) Add a new per-element variant e.g. mort2norm_each(words) -> (normed[], parent[], order[]) returning arrays, leaving scalar mort2norm frozen. Clean, but a new public surface to maintain — only worth it if there's a real consumer that needs normed/parent per-element on mixed arrays (I don't see one downstream yet).
• (c) Add a flag — doesn't actually resolve the mixed case (can't change the scalar shape), so I'd not go this way.

Which would you like? I've left mort2norm untouched pending your call.

---

Generated by Claude Code

[espg]

🤖 from Claude (review)

Phase-5 review (commit 20cd212) — removes the point-word ValueError from mort2bbox/mort2polygon so points yield their containing order-29 cell geometry.

Correctness: sound, no defect found.

• orders_of maps suffix 48..=63 → order 29, so points group with order-29 area words and decode through the same mort2norm→nested-cell path. The oracle mort2bbox(_point()) == mort2bbox(_area(29)) is valid and non-circular: both derive from the same lat/lon via geo2mort, and the equality genuinely checks that the point's suffix bits don't perturb the decoded cell vs the area word. Depth-29 agreement is independently pinned by test_matches_kernel_depth_decode.
• Edge cases raised in review are clean: the empty sentinel 0 decodes as order 0 with is_point(0)==False, so the removed raise never guarded it — no new silent-wrong path. Empty-array input never entered the old words.size and np.any(is_point(...)) guard either, so its behavior is unchanged. Southern base cells (bit 63) are exercised by the -45,-120 fixture.
• is_point is no longer used inside tools.py but remains public (exported in __init__.py, directly tested) — not dead code.
• Docstrings are correctly updated; the spec §1/§4 "no area claim" language for points is consistent with this change (the point carries no area claim; the returned box is its containing cell's), so nothing stale there.

One diff-scoped test-strengthening and one cosmetic nit inline. Nothing blocking.

---

Generated by Claude Code

[espg]

🤖 from Claude (review)

Point-scatter is only exercised with contiguous group blocks: np.concatenate([_area(6), _point()]) puts the two order-6 words at positions [0,1] and the two order-29 points at [2,3]. Because the order-6 and order-29 groups are already contiguous ascending-order blocks, a buggy scatter that just concatenates per-order group results (ignoring original positions) would still pass this test — so it doesn't actually pin scatter-back-to-position for the point path.

The pure-area oracle test_mort2bbox_matches_uniform gets this right by using shuffled/interleaved _mixed_area(), but that helper excludes points, so the point path never gets the same positional rigor. Suggest interleaving points among area words (e.g. shuffle, or [_point()[:1], _area(6), _point()[1:], _area(19)]) and asserting each element equals its containing-cell/uniform bbox at its input index. Low severity — the underlying scatter mechanism is shared and covered for area words — but this is the one test that's supposed to lock the point behavior.

---

Generated by Claude Code

[espg]

🤖 from Claude (review)

Nit (non-blocking): the class name TestPointPostures is stale after this phase. It made sense when the tests asserted raise "postures"; the class now asserts containing-cell geometry. Consider renaming (e.g. TestPointContainingCell) so the intent reads from the name. Cosmetic only.

---

Generated by Claude Code

[espg]

🤖 from Claude (review)

Adversarial review of the newest phase (20cd212 — point words yield containing-cell geometry). Verdict: clean, no defects found.

What I checked and confirmed:

• Demotion correctness — point words (suffix 48..=63) are classified order 29 by orders_of and flow through the order-29 group; mort2bbox/mort2polygon/mort2geo on a point return exactly the geometry of the order-29 AREA word at the same location. Verified across 505 random + edge locations (poles, ±180 antimeridian): 0 bbox / 0 polygon mismatches. No off-by-one, no wrong/empty geometry.
• Scatter through dispatch — points mixed with area words (including a point and a genuine order-29 area word coexisting in the same order-29 group, an untested combination) each land at the correct input position with the correct per-element geometry.
• Test replacement — the old point-raise assertions in TestPointPostures are properly replaced with positive oracles (point geometry == same-location order-29 area geometry), scalar and array postures both covered. All 23 tests in test_mixed_order.py pass locally.
• Consistency — mort2geo(point) lands on the same order-29 cell center as the area word, so geo/bbox/polygon treat a point as its containing cell uniformly.
• Docstrings — mort2bbox/mort2polygon docstrings and Raises sections updated to match the new (non-raising) contract; params no longer say "area words only". Terse, matches surrounding style.

One non-blocking nit inline (oracle location breadth). One observation for awareness: this deliberately swaps a loud raise for a silently-plausible cell box when a caller passes a point word by mistake — sanctioned by the request on this PR, and is_point remains exported for callers that need to distinguish. is_point is no longer referenced inside tools.py after the guard removal, but it stays a legitimate public introspection helper (in __all__, covered by TestIsPoint) — not dead code.

---

Generated by Claude Code

[espg]

🤖 from Claude (review)

Nit (non-blocking): the point-demotion oracle mort2bbox(_point()) == mort2bbox(_area(29)) only exercises the two module-level locations _LATS/_LONS (45,45) and (-45,-120). The load-bearing invariant here is that the Rust kernel decodes a point word (suffix 48..=63) to the same nested cell as the order-29 area word at that location — worth hardening with a few edge locations (poles, ±180 antimeridian, a base-cell 7-11 southern column already partly covered).

I independently swept 505 random + edge locations (poles and antimeridian included) and found 0 bbox/0 polygon mismatches, so the invariant does generalize — this is a test-coverage hardening suggestion, not a defect. mort2geo(point) likewise lands on the same order-29 cell center, so geo/bbox/polygon treat a point as its containing cell consistently.

---

Generated by Claude Code