Skip to content
Closed
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
13 changes: 11 additions & 2 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ set(LIB_SOURCE_FILES
src/h3lib/lib/localij.c
src/h3lib/lib/latLng.c
src/h3lib/lib/directedEdge.c
src/h3lib/lib/iterGosper.c
src/h3lib/lib/mathExtensions.c
src/h3lib/lib/iterators.c
src/h3lib/lib/faceijk.c
Expand Down Expand Up @@ -281,6 +282,7 @@ set(OTHER_SOURCE_FILES
src/apps/testapps/testH3IteratorsInternal.c
src/apps/testapps/testMathExtensionsInternal.c
src/apps/testapps/testDescribeH3Error.c
src/apps/testapps/testGosperIter.c
src/apps/testapps/testGeoLoopArea.c
src/apps/miscapps/cellToBoundaryHier.c
src/apps/miscapps/cellToLatLngHier.c
Expand Down Expand Up @@ -320,6 +322,7 @@ set(OTHER_SOURCE_FILES
src/apps/benchmarks/benchmarkGridDiskCells.c
src/apps/benchmarks/benchmarkGridPathCells.c
src/apps/benchmarks/benchmarkDirectedEdge.c
src/apps/benchmarks/benchmarkGosperIter.c
src/apps/benchmarks/benchmarkVertex.c
src/apps/benchmarks/benchmarkIsValidCell.c
src/apps/benchmarks/benchmarkH3Api.c
Expand Down Expand Up @@ -672,11 +675,17 @@ if(BUILD_BENCHMARKS)
src/apps/benchmarks/benchmarkGridPathCells.c)
add_h3_benchmark(benchmarkDirectedEdge
src/apps/benchmarks/benchmarkDirectedEdge.c)
if(ENABLE_REQUIRES_ALL_SYMBOLS)
add_h3_benchmark(benchmarkGosperIter
src/apps/benchmarks/benchmarkGosperIter.c)
endif()
add_h3_benchmark(benchmarkVertex src/apps/benchmarks/benchmarkVertex.c)
add_h3_benchmark(benchmarkIsValidCell
src/apps/benchmarks/benchmarkIsValidCell.c)
add_h3_benchmark(benchmarkCellsToPolyAlgos
src/apps/benchmarks/benchmarkCellsToPolyAlgos.c)
if(ENABLE_REQUIRES_ALL_SYMBOLS)
add_h3_benchmark(benchmarkCellsToPolyAlgos
src/apps/benchmarks/benchmarkCellsToPolyAlgos.c)
endif()
add_h3_benchmark(benchmarkCellToChildren
src/apps/benchmarks/benchmarkCellToChildren.c)
add_h3_benchmark(benchmarkPolygonToCells
Expand Down
1 change: 1 addition & 0 deletions CMakeTests.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,7 @@ add_h3_test(testH3IteratorsInternal src/apps/testapps/testH3IteratorsInternal.c)
add_h3_test(testMathExtensionsInternal
src/apps/testapps/testMathExtensionsInternal.c)
add_h3_test(testDescribeH3Error src/apps/testapps/testDescribeH3Error.c)
add_h3_test(testGosperIter src/apps/testapps/testGosperIter.c)
add_h3_test(testGeoLoopArea src/apps/testapps/testGeoLoopArea.c)

add_h3_test_with_arg(testH3NeighborRotations
Expand Down
42 changes: 42 additions & 0 deletions branch_status.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
# Branch: `child_edge_iter`

**Date:** 2026-03-08 21:01 PDT

## Summary

This branch adds a Gosper island boundary iterator (`IterGosper`) and integrates it into the `cellsToMultiPolygon` pipeline. The iterator walks the fractal boundary of a cell's child set at a given resolution, yielding directed edges in geometric (CCW) order without enumerating any internal edges.

## What was done

### Gosper iterator

- **`gosperIter.h` / `gosperIter.c`** — New iterator that yields the directed edges forming the outline of a cell's child set at a target resolution. Works for any parent/child resolution gap (0 to 15), hexagons and pentagons. Uses a boundary walk algorithm: at each resolution level, the outline of a parent's child set follows a repeating 18-step cycle (3 boundary segments per face, 6 faces).
- **`mathExtensions.h`** — Added `_ipow()` helper for integer exponentiation.
- **`testGosperIter.c`** — Exhaustive tests for res 0-4 (all cells x all child resolutions), plus high-res tests covering hex/pentagon at res 5/8/10 with gaps reaching res 15. Verifies edge count, validity, boundary membership, and loop connectivity. Includes expected-edge-sequence tests generated by an independent implementation.
- **`benchmarkGosperIter.c`** — Benchmarks for the iterator at various resolution gaps.
- **`CMakeLists.txt` / `CMakeTests.cmake`** — Build integration for tests and benchmarks.

### Integration into cellsToMultiPolygon

- **`cellsToMultiPoly.c`** — Added `cellsToMultiPolygonGosper()`, a new entry point that accepts possibly-compacted (mixed-resolution) cells and a target resolution. Uses the Gosper iterator to enumerate only boundary edges, then feeds them through the existing cancel-pairs / extract-loops / build-polygons pipeline. Extracted shared `arcSetToMultiPolygon()` helper to eliminate duplication between the flat and Gosper paths. Added overflow check for safety.
- **`cellsToMultiPoly.h`** — Declared `cellsToMultiPolygonGosper()`.
- **`benchmarkCellsToPolyAlgos.c`** — Added Colorado-based benchmarks at res 6, 7, and 8 comparing direct (flat), Gosper (flat), and Gosper (compacted) approaches. Extracted `coloradoCells()` and `compactAndCount()` helpers.

## Benchmark results

Compacted cells + Gosper iterator vs. flat `cellsToMultiPolygon` on Colorado:

| Resolution | Direct (flat) | Gosper (flat) | Gosper (compact) | Compact speedup |
|------------|---------------|---------------|------------------|-----------------|
| 6 | 1,932 us | 1,711 us | 537 us | 3.6x |
| 7 | 24,371 us | 20,367 us | 1,908 us | 12.8x |
| 8 | 265,051 us | 240,309 us | 7,640 us | 34.7x |

The Gosper iterator on flat (same-resolution) cells is only ~10-15% faster than the direct approach, since both enumerate the same set of edges. The real win comes from compaction: `compactCells` replaces complete child sets with their parents, and the Gosper iterator then walks the parent's boundary directly, skipping all internal edges. The speedup grows with resolution because compaction removes exponentially more cells.

## Remaining work

- Decide on public API surface for `cellsToMultiPolygonGosper` (currently internal)
- Add tests for the Gosper integration path (error cases, compacted input, mixed resolutions)
- Add input validation to `cellsToMultiPolygonGosper`: require sorted, pre-compacted input and verify cheaply with a linear scan (sorted order, no overlapping ancestors, no complete sibling sets). May need right-to-left scan for the sibling check.
- Consider whether the public `cellsToMultiPolygon` should auto-compact internally and delegate to the Gosper path
145 changes: 145 additions & 0 deletions edge_iter_plan.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
# GosperIter: Fresh Branch Plan

Reimplement the EdgeIter from the `child_edge_iter` branch on a clean
branch off `master` as `GosperIter`, incorporating improvements observed
in the Zig implementation (`h3cellset/src/edge_iter.zig`).

The name `GosperIter` reflects the geometry: the iterator traces the
Gosper island outline (the boundary of a cell's child set), distinguishing
it from a hypothetical iterator that yields all directed edges (interior
+ boundary).

## Background

The `child_edge_iter` branch has diverged from master and carries scratch
files (`notes.c`, `todo.md`, justfile changes). The algorithm itself is
solid -- identical to the proven Zig version -- but the C code misses a
couple of cleanups the Zig version got right.

## Naming Convention

Follow the existing iterator naming pattern from `iterators.h`:

| Existing | GosperIter |
|------------------------|-------------------------------|
| `IterCellsChildren` | `IterGosper` |
| `iterInitParent(h, r)` | `iterInitGosper(h, childRes)` |
| `iterStepChild(&iter)` | `iterStepGosper(&iter)` |
| `.h` (current cell) | `.e` (current directed edge) |

The old branch used `init_EdgeIter` / `step_EdgeIter` which doesn't
match codebase conventions.

## Scope

- **Internal API only** -- no `DECLSPEC`, no `H3_EXPORT`. This is a
building block for `cellsToMultiPolygon`, not a public function.
- **No input validation** -- callers are internal and already validate
upstream.
- **Own header/source** -- `gosperIter.h` / `gosperIter.c`, separate
from `iterators.h` since the concern is different.

## Priority: Clarity Over Speed

This iterator is expected to provide a significant speed benefit over
the current approach (enumerating child cells then computing boundaries),
but the goal of this initial implementation is **clarity for code
review**, not maximum performance. An optimization pass will follow.

That said, avoid structural choices that would require large refactors
later. Specifically: keep the iterator state self-contained in the
struct (no heap allocation), keep the per-step work O(1), and keep the
boundary walk logic in its own static function so it can be replaced or
inlined independently. The current algorithm already satisfies all of
these -- just don't trade them away for readability shortcuts.

## Files to Create

| File | Description |
|---------------------------------------------|-------------------------|
| `src/h3lib/include/gosperIter.h` | Struct + init/step API |
| `src/h3lib/lib/gosperIter.c` | Iterator implementation |
| `src/apps/testapps/testGosperIter.c` | Test suite |
| `src/apps/benchmarks/benchmarkGosperIter.c` | Benchmark suite |

## Files to Modify

| File | Change |
|--------------------------------------|---------------------------------------|
| `CMakeLists.txt` | Add header, source, benchmark entries |
| `CMakeTests.cmake` | Add `testGosperIter` test entry |
| `src/h3lib/include/mathExtensions.h` | Add `MIN` macro |

## Improvements Over Current Branch

### 1. Same-resolution fast path in `init`

Borrowed from Zig. When `parent_res == child_res`, skip the `I` array
setup, `H3_SET_RESOLUTION`, and digit-setting entirely. Just set mode
and reserved bits on the input cell directly.

```c
if (parent_res == child_res) {
// No digit setup, resolution change, or I[] init needed
iter.e = h;
H3_SET_MODE(iter.e, H3_DIRECTEDEDGE_MODE);
H3_SET_RESERVED_BITS(iter.e, edge_seq[0]);
iter.num_edges = is_pentagon ? 5 : 6;
return iter;
}
```

### 2. Simpler same-resolution stepping

Borrowed from Zig. Replace the cryptic modular-arithmetic loop:

```c
// Before (current branch)
do {
ei->i = (ei->i + 7) % 6;
} while (ei->is_pentagon && edge_seq[ei->i] == 1);
```

With a straight increment and pentagon skip:

```c
// After
iter->_i++;
if (iter->_isPentagon && iter->_i == 1) iter->_i = 2;
```

This works because `num_edges` guarantees we never wrap past index 5.
Clearer intent, no modular arithmetic for the simple case.

### 3. Consistent naming

Rename `init_EdgeIter` / `step_EdgeIter` to `iterInitGosper` /
`iterStepGosper`. Prefix internal struct fields with `_` to match
`IterCellsChildren` convention (e.g. `_parentRes`, `_skipDigit`).

### 4. Drop scratch files

The new branch carries only the iterator itself -- no `notes.c`,
`todo.md`, `.gitignore` changes, or `justfile` additions.

## Not Changing

- **API shape**: `init` returns struct, `.e` is current value (`H3_NULL`
when done), `step` advances. Matches existing `IterCellsChildren`
convention.
- **Core algorithm**: The boundary walk (`base[18]`, `edge_seq[6]`,
`+19 mod 18`, `-6` parent adjustment, recursive `step_boundaryCell`)
is proven correct in both C and Zig with exhaustive testing.
- **Test coverage**: Exhaustive res 0-4 + specific edge sequences +
loop connectivity + per-edge validity. Already matches the Zig suite.
- **Benchmark coverage**: Same-res through +11 gap, hex and pentagon,
multiple base resolutions.

## Steps

1. `git checkout master && git checkout -b gosper_iter`
2. Create `gosperIter.h` and `gosperIter.c` with improvements above
3. Create `testGosperIter.c` and `benchmarkGosperIter.c`
4. Update `CMakeLists.txt` and `CMakeTests.cmake`
5. Add `MIN` macro to `mathExtensions.h`
6. Build and run tests: `just build && just test`
64 changes: 64 additions & 0 deletions justfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
init: purge
mkdir build

build:
cd build; cmake -DCMAKE_BUILD_TYPE=Release ..; make

purge:
rm -rf build
rm -rf *.trace
rm -rf .ipynb_checkpoints
rm -rf .cache
rm -rf .claude

test-fast: build
cd build; make test-fast

test-slow: build
cd build; make test

# Run a single test binary. Dots (progress) go to /dev/null; failures print to stderr.
test-one TEST: build
./build/bin/{{TEST}} > /dev/null

test:
just test-fast

bench: build
./build/bin/benchmarkCellsToPolyAlgos

coverage: purge
mkdir build
cd build; cmake -DCMAKE_BUILD_TYPE=Debug -DENABLE_COVERAGE=ON ..; make; make coverage
open build/coverage/index.html

# Show uncovered lines/branches for a source file from the lcov .info data.
# Run `just coverage` first. Example: just coverage-gaps linkedGeo.c
coverage-gaps FILE:
#!/usr/bin/env bash
set -e
info="build/coverage.cleaned.info"
if [ ! -f "$info" ]; then
echo "No coverage data found — run 'just coverage' first."
exit 1
fi
# Extract the section for this file
section=$(sed -n "/SF:.*\/{{FILE}}$/,/end_of_record/p" "$info")
if [ -z "$section" ]; then
echo "File {{FILE}} not found in coverage data."
echo "Available files:"
grep '^SF:' "$info" | sed 's|.*src/h3lib/||'
exit 1
fi
echo "=== Summary ==="
lf=$(echo "$section" | grep '^LF:' | cut -d: -f2)
lh=$(echo "$section" | grep '^LH:' | cut -d: -f2)
brf=$(echo "$section" | grep '^BRF:' | cut -d: -f2)
brh=$(echo "$section" | grep '^BRH:' | cut -d: -f2)
echo "Lines: $lh/$lf Branches: $brh/$brf"
echo ""
echo "=== Uncovered lines (DA:line,0) ==="
echo "$section" | grep '^DA:' | awk -F'[,:]' '$3 == 0 {print " line " $2}' || echo " (none)"
echo ""
echo "=== Untaken branches (BRDA:line,block,branch,0) ==="
echo "$section" | grep '^BRDA:' | awk -F'[,:]' '$5 == 0 {print " line " $2 " branch " $4}' || echo " (none)"
1 change: 1 addition & 0 deletions src/apps/applib/include/benchmark.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#ifdef _WIN32

#define NOGDI
#define WIN32_LEAN_AND_MEAN
#include <Windows.h>

#define START_TIMER \
Expand Down
Loading
Loading