numpy-level point-kind geo2mort: geo2mort(..., points=True)

[espg]

Closes #96.

What this does

Adds a vectorized, numpy-in/numpy-out point-kind encoder on the geo2mort surface, so zagg's hot path (HealpixGrid.assign, per-photon for ATL03) gets a public vectorized point encode and drops the MortonIndexArray → ndarray unwrap entirely (the ~12x penalty measured in the issue).

lat/lon inputs are treated as points by default (indeterminate resolution, encoded at max precision), so:

geo2mort(lats, lons)                    # -> order-29 Kind::Point words (bare call)
geo2mort(lats, lons, order=12)          # -> order-12 Kind::Area cells (explicit order => area)
geo2mort(lats, lons, points=True)       # -> order-29 Kind::Point words (explicit)
geo2mort(lats, lons, order=12, points=False)  # -> order-12 area (same as `order=12`)

Result is always a plain contiguous uint64 ndarray — no ExtensionArray wrapper. Point encoding is order-29-only, so points=True with an explicit order != 29 raises ValueError (matching MortonIndexArray.from_latlon). Non-finite lat/lon encode to the reserved empty word 0 (base cell 0 is the null sentinel) on both routes.

Recorded design decisions (from review thread)

• points defaults to point. Per this comment ("lat/lon inputs are generally expected to be of type point ... flip it"), a bare geo2mort(lat, lon) returns order-29 point words. An explicit order implies an area cell (points inferred False), so geo2mort(lat, lon, order=18) does not raise. Resolved via a tri-state order=None/points=None: points = order is None, then order = order or 29 — using an identity check so order=0 stays 0.
• Non-finite → reserved 0. Per this comment ("why don't we hash to the reserved 0? we already have base cell '0' reserved for this"), non-finite lat/lon map to 0 on both paths. The guard lives in a new geo2mort_word helper scoped to the public geo2mort surface, so linestring's internal geo2mort_scalar hashing is unchanged.
• Default order 18→29 is breaking (bare call now returns order-29 words, not order-18); flagged in CHANGELOG.md. All in-tree callers pass an explicit order.

Phases

• Phase 1 — Rust fused point path + binding. geo2mort_point_scalar; rust_geo2mort gains points flag; Rust unit tests (point ≠ area at 29 but same nested cell; fused == compose).
• Phase 2 — Python surface + tests. geo2mort points/order plumbing + validation + uint64 coercion; the four issue-specified tests + from_latlon equivalence.
• Phase 3 — fold self-review 1. Real non-finite assertion, dead-noqa removal, broadcast test, MAX_ORDER constant.
• Phase 4 — maintainer directives. Non-finite → reserved 0 (geo2mort_word, scoped to the public surface, + Rust test); flip the points default to point-by-default via the tri-state resolution; CHANGELOG line; updated/added tests (reserved-0, bare-call-defaults-to-point).
• Phase 5 — fold self-review 2. Pin order=0 resolves to an order-0 word (guards against a future truthiness regression); clarify the binding's low-level-default doc comment.

How it was tested

• cargo test (13 geo2mort tests), cargo fmt --check clean, cargo clippy (no new warnings — only the pre-existing useless_conversion false-positives, none in this diff).
• flake8 mortie --select=E9,F63,F7,F82 clean.
• pytest -v: 548 passed, 11 skipped. test_geo2mort_points.py covers the four issue tests, from_latlon/kernel equivalence, broadcast, reserved-0 non-finite handling, bare-call-defaults-to-point, and pins the explicit-order (incl. order=0) area path.
• CI on the PR: codecov 100% on modified lines, CodSpeed reports no perf change.

Side note: the scalar path previously returned int64 for small (northern) words and uint64 for large (bit-63-set southern) words — a latent hemisphere-dependent dtype inconsistency. geo2mort now always returns a contiguous uint64 ndarray.

Questions for review

None outstanding — both earlier questions were answered on the thread and implemented in phase 4. Ready for your review.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.73%. Comparing base (476f1a4) to head (fbe3477).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #100      +/-   ##
==========================================
+ Coverage   93.70%   93.73%   +0.03%     
==========================================
  Files           9        9              
  Lines        1223     1230       +7     
==========================================
+ Hits         1146     1153       +7     
  Misses         77       77              

Flag	Coverage Δ
unittests	93.73% <100.00%> (+0.03%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
mortie/tools.py	97.14% <100.00%> (+0.08%)	⬆️

---

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update adca957...fbe3477. Read the comment docs.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[codspeed-hq]

Merging this PR will not alter performance

✅ 67 untouched benchmarks
⏩ 1 skipped benchmark¹

---

_{Comparing claude/96-geo2mort-points (fbe3477) with main (4288bd2)}

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]

🤖 from Claude (review)

Adversarial pass on the point-encoder. I built the extension (maturin develop --release) and exercised the behavior directly. The core encode is correct against every acceptance criterion in issue #96: points=True words differ from area at order 29 but decode to the same nested cell; clip2order coarsening is bit-identical at orders 0–28 (clip at 29 preserves kind, so the test's range(0, 29) cap is intentional and correct — including 29 would fail, which is right); a lone point survives common_ancestor unchanged; output is uint64/C-contiguous/same-shape; the uint64 coercion is correct including base cells 7–11 with bit 63 set; from_latlon(points=True) equivalence holds; and the ValueError message matches from_latlon verbatim. The fused Rust path and default-order flip are both what the maintainer signed off on. No correctness bug found.

The findings are all test-quality / robustness, plus one thing to surface:

1. test_nonfinite_handling_agrees_between_paths is close to vacuous (biggest one). It compares only exception type, but empirically neither path raises on NaN/±inf — both silently emit a valid-looking non-zero word — so the assert passes without checking anything. It should assert decoded-nested equality (which does hold), and use except BaseException (a PyO3 panic surfaces as PanicException, a BaseException). Related: this quietly means a NaN fill value on the per-photon hot path encodes to a plausible word rather than the reserved 0 or a loud failure the issue mentioned — pre-existing area-path behavior, but worth a maintainer eye.
2. Dead # noqa: BLE001 — no ruff/bugbear is configured (§7), so it silences nothing and misleads.
3. Untested broadcast branch for points=True (scalar + array), one of the three sub-paths in rust_geo2mort.
4. Default order 18→29 is a silent public behavior change — authorized, but flag it as breaking in the PR body/changelog; and the Python side hardcodes 29 where the siblings use MAX_ORDER.

None of these block the encode; (1) is the one I'd fix before merge.

---

Generated by Claude Code

[espg]

🤖 from Claude (review)

This is the acceptance criterion the issue cares most about ("non-finite lat/lon should fail loudly or produce the sentinel 0 — the two paths must agree"), but as written the test is close to vacuous. I built the extension and ran the actual behavior:

nonfinite nan  points=False -> value 5764607523034234909
nonfinite nan  points=True  -> value 5764607523034234928
nonfinite inf  points=False -> value 1152921504606847005
nonfinite -inf points=False -> value 10376293541461622813

So on both paths a non-finite input neither raises nor returns 0 — the healpix hash silently maps NaN/±inf to a real cell (NaN → nested 1152921504606846976) and packs a valid-looking non-zero word. Because nothing raises, area_err == point_err == None and the assert passes without checking anything meaningful.

Two concrete problems:

1. It never verifies value agreement. The docstring promises the paths "resolve identically", but the test only compares whether-it-raised. I confirmed the area and point words for NaN decode to the same nested cell (1152921504606846976 both) — that's the property worth pinning. Assert decoded-nested equality (via rust_mort2nested), not just exception type.
2. except Exception is the wrong net for the Rust boundary. If the hash ever panicked instead of returning a word, PyO3 surfaces it as pyo3_runtime.PanicException, which derives from BaseException, not Exception — it would escape this handler and error the test rather than being compared. except BaseException (or asserting the value) is the robust form.

Separately, worth surfacing for a maintainer decision (not something to silently absorb): the issue said "0 remains the only reserved word, never a valid encode result," yet a NaN photon — a realistic ATL03 fill value on the per-photon hot path this PR targets — silently encodes to a plausible non-zero word on both the area and point paths. This is pre-existing area-path behavior and matches the "mirror the area path" directive, so it isn't a regression, but the current test actively obscures it.

---

Generated by Claude Code

[espg]

🤖 from Claude (review)

# noqa: BLE001 is dead here. Per CLAUDE.md §7 the only linters configured are flake8 mortie --select=E9,F63,F7,F82 (blocking) and a non-blocking --max-line-length=88 style pass — there is no ruff/flake8-bugbear, so BLE001 (a ruff code) is never emitted. The suppression silences nothing and reads as if a bugbear check exists. CLAUDE.md §4 also discourages adding # noqa to quiet warnings rather than fixing the cause. Drop the # noqa (and if you take the except BaseException suggestion on the assert below, the broad-except concern the code was pre-empting goes away too).

---

Generated by Claude Code

[espg]

🤖 from Claude (review)

Coverage gap on the point path: the points=True branch in rust_geo2mort has three sub-branches — both-scalar, equal-length array, and broadcast (scalar-lat + array-lon, or vice-versa, via the lat_bcast/lon_bcast flags in lib.rs). This suite exercises the equal-length array path (_point()) and the both-scalar path (this line), but never the broadcast branch with points=True. A one-liner like geo2mort(45.0, _LONS, points=True) asserting shape == _LONS.shape and dtype uint64 would close it and guard the branch that geo2mort_point_scalar is threaded through. (Empty-input geo2mort([], [], points=True) also works — returns (0,) uint64 — and is worth a line if you want the boundary pinned.)

---

Generated by Claude Code

[espg]

🤖 from Claude (review)

The order=18 → order=29 default flip is an authorized change (the maintainer asked for it directly on the issue), so this is not an objection — two follow-through notes:

1. It's a silent public behavior change for the bare geo2mort(lat, lon) call: external callers relying on the old order-18 default now get a different (order-29) word. In-tree this is safe — I grepped every caller and all pass an explicit order= except the docstring examples — but it should be called out explicitly in the PR description / changelog as a breaking behavior change, not just as "issue numpy-level point-kind geo2mort: geo2mort(lat, lon, order=29, points=True) #96".
2. Minor consistency: this function hardcodes the literal 29 twice (the order=None resolution on line 140 and the guard on line 141), whereas the sibling MortonIndexArray.from_latlon and the Rust guard both compare against the MAX_ORDER constant. Not load-bearing (29 is the u64-packing ceiling and won't move), but a named constant would keep the two surfaces from drifting.

---

Generated by Claude Code

[espg]

Non-finite input semantics. The issue asked the point path to match the area path on non-finite lat/lon. Today neither raises: a NaN/±inf lat/lon hashes to a definite order-29 nested cell (e.g. NaN → 1152921504606846976) and encodes to a plausible-looking word rather than the reserved 0 or a loud error. The point path matches the area path exactly (the shared hash step), so this PR is faithful to the directive — but on zagg's per-photon hot path a NaN fill silently becoming a valid word may be undesirable. Do you want the area path (and therefore both) changed to reject non-finite input? That's a behavior change to the existing area encode, so I've left it out of this PR's scope and raise it here rather than acting on it.

why don't we has to the reserved 0 ? we already have base cell '0' reserved for this...

[espg]

Default-order change (breaking). I read "update the default to order 29 / point" as: default order → 29, points stays an opt-in flag defaulting False (so a bare geo2mort(lat, lon) returns order-29 area words, and points=True opts into point words). This honors the issue body's "points=False (default) → area-kind" while making 29 the new default order. The backward-compat consequence, flagged explicitly: a bare no-order geo2mort(lat, lon) now returns order-29 words instead of order-18. All in-tree callers pass an explicit order, so nothing internal breaks, but it's a public change worth a changelog line.

this seems fine.

All in-tree callers pass an explicit order, so nothing internal breaks, but it's a public change worth a changelog line.

agreed.

If you instead meant the default should emit point words, say so and I'll flip the points default.

i did mean that. lan / lon inputs are generally expected to be of type point (i.e., indeterminate resolution, encoded at max precision). so... flip it

[espg]

🤖 from Claude (review)

Fresh-eyes review of phase 4 (295113f) — the non-finite→reserved-0 and default-flip directives. Both directives are faithfully implemented; the change is correct. Findings are all non-blocking nits.

Verified correct

• Tri-state truth table — enumerated all 8 cases (bare / order=18 / order=29 / points=True / points=True+order=18 / points=False / points=False+order=18 / order=0). All resolve as intended. Critically, the order=0 falsy trap is avoided: the code uses order is None (identity), not order or MAX_ORDER (truthiness), so order=0 stays 0 and infers points=False — an order-0 area cell, not a coerced order-29 word.
• Non-finite guard — correct for NaN/±inf in either lat or lon, on both routes, and correctly scoped: living in geo2mort_word (not geo2mort_scalar) leaves linestring's internal hashing unchanged (linestring.rs:126 still calls the scalar directly). Array path zeroes only the bad rows (tested), empty array still fine. 0 is genuinely unreachable by a real encode (prefix is base+1, so the empty sentinel never collides).
• CHANGELOG "Breaking" claim — "was order-18 area cells" is accurate against the last release (0.8.4 defaulted order=18); the intermediate order-29-area default was itself unreleased. The [Unreleased] block has a single, non-contradictory line.
• Point/area/broadcast/dtype/from_latlon-equivalence and the points=True, order=18 raise all remain covered.

Non-blocking nits (inline)

1. No test pins that a falsy order=0 yields an order-0 word — the backward-compat test compares order=0 to order=0, so a future order or MAX_ORDER regression would slip past. A one-line infer_order_from_morton(...) == 0 assert would lock it down. (Highest-value follow-up.)
2. rust_geo2mort's pyo3 signature default (order=29, points=false) and docstring now describe an internal-only default that diverges from the public geo2mort point default — harmless (Python always passes explicit args) but a drift footgun for direct-binding callers.

Nothing here blocks the phase.

---

Generated by Claude Code

[espg]

🤖 from Claude (review)

Tri-state resolution is correct, and I want to explicitly flag the order=0 falsy trap since it's the easiest place to have introduced a bug — you avoided it. Because this uses the identity test order is None (not a truthiness test like order or MAX_ORDER), a falsy-but-valid order=0 is handled right:

• geo2mort(lat, lon, order=0) → points = (0 is None) = False, order stays 0 → order-0 area cell. ✔
• geo2mort(lat, lon, order=0, points=True) → raises (0 != 29). ✔ (tested)

Enumerated the full truth table (bare / 18 / 29 / points=True / points=True,order=18 / points=False / points=False,order=18 / order=0) — all resolve as intended.

One residual gap: nothing in the suite actually pins that order=0 produces an order-0 word. test_area_default_backward_compatible_at_explicit_order iterates order=0 but only compares order=0 against order=0, points=False, so a hypothetical future order or MAX_ORDER regression (0 → 29) would keep both sides equal and slip past. A one-line assert infer_order_from_morton(int(geo2mort(_LATS, _LONS, order=0)[0])) == 0 would lock the falsy-order behavior down. Non-blocking.

---

Generated by Claude Code

[espg]

🤖 from Claude (review)

Good coverage of the flipped default here (bare == points=True, explicit order == area). The one thing not asserted anywhere is that an explicit falsy order=0 stays order 0 rather than being coerced to 29 — see my note on tools.py. Since order=0 is now the only falsy-but-valid order flowing through the order is None gate, it's worth an explicit pin (infer_order_from_morton(...) == 0) so the tri-state logic can't silently regress. Non-blocking.

---

Generated by Claude Code

[espg]

🤖 from Claude (review)

Routing both the scalar and array paths through geo2mort_word is a clean dedup, and scoping the non-finite→0 guard inside geo2mort_word (rather than geo2mort_scalar) correctly leaves linestring's internal hashing untouched — verified linestring.rs:126 still calls geo2mort_scalar directly. ✔

Minor drift note (non-blocking): this pyfunction still carries #[pyo3(signature = (lats, lons, order=29, points=false))] and the docstring says "order (default 29)". All of the new point/area/tri-state policy now lives in Python (tools.py always passes both args explicitly), so this Rust-level default is dead for the public surface — but a direct _rustie.rust_geo2mort(lat, lon) still yields an order-29 area word, i.e. the pre-flip behavior. Harmless today, but it's a documentation/behavior-drift footgun if anyone later calls the binding directly. Consider a one-line comment noting the binding defaults are internal-only and the public default lives in geo2mort.

---

Generated by Claude Code

[espg]

🤖 from Claude

Both landed in phase 4 (self-reviewed + folded in phase 5):

• Non-finite → reserved 0. Non-finite lat/lon now encode to 0 on both the area and point routes, via a new geo2mort_word helper. I scoped the guard to the public geo2mort surface rather than the shared geo2mort_scalar kernel, so linestring's internal hashing is unchanged (it can keep its own handling of bad coordinates). In an array, only the non-finite rows are zeroed.
• Point-by-default. A bare geo2mort(lat, lon) now returns order-29 Kind::Point words. To avoid making geo2mort(lat, lon, order=18) raise, an explicit order implies an area cell (points inferred False); points=True still forces the order-29 point path. Resolved with a tri-state (points = order is None, then order = order or 29), using an identity check so order=0 stays 0 — pinned by a test.

Truth table now: bare → point@29; order=N → area@N; points=True → point@29 (non-29 order raises); points=False → area (order defaults 29). CHANGELOG carries the breaking-default note. Green locally (548 pytest passed, cargo test/fmt/clippy clean) and CI is green (codecov 100% on the diff, CodSpeed no perf change). Ready for another look.

---

Generated by Claude Code