Fix the wrong normalized address in ExampleUsage.ipynb cell 15

[espg]

Closes #142

What this changes

examples/ExampleUsage.ipynb cell 15 fed a UNIQ value into arithmetic that expects a NESTED index, so every element of normed overshot by exactly 4**(order+1). Because norm2mort composes (parent << 2*order) | normed, that excess lands squarely in the base-cell field and the outcome is data-dependent.

The cell now does what one call already does, per the issue's suggested fix:

%%time

# geo2mort does the whole geographic -> morton encoding in one call
order = 9
mortons6 = mt.geo2mort(b4.Lat.values, b4.Lon.values, order=order)
print(len(np.unique(mortons6).ravel()))
np.unique(mortons6).ravel()

Cell 17's leading comment (# Alternative function that's more compact from above) was orphaned by the fix — cell 15 is that function now — so it was retargeted at what cell 17 actually demonstrates (clipping an order-18 encoding back to order 6). No code change in cell 17.

Phases

• Phase 1 (72a9ade) — fix cell 15's code; clear that cell's unreproducible committed outputs; retarget cell 17's orphaned comment.
• Phase 2 (c4d62cb) — regression test pinning the identity the notebook now relies on.
• Phase 3 (eb25647) — fold the adversarial self-review's diff-scoped findings.

Empirical reproduction

Verified against a 5° global lat/lon sweep (2592 points, all 12 base cells present) at order = 9, on this branch's build. The issue's claims hold exactly:

offset unique: [1048576]   4**(order+1) = 1048576
bug normed in [0, 4**order)? False
fix normed in [0, 4**order)? True

base   points  outcome    detail
0         252  WRONG      e.g. 5767615786847830025 != 1155929768420442121
1         252  WRONG      e.g. 6920537291454677001 != 2308851273027289097
2         252  WRONG      e.g. 8073458796061523977 != 3461772777634136073
3         252  WRONG      e.g. 9226380300668370953 != 4614694282240983049
4         144  MATCHES
5         144  MATCHES
6         144  MATCHES
7         144  MATCHES
8         252  RAISES     ValueError: nested index 3146376 too large for depth 9 (base 12 > 11)
9         252  RAISES     ValueError: nested index 3408520 too large for depth 9 (base 13 > 11)
10        252  RAISES     ValueError: nested index 3670664 too large for depth 9 (base 14 > 11)
11        252  RAISES     ValueError: nested index 3932808 too large for depth 9 (base 15 > 11)

corrected chain == geo2mort over all 12 base cells: True

Equivalence of the collapse is confirmed, not assumed: norm2mort(uniq - 4*4**order - parents*4**order, parents, order) equals geo2mort(lats, lons, order=order) elementwise over all 2592 points. Both routes share the same latitude="authalic" default, so the collapse changes nothing but the arithmetic.

One finding beyond the issue

The issue's table is right in general, but for the notebook's own data the cell does not silently mislead — it simply raises. Basin 4 of the Antarctic drainage systems lies entirely in base cell 11:

b4 base cells: [11]
b4 notebook chain RAISES: ValueError nested index 3950442 too large for depth 9 (base 15 > 11)
b4 truth first 3    : [13915463141598167049 13915463141598167049 13915463141598167049]

So cell 15 as it stood on main could not produce its own committed outputs under any build — which is what motivated clearing them (see below).

The regression test

mortie/tests/test_notebook_normalized_address.py, four tests, all driven off a 5° sweep that provably reaches every base cell:

• test_sweep_reaches_every_base_cell — guards the other three. A sweep missing base cells 0-3 or 8-11 is exactly the spot check that let this bug survive.
• test_geo2mort_matches_the_normalized_chain — the identity the fixed cell relies on: the corrected UNIQ chain equals geo2mort across all twelve base cells, including 0-3 which used to be silently wrong.
• test_unnormalized_uniq_leaves_norm2mort_domain — pins the bug's signature: the un-normalized address is outside norm2mort's documented 0 <= normed < 4**order domain, and overshoots mort2norm of the correct word by exactly 4**(order+1).
• test_notebook_data_lands_in_a_raising_base_cell — pins the finding above against the in-tree basins fixture.

How it was tested

• .venv/bin/python -m pytest -q — 1780 passed, 16 skipped (baseline on main is 1776/16; the four added are this PR's).
• flake8 mortie --select=E9,F63,F7,F82 — clean.
• ruff check mortie/tests/test_notebook_normalized_address.py — clean.
• No Rust changed, so no cargo test / cargo clippy run; the extension was built with maturin only to run the suite.
• Post-ruling re-verification (2026-08-24, after merging main): pytest -q — 1783 passed, 24 skipped (one flake, test_wkb_batch_memory.py::test_the_chunk_copy_is_capped_in_bytes_not_only_in_blob_count, fails under full-suite load but passes in isolation — RSS-measurement noise, pre-existing and unrelated); jupyter nbconvert --execute ran the notebook end to end with zero cell errors.

Questions for review — all four RESOLVED (espg rulings, 2026-08-24)

(1) RESOLVED — keep the authalic default; rewrite the prose. Cell 15 stays on latitude="authalic" (373 unique cells at order 9; the order-18 encoding clipped to order 6 gives 40). Cell 16 no longer claims "still 380": it now teaches the difference — healpy's ang2pix above treats latitudes as spherical (380 cells); mortie's default maps geodetic latitudes to the authalic sphere so cells are equal-area on the WGS84 ellipsoid by construction (#186/#188); passing latitude="geodetic-spherical" reproduces healpy's 380. (26deb75)

(2) RESOLVED — regenerate the whole notebook. Full top-to-bottom re-execution against current main (merged in via 490c420), so every committed output is current: packed-uint64 morton words everywhere (cell 17's retired decimal form is gone), both gnomview plots re-rendered, cell 15 prints 373 and cell 17's clip prints 40. (35ba614)

(3) RESOLVED — option (a). Cell 3 now loads the in-tree fixture (../mortie/tests/Ant_Grounded_DrainageSystem_Polygons.txt, relative to examples/), so the notebook is launch-and-run — Binder-runnability was the explicit goal. The NASA link in cell 2 stays as data provenance. (8f4f393)

(4) RESOLVED — folded into (3)'s commit. delim_whitespace=True (removed in pandas 3.0) is replaced by sep=r"\s+"; without it the re-execution in (2) is impossible. (8f4f393)

Refs #68, #130, #136, #180, #186.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.67%. Comparing base (b2ad13a) to head (35ba614).

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #209   +/-   ##
=======================================
  Coverage   96.67%   96.67%           
=======================================
  Files          20       20           
  Lines        2283     2283           
=======================================
  Hits         2207     2207           
  Misses         76       76           

Flag	Coverage Δ
unittests	96.67% <ø> (ø)

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

---

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 b2ad13a...35ba614. Read the comment docs.

🚀 New features to boost your workflow:

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

[espg]

🤖 from Claude (review)

Adversarial review of claude/142-notebook-normalized-address (2 commits, 2 files), against CLAUDE.md and the acceptance criteria in #142.

The core diagnosis is correct and I verified it independently. Basin 4 lands entirely in base cell 11, and the old cell's normed overshoots by 4**(order+1), so norm2mort raises ValueError: nested index ... too large for depth from src_rust/src/decimal_morton.rs, surfaced as PyValueError at the binding in src_rust/src/lib.rs. The collapse to mt.geo2mort(...) is the right fix and is genuinely equivalent.

Findings (2 medium on the notebook, 2 low on the test module — see the inline comments):

1. Cell 15 now yields 373 cells while the next markdown cell claims 380; the gap is the authalic default (#186). MEDIUM.
2. Cell 17's comment is edited but its stale, unreproducible output is left in place. MEDIUM.
3. The overshoot assertion is an algebraic tautology — it exercises no library code. LOW.
4. The base-cells-0-3 assertion pins undefined behavior and would break under the obvious normed-range hardening of norm2mort. LOW/MEDIUM.

Conventions: no new dependencies, no workflow changes, no weakened or skipped tests, commit messages are title-only in the repo's style, branch is correctly keyed to the issue. Coverage of the fix is real — the all-12-base-cells sweep is genuinely new, though mortie/tests/test_convert.py already covers part of the uniq → nest → normed decomposition.

Minor, not blocking: the new module np.loadtxts the 1.24 M-row basins fixture for a single assertion. Neighbouring tests do the same and are likewise unmarked, so this matches the surrounding code rather than departing from it.

---

Generated by Claude Code

[espg]

🤖 from Claude (review)

overshoot is an algebraic tautology and exercises no library code.

(uniq - p*4**o) - (uniq - 4*4**o - p*4**o) cancels to the constant 4*4**o for any inputs whatsoever — the uniq and parents terms both drop out. So this assertion passes even if geo2uniq and unique2parent returned garbage. It reads as a pin on the bug's signature but pins nothing.

To make it real, assert against something derived from the library — e.g. that the un-normalized value is out of norm2mort's documented domain (normed >= 4**ORDER, against the 0 <= normed < 4**order contract in mortie/convert.py), which is the property that actually caused #142.

---

Generated by Claude Code

[espg]

🤖 from Claude

Fixed in eb25647. You were right — the two expressions cancelled algebraically, so nothing in the library was being exercised.

The assertion is now grounded in library output on both halves:

unnormalized = uniq - parents * 4**ORDER

assert (unnormalized >= 4**ORDER).all()
np.testing.assert_array_equal(
    unnormalized - mt.mort2norm(mt.geo2mort(lats, lons, order=ORDER))[0],
    4 ** (ORDER + 1),
)

The first line is the domain violation you suggested (0 <= normed < 4**order, per mortie/convert.py). The second measures the overshoot against mort2norm of the correct word — an independent decode path — so geo2uniq, unique2parent, geo2mort and mort2norm all have to agree for it to pass. The test is renamed test_unnormalized_uniq_leaves_norm2mort_domain to match what it now checks.

---

Generated by Claude Code

[espg]

🤖 from Claude (review)

This locks in undefined behavior and would block the natural hardening fix.

norm2mort's contract is 0 <= normed < 4**order (mortie/convert.py), and the Rust kernel only guards the decoded base (base_u64 <= 11), not normed's magnitude. These lines deliberately feed normed >= 4**order for base cells 0-3 and assert the resulting word merely differs from the truth.

That is fine as a description of today's behavior, but if anyone adds the obvious range check to norm2mort — the very fix that would have made #142 loud instead of silent — this test starts failing with a ValueError and looks like a regression. Express it as "raises-or-differs", or state in a comment that the assertion intentionally pins unvalidated input.

---

Generated by Claude Code

[espg]

🤖 from Claude

Fixed in eb25647, taking the "raises-or-differs" option.

# Base cells 0-3 are the silent ones: today norm2mort does not range-check
# ``normed``, so they return a wrong word rather than raising.  Accept a
# raise too, so adding that check to norm2mort reads as a fix, not a
# regression.
low = parents < 4
try:
    wrong = mt.norm2mort(unnormalized[low].ravel(), parents[low].ravel(), ORDER)
except ValueError:
    return
assert (wrong != mt.geo2mort(lats[low], lons[low], order=ORDER)).all()

The point that mattered is preserved — base cells 0-3 never come back with the right answer — while the test no longer turns the obvious hardening of norm2mort into a red build. The comment records why the branch is there, so a future reader does not "simplify" it back.

---

Generated by Claude Code

[espg]

🤖 from Claude (review)

Two findings on the notebook.

(1) MEDIUM — the rewritten cell 15 now produces 373 cells, but the very next markdown cell asserts 380.

Verified against the committed data: mt.geo2mort(b4.Lat.values, b4.Lon.values, order=9) yields 373 unique cells, while cell 12's healpy comparison yields 380, and the markdown immediately below cell 15 reads "Still 380 cells like in the other example". The gap is the authalic-latitude default (#186): latitude="geodetic-spherical" gives exactly 380. Because the outputs were cleared and execution_count set to null, a reader on Binder is the first to see the mismatch — and the notebook's whole "mortie reproduces the healpy cells, but multi-resolution" narrative breaks. Either pin the cell to geodetic-spherical (matching what healpy did in cells 9/12) or rewrite the 380 claim.

(2) MEDIUM — the diff edits cell 17's comment but leaves its stale, now-wrong committed output in place.

mt.clip2order(6, mt.geo2mort(b4.Lat.values, b4.Lon.values, 18)) yields 40 unique cells today (same authalic cause; healpy's order-6 count is 39), yet the committed stdout still says 39 and the committed text/plain array is the retired decimal encoding (array([-6121322, -6121321, ...])), not packed uint64 words. Given the PR's stated purpose is making these cells honest, touching cell 17 without refreshing its output leaves a second cell whose committed result cannot be reproduced.

Worth knowing: examples/ notebooks are not executed by CI — mortie/tests/test_example_notebooks.py explicitly pins only the Binder link and states "Deliberately not checked: that the notebook executes." So neither of the above will be caught by the suite; the new tests cover the API identity, not the notebook cell.

---

Generated by Claude Code

[espg]

🤖 from Claude

Both findings are real; neither is folded, and here is why each is left standing rather than silently decided.

(1) 373 vs the "Still 380 cells" claim. Confirmed, and it is already question (1) in the PR body with the same table. I am deliberately not picking between latitude="geodetic-spherical" and rewriting the claim: that decides whether this example is pinned to the legacy convention or to the current authalic default, which is a #186 design call, not a #142 bug fix. CLAUDE.md §2 says a design fork is a block, so the PR is going to waiting with this as the question.

Worth noting the mismatch is not introduced here — on main the cell raises for basin 4 (base cell 11), so 380 was already unreachable. This PR changes it from "unreachable" to "reachable but 373".

(2) Cell 17's stale output. Also confirmed — 40 today, committed 39, in the retired decimal encoding. I kept the comment edit and did not refresh the output, because the comment (# Alternative function that's more compact from above) was orphaned by this fix: cell 15 now is that compact function, so leaving the line would be actively misleading. Refreshing cell 17's output, by contrast, means re-executing it — which runs straight into (1), since the 39-vs-40 gap has the identical authalic cause. Doing that unilaterally would settle (1) by implication.

So the honest split is: fix what my change broke (the comment), leave what predates it (the output) for the full-notebook refresh that (1) unblocks. Both cells land in the same follow-up.

On CI not executing the notebooks — agreed, and it is the reason neither finding is caught by the suite. mortie/tests/test_example_notebooks.py says as much in its own docstring and points at the PR thread on #180 for why. Wiring up an execution runner is a dependency addition (§4) plus a workflow change, so it is not something to slip into this PR.

---

Generated by Claude Code

[codspeed-hq]

Merging this PR will degrade performance by 11.26%

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

❌ 2 regressed benchmarks
✅ 69 untouched benchmarks
⏩ 1 skipped benchmark¹

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

	Benchmark	BASE	HEAD	Efficiency
❌	test_coverage_triangle_order4	324.5 µs	370.7 µs	-12.45%
❌	coverage_square_polar[4]	231.2 µs	257 µs	-10.06%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.

---

_{Comparing claude/142-notebook-normalized-address (35ba614) with main (b2ad13a)}

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

All three phases are landed and CI is green on eb25647 — test 3.10/3.11/3.12, ruff, numpydoc validation, Build (verify only), and every wheel build pass; codecov reports all modified lines covered with project coverage flat at 96.67%. Locally: 1780 passed, 16 skipped against a main baseline of 1776/16.

Marking waiting on one blocking question — it is question (1) in the PR body, restated here so it is answerable in one pass.

Cell 15 now yields 373 cells, but the markdown cell directly below it claims "Still 380 cells like in the other example". That claim asserts mortie agrees with the healpy cells above it, which is only true under the legacy latitude convention:

route	order=9 unique	clip2order(6, order=18) unique
latitude="authalic" (current default)	373	40
latitude="geodetic-spherical" (legacy)	380	39
healpy ang2pix (cells 9 / 12)	380	39

• (a) pass latitude="geodetic-spherical" in cell 15, matching what healpy does in cells 9/12, and keep the narrative intact;
• (b) keep the authalic default and rewrite cell 16's 380 claim, dropping the healpy-agreement framing.

I did not pick one because it decides which convention the flagship example is pinned to — a #186 call rather than a #142 bug fix. Worth noting the mismatch is not introduced here: on main the cell raises for basin 4 (base cell 11), so 380 was already unreachable; this changes it from unreachable to reachable-but-373.

Whichever way it goes, the answer also unblocks question (2) — regenerating the notebook's committed outputs, which are all still in the retired decimal encoding — and I would fold both plus question (3)'s one-line data-path fix into a follow-up phase here.

Two smaller items are flagged in the PR body and need no decision now: the notebook's data path does not resolve from examples/ (question 3), and cell 3's delim_whitespace=True was removed in pandas 3.0 (question 4). Both block any re-execution of the notebook but are untouched here as out of scope.

One process note for the record: no subagent-spawning tool was available in this session, so the adversarial review was run through the repo's code-review skill in a forked execution with fresh context rather than as a spawned review subagent. It produced four findings; the two diff-scoped ones are folded in eb25647 and the two notebook ones are answered on their thread.

---

Generated by Claude Code

[espg]

🤖 from Claude

All four "Questions for review" are resolved per the in-session rulings (2026-08-24):

• (1) Keep the latitude="authalic" default — cell 16 rewritten to teach the count difference instead of claiming 380: healpy's spherical ang2pix gives 380; latitude="geodetic-spherical" reproduces that; the authalic default is equal-area on the WGS84 ellipsoid by construction (Latitude convention decision: geodetic-as-spherical (status quo) vs authalic (equal-area by construction) — window closes with zagg's δ rebuild #186/Authalic latitude convention: new default latitude="authalic" (issue #186) #188). Cell 15 prints 373 at order 9. — 26deb75
• (2) Whole notebook regenerated top-to-bottom against current main (merged in first: 490c420, picking up the Spec: normative toc word grammar, frozen for 1.x (issue #193) #206/workspace split: extract mortie-core (issue #200) #207-era work). All outputs are now packed-uint64 morton words — cell 17's retired decimal form is gone; its clip-to-order-6 result is 40; both gnomview plots re-rendered; zero cell errors. — 35ba614
• (3) Option (a): cell 3 loads the in-tree fixture ../mortie/tests/Ant_Grounded_DrainageSystem_Polygons.txt, so the notebook is launch-and-run. The NASA link in cell 2 stays as provenance. — 8f4f393
• (4) delim_whitespace=True → sep=r"\s+" (pandas 3.0 removed the former), folded into the same commit. — 8f4f393

Verification on the merged branch: pytest -q 1783 passed / 24 skipped (one full-suite-only flake in test_wkb_batch_memory.py, RSS-measurement noise, passes in isolation, pre-existing); jupyter nbconvert --execute clean end to end.

Binder note (standing question, no action taken): the repo already ships binder/environment.yml + binder/postBuild, and the badge in cell 0 points at this notebook — but postBuild installs mortie from a pinned PyPI wheel, so the Binder launch runs the released package, not this branch. The notebook's data and code paths are now Binder-clean; whether the pin should advance with the next release is release-workflow territory (issue #67) and out of scope here.

[espg]

🤖 from Claude

CI concluded on 35ba614: 17 pass, 0 real failures (release/publish jobs skipped as expected). The one red mark is CodSpeed Performance Analysis — known noise on this repo, and this PR changes only examples/ExampleUsage.ipynb plus the merge of main, so no perf-relevant code moved. Rust and Python benchmark jobs themselves both pass. Marked ready for review; implement + waiting labels stand.