Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions docs/specification.md
Original file line number Diff line number Diff line change
Expand Up @@ -140,15 +140,15 @@ cell spacing `sqrt(4πR² / (12 · 4^order))`. The table below is regenerated
from these formulas and pinned by `mortie/tests/test_spec_page.py` so it
cannot drift.

**Note — code vs. page.** These are the **normative, sphere-derived**
values. `mortie.tools.order2res` in *code* retains the historical flat
constant `111 km/deg × 58.6323 × 0.5^order` (an implied sphere `R ≈ 6366
km`) for **behavioral compatibility** — it is consumed by `res2display` and
by buffer-pad computations (e.g. `tests/test_coverage_boundary.py`, which
scales the coverage pad by `order2res`) whose outputs would shift if the
constant changed. The page's cell-scale column is therefore ~0.2% larger
than `order2res` at every order. Unifying `order2res` onto this sphere is a
behavioral change tracked separately in
**Note — code and page unified.** These are the **normative, sphere-derived**
values, and `mortie.tools.order2res` now derives from the same sphere:
`order2res(order) = sqrt(4πR² / (12 · 4^order))` with the single
`mortie.tools.EARTH_RADIUS_KM = 6371.0088` constant. Its consumers
(`res2display` and the buffer-pad computation in
`tests/test_coverage_boundary.py`) therefore read the cell-scale column
below directly. This replaced the historical flat constant
`111 km/deg × 58.6323 × 0.5^order` (an implied sphere `R ≈ 6366 km`), a
behavioral change of ~0.2% at every order, per
[mortie #119](https://github.com/espg/mortie/issues/119).

<!-- table:order2res:begin -->
Expand Down
11 changes: 5 additions & 6 deletions mortie/tests/test_spec_page.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,9 @@
**Both** columns derive from the exact HEALPix sphere at mean radius
``EARTH_RADIUS_KM``: every order-k cell has identical area
``4*pi*R**2 / (12 * 4**k)``, and the cell scale is the square root of that
area (RMS cell spacing). This is the sphere-derived normative value, not the
historical ``order2res`` constant kept in ``mortie.tools`` for behavioral
compatibility (spec §3 code-vs-page note; unification tracked as a mortie
follow-up issue). This test compares every regenerated row literally against
area (RMS cell spacing). ``mortie.tools.order2res`` is derived from this same
``EARTH_RADIUS_KM`` sphere (issue #119), so the code and page now share one
Earth model. This test compares every regenerated row literally against
the rows between the ``table:order2res`` markers; to refresh the doc after a
deliberate formula change, paste the rows this module's ``table_rows()``
produces.
Expand All @@ -17,13 +16,13 @@
import math
from pathlib import Path

from mortie.tools import MAX_ORDER
from mortie.tools import EARTH_RADIUS_KM, MAX_ORDER

SPEC_PAGE = Path(__file__).resolve().parents[2] / "docs" / "specification.md"
BEGIN = "<!-- table:order2res:begin -->"
END = "<!-- table:order2res:end -->"
# Exact HEALPix sphere, mean Earth radius (km); drives BOTH table columns.
EARTH_RADIUS_KM = 6371.0088
# order2res is now unified onto this same sphere (issue #119).


def format_row(order):
Expand Down
29 changes: 18 additions & 11 deletions mortie/tests/test_tools.py

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ [ruff] <I001> reported by reviewdog 🐶
Import block is un-sorted or un-formatted

import math
import pytest
import numpy as np
from numpy.testing import assert_array_equal, assert_allclose
from mortie import tools

Original file line number Diff line number Diff line change
Expand Up @@ -11,32 +11,39 @@
- Tests focus on consistency, determinism, and structural validation
"""

import math

import pytest
import numpy as np
from numpy.testing import assert_array_equal, assert_allclose

from mortie import tools


def _sphere_res(order):
"""Reference RMS cell spacing (km): sqrt of the equal-area cell area."""
R = tools.EARTH_RADIUS_KM
return math.sqrt(4 * math.pi * R**2 / (12 * 4**order))


class TestOrder2Res:
"""Test order to resolution conversion"""

def test_order2res_basic(self):
"""Test basic order to resolution calculations"""
# Order 0 should be largest resolution
# Order 0 is the RMS cell spacing on the unified HEALPix sphere.
res0 = tools.order2res(0)
assert_allclose(res0, 111 * 58.6323, rtol=1e-10)
assert_allclose(res0, _sphere_res(0), rtol=1e-10)

# Order 1 should be half of order 0
# Each order halves the cell scale (area drops by 4).
res1 = tools.order2res(1)
assert_allclose(res1, res0 / 2.0, rtol=1e-10)

def test_order2res_range(self):
"""Test full range of valid orders"""
"""Test full range of valid orders against the sphere formula"""
for order in range(20):
res = tools.order2res(order)
expected = 111 * 58.6323 * (0.5 ** order)
assert_allclose(res, expected, rtol=1e-10)
assert_allclose(res, _sphere_res(order), rtol=1e-10)

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 from Claude (review)

Self-referential pin (acceptable, but worth one literal anchor). Both test_order2res_range and test_order2res_basic now assert order2res(order) == _sphere_res(order), where _sphere_res recomputes the same formula sqrt(4πR²/(12·4^order)) with the same tools.EARTH_RADIUS_KM. This checks that order2res follows the formula, but it can no longer catch a wrong constant/formula: flip EARTH_RADIUS_KM and both sides move together, so the test stays green. Contrast the old pin (111 * 58.6323), which was an independent literal.

Coverage isn't actually lost overall — TestRes2Display pins hard literal strings that flow through order2res (1.592 km@12, 795.852 m@13, 19.43 cm@25, 1.214 cm@29 — all recomputed and correct), and test_spec_page pins the committed table literals (6,519.623 km@0), so a bad R would fail those. So this is a suggestion, not a defect: consider adding one literal assertion here (e.g. assert_allclose(tools.order2res(0), 6519.623, atol=1e-3)) so the direct order2res test is self-anchoring rather than relying on downstream tests.


Generated by Claude Code


def test_order2res_decreasing(self):
"""Test that resolution decreases with order"""
Expand Down Expand Up @@ -72,12 +79,12 @@ def test_unit_ladder_km_m_cm(self, capsys):
tools.res2display()
lines = capsys.readouterr().out.strip().split('\n')
by_order = {int(line.rsplit(' ', 1)[1]): line for line in lines}
# order 12 = 1.589 km, order 13 = 794.456 m (the issue's examples)
assert by_order[12] == '1.589 km at tessellation order 12'
assert by_order[13] == '794.456 m at tessellation order 13'
# order 12 = 1.592 km, order 13 = 795.852 m (unified sphere, issue #119)
assert by_order[12] == '1.592 km at tessellation order 12'
assert by_order[13] == '795.852 m at tessellation order 13'
# finest orders drop to cm rather than tiny km/m fractions
assert by_order[25] == '19.396 cm at tessellation order 25'
assert by_order[29] == '1.212 cm at tessellation order 29'
assert by_order[25] == '19.43 cm at tessellation order 25'
assert by_order[29] == '1.214 cm at tessellation order 29'

def test_rounds_within_bracket(self, capsys):
"""Values are rounded to three decimals inside the chosen unit"""
Expand Down
19 changes: 16 additions & 3 deletions mortie/tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@
from . import _healpix as hp
from . import _rustie

# Mean Earth radius (km), the exact HEALPix sphere the spec page's resolution
# table (docs/specification.md §3) derives from. order2res is the RMS cell
# spacing on this sphere; unified with the page in issue #119.
EARTH_RADIUS_KM = 6371.0088

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 from Claude (review)

Non-blocking observation (out of scope for this diff). The module now carries two Earth-radius constants: EARTH_RADIUS_KM = 6371.0088 (here) and the pre-existing _EARTH_RADIUS_M = 6_371_008.7714 at line 752 (used by the buffer cell_width_m). These are the same physical radius to ~0.03 m (6371.0088 km = 6,371,008.8 m vs 6,371,008.7714 m). Not a defect and not introduced by #119, but a future cleanup could single-source them so the buffer path and order2res provably share one Earth model. Flagging only; no change needed in this PR.


Generated by Claude Code


# Rust-native geo2mort (uses healpix crate, no Python HEALPix backend)
_rust_geo2mort = _rustie.rust_geo2mort
# Packed-word kernel bridge: morton <-> HEALPix NESTED (vectorized).
Expand All @@ -20,8 +25,16 @@


def order2res(order):
res = 111 * 58.6323 * .5**order
return res
"""Approximate cell scale (km) at a HEALPix tessellation ``order``.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 from Claude (review)

Nit (wording): the summary line calls this "Approximate cell scale", but the body — correctly — says it is the exact RMS cell spacing on the equal-area HEALPix sphere (sqrt(area)). There is no approximation in the closed form; the only "approx" is that RMS spacing stands in for a per-cell shape. Consider dropping "Approximate" (or say "RMS cell scale") so the one-liner doesn't undersell the exactness the rest of the docstring establishes. Not blocking.


Generated by Claude Code


The exact RMS cell spacing on the mean-radius HEALPix sphere: every
order-k cell has identical area ``4*pi*R**2 / (12 * 4**order)`` (HEALPix is
equal-area), and the cell scale is the square root of that area. Derived
from :data:`EARTH_RADIUS_KM` so code and the spec page (§3) share one Earth
model (issue #119).
"""
area = 4 * np.pi * EARTH_RADIUS_KM**2 / (12 * 4**order) # km2
return float(np.sqrt(area))

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 from Claude (review)

Minor behavioral narrowing: float(np.sqrt(area)) forces a scalar return, whereas the old 111 * 58.6323 * .5**order broadcast over an array order. I checked every consumer (res2display, test_coverage_boundary.py:78, USAGE.md, the notebooks) and all pass a scalar, so nothing breaks today — but if any caller ever passes an array of orders, float() on a non-scalar array raises TypeError. If you want to keep the previous vectorization, np.sqrt(area) (unwrapped) would return a numpy scalar for scalar input and an array for array input. Purely a note; the scalar form is fine for current usage.


Generated by Claude Code



def res2display(max_order=MAX_ORDER):
Expand All @@ -32,7 +45,7 @@ def res2display(max_order=MAX_ORDER):
Each resolution is rendered in the largest sensible unit -- km at coarse
orders, m once it drops below 1 km, cm once it drops below 1 m -- and
rounded to three decimals within that bracket, so fine orders read
naturally (e.g. order 12 -> ``1.589 km``, order 13 -> ``794.456 m``)
naturally (e.g. order 12 -> ``1.592 km``, order 13 -> ``795.852 m``)
rather than as tiny km fractions.

``max_order`` must lie in 0..MAX_ORDER, the order range the packed-u64
Expand Down
Loading