Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Alternate maxPolygonToCellsSize implementation #805

Merged
merged 9 commits into from
Dec 4, 2023
Merged

Alternate maxPolygonToCellsSize implementation #805

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
7f4641d
First pass, implement polyfill-based maxPolygonToCells
Nov 26, 2023
692e3d7
Fix logic in max size estimator
Nov 29, 2023
4dded6a
Fix tests
Nov 29, 2023
162c443
Use new estimator in benchmarks
Nov 29, 2023
6d20575
Use new estimator in tests
Nov 29, 2023
31ee58b
Drop threshold to 10
Nov 29, 2023
795def8
Fail estimator for invalid flags
Nov 29, 2023
1ead20f
Fix coverage
Nov 29, 2023
c5556bb
Comment update
nrabinowitz Dec 4, 2023
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
17 changes: 13 additions & 4 deletions scripts/make_countries.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,41 +140,50 @@ const GeoPolygon COUNTRIES[${polygons.length}] = {${
BEGIN_BENCHMARKS();

int MAX_RES = 5;
// Max size of largest polygon at res 5, rounded up with padding
int64_t numHexagons = 400000;
H3Index *hexagons = calloc(numHexagons, sizeof(H3Index));
int64_t numHexagons;
H3Index *hexagons;

for (int res = 0; res < MAX_RES + 1; res++) {

printf("Res %d", res);

BENCHMARK(polygonToCells_AllCountries1, 5, {
for (int index = 0; index < ${polygons.length}; index++) {
H3_EXPORT(maxPolygonToCellsSize)(&COUNTRIES[index], res, CONTAINMENT_CENTER, &numHexagons);
hexagons = calloc(numHexagons, sizeof(H3Index));
H3_EXPORT(polygonToCells)(&COUNTRIES[index], res, CONTAINMENT_CENTER, hexagons);
free(hexagons);
}
});

BENCHMARK(polygonToCells_AllCountries2, 5, {
for (int index = 0; index < ${polygons.length}; index++) {
H3_EXPORT(maxPolygonToCellsSizeExperimental)(&COUNTRIES[index], res, CONTAINMENT_CENTER, &numHexagons);
hexagons = calloc(numHexagons, sizeof(H3Index));
H3_EXPORT(polygonToCellsExperimental)(&COUNTRIES[index], res, CONTAINMENT_CENTER, hexagons);
free(hexagons);
}
});

BENCHMARK(polygonToCells_AllCountries3, 5, {
for (int index = 0; index < ${polygons.length}; index++) {
H3_EXPORT(maxPolygonToCellsSizeExperimental)(&COUNTRIES[index], res, CONTAINMENT_CENTER, &numHexagons);
hexagons = calloc(numHexagons, sizeof(H3Index));
H3_EXPORT(polygonToCellsExperimental)(&COUNTRIES[index], res, CONTAINMENT_FULL, hexagons);
}
});

BENCHMARK(polygonToCells_AllCountries4, 5, {
for (int index = 0; index < ${polygons.length}; index++) {
H3_EXPORT(maxPolygonToCellsSizeExperimental)(&COUNTRIES[index], res, CONTAINMENT_CENTER, &numHexagons);
hexagons = calloc(numHexagons, sizeof(H3Index));
H3_EXPORT(polygonToCellsExperimental)(&COUNTRIES[index], res, CONTAINMENT_OVERLAPPING, hexagons);
free(hexagons);
}
});

}

free(hexagons);

END_BENCHMARKS();
`
Expand Down
26 changes: 21 additions & 5 deletions src/apps/testapps/testPolyfillInternal.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,10 @@ SUITE(polyfillInternal) {
IterCellsPolygonCompact iter;
H3Index cell;

// Give the iterator a cell with a bad base cell
iter = iterInitPolygonCompact(&sfGeoPolygon, 9, CONTAINMENT_CENTER);
t_assertSuccess(iter.error);

// Give the iterator a cell with a bad base cell
cell = 0x85283473fffffff;
H3_SET_BASE_CELL(cell, 123);
iter.cell = cell;
Expand All @@ -81,10 +81,10 @@ SUITE(polyfillInternal) {
"Got expected error for invalid cell");
t_assert(iter.cell == H3_NULL, "Got null output for invalid cell");

// Give the iterator a cell with a bad base cell, at the target res
iter = iterInitPolygonCompact(&sfGeoPolygon, 9, CONTAINMENT_CENTER);
t_assertSuccess(iter.error);

// Give the iterator a cell with a bad base cell, at the target res
cell = 0x89283470003ffff;
H3_SET_BASE_CELL(cell, 123);
iter.cell = cell;
Expand All @@ -95,11 +95,11 @@ SUITE(polyfillInternal) {
t_assert(iter.cell == H3_NULL,
"Got null output for invalid cell at res");

// Give the iterator a cell with a bad base cell, at the target res
// (full containment)
iter = iterInitPolygonCompact(&sfGeoPolygon, 9, CONTAINMENT_FULL);
t_assertSuccess(iter.error);

// Give the iterator a cell with a bad base cell, at the target res
// (full containment)
cell = 0x89283470003ffff;
H3_SET_BASE_CELL(cell, 123);
iter.cell = cell;
Expand All @@ -110,12 +110,28 @@ SUITE(polyfillInternal) {
t_assert(iter.cell == H3_NULL,
"Got null output for invalid cell at res");

iter = iterInitPolygonCompact(&sfGeoPolygon, 9, CONTAINMENT_CENTER);
// Give the iterator a cell with a bad base cell, at the target res
// (overlapping bounding box)
iter = iterInitPolygonCompact(&sfGeoPolygon, 9,
CONTAINMENT_OVERLAPPING_BBOX);
t_assertSuccess(iter.error);

cell = 0x89283470003ffff;
H3_SET_BASE_CELL(cell, 123);
iter.cell = cell;

iterStepPolygonCompact(&iter);
t_assert(iter.error == E_CELL_INVALID,
"Got expected error for invalid cell");
t_assert(iter.cell == H3_NULL,
"Got null output for invalid cell at res");

// Give the iterator a cell that's too fine for a child check,
// and a target resolution that allows this to run. This cell has
// to be inside the polygon to reach the error.
iter = iterInitPolygonCompact(&sfGeoPolygon, 9, CONTAINMENT_CENTER);
t_assertSuccess(iter.error);

cell = 0x8f283080dcb019a;
iter.cell = cell;
iter._res = 42;
Expand Down
5 changes: 3 additions & 2 deletions src/apps/testapps/testPolygonToCells.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include "constants.h"
#include "h3Index.h"
#include "latLng.h"
#include "polygon.h"
#include "test.h"
#include "utility.h"

Expand Down Expand Up @@ -461,7 +462,7 @@ SUITE(polygonToCells) {

TEST(invalidFlags) {
int64_t numHexagons;
for (uint32_t flags = 3; flags <= 32; flags++) {
for (uint32_t flags = CONTAINMENT_INVALID; flags <= 32; flags++) {
t_assert(
H3_EXPORT(maxPolygonToCellsSize)(
&sfGeoPolygon, 9, flags, &numHexagons) == E_OPTION_INVALID,
Expand All @@ -471,7 +472,7 @@ SUITE(polygonToCells) {
t_assertSuccess(H3_EXPORT(maxPolygonToCellsSize)(&sfGeoPolygon, 9, 0,
&numHexagons));
H3Index *hexagons = calloc(numHexagons, sizeof(H3Index));
for (uint32_t flags = 3; flags <= 32; flags++) {
for (uint32_t flags = CONTAINMENT_INVALID; flags <= 32; flags++) {
t_assert(H3_EXPORT(polygonToCells)(&sfGeoPolygon, 9, flags,
hexagons) == E_OPTION_INVALID,
"Flags other than polyfill modes are invalid for "
Expand Down
52 changes: 26 additions & 26 deletions src/apps/testapps/testPolygonToCellsExperimental.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,8 @@ static void fillIndex_assertions(H3Index h) {
.holes = 0};

int64_t polygonToCellsSize;
t_assertSuccess(H3_EXPORT(maxPolygonToCellsSize)(&polygon, nextRes, 0,
&polygonToCellsSize));
t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)(
&polygon, nextRes, 0, &polygonToCellsSize));
H3Index *polygonToCellsOut =
calloc(polygonToCellsSize, sizeof(H3Index));
t_assertSuccess(H3_EXPORT(polygonToCellsExperimental)(
Expand Down Expand Up @@ -161,7 +161,7 @@ SUITE(polygonToCells) {

TEST(polygonToCells) {
int64_t numHexagons;
t_assertSuccess(H3_EXPORT(maxPolygonToCellsSize)(
t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)(
&sfGeoPolygon, 9, CONTAINMENT_CENTER, &numHexagons));
H3Index *hexagons = calloc(numHexagons, sizeof(H3Index));

Expand All @@ -175,7 +175,7 @@ SUITE(polygonToCells) {

TEST(polygonToCells_FullContainment) {
int64_t numHexagons;
t_assertSuccess(H3_EXPORT(maxPolygonToCellsSize)(
t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)(
&sfGeoPolygon, 9, CONTAINMENT_FULL, &numHexagons));
H3Index *hexagons = calloc(numHexagons, sizeof(H3Index));

Expand All @@ -190,7 +190,7 @@ SUITE(polygonToCells) {

TEST(polygonToCells_Overlapping) {
int64_t numHexagons;
t_assertSuccess(H3_EXPORT(maxPolygonToCellsSize)(
t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)(
&sfGeoPolygon, 9, CONTAINMENT_OVERLAPPING, &numHexagons));
H3Index *hexagons = calloc(numHexagons, sizeof(H3Index));

Expand All @@ -205,7 +205,7 @@ SUITE(polygonToCells) {

TEST(polygonToCellsHole) {
int64_t numHexagons;
t_assertSuccess(H3_EXPORT(maxPolygonToCellsSize)(
t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)(
&holeGeoPolygon, 9, CONTAINMENT_CENTER, &numHexagons));
H3Index *hexagons = calloc(numHexagons, sizeof(H3Index));

Expand All @@ -220,7 +220,7 @@ SUITE(polygonToCells) {

TEST(polygonToCellsHoleFullContainment) {
int64_t numHexagons;
t_assertSuccess(H3_EXPORT(maxPolygonToCellsSize)(
t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)(
&holeGeoPolygon, 9, CONTAINMENT_FULL, &numHexagons));
H3Index *hexagons = calloc(numHexagons, sizeof(H3Index));

Expand All @@ -236,7 +236,7 @@ SUITE(polygonToCells) {

TEST(polygonToCellsHoleOverlapping) {
int64_t numHexagons;
t_assertSuccess(H3_EXPORT(maxPolygonToCellsSize)(
t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)(
&holeGeoPolygon, 9, CONTAINMENT_OVERLAPPING, &numHexagons));
H3Index *hexagons = calloc(numHexagons, sizeof(H3Index));

Expand All @@ -251,7 +251,7 @@ SUITE(polygonToCells) {

TEST(polygonToCellsEmpty) {
int64_t numHexagons;
t_assertSuccess(H3_EXPORT(maxPolygonToCellsSize)(
t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)(
&emptyGeoPolygon, 9, CONTAINMENT_CENTER, &numHexagons));
H3Index *hexagons = calloc(numHexagons, sizeof(H3Index));

Expand All @@ -266,7 +266,7 @@ SUITE(polygonToCells) {

TEST(polygonToCellsContainsPolygon) {
int64_t numHexagons;
t_assertSuccess(H3_EXPORT(maxPolygonToCellsSize)(
t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)(
&sfGeoPolygon, 4, CONTAINMENT_CENTER, &numHexagons));
H3Index *hexagons = calloc(numHexagons, sizeof(H3Index));

Expand All @@ -290,7 +290,7 @@ SUITE(polygonToCells) {
centerGeoPolygon.numHoles = 0;

int64_t numHexagons;
t_assertSuccess(H3_EXPORT(maxPolygonToCellsSize)(
t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)(
&centerGeoPolygon, 4, CONTAINMENT_CENTER, &numHexagons));
H3Index *hexagons = calloc(numHexagons, sizeof(H3Index));

Expand All @@ -306,7 +306,7 @@ SUITE(polygonToCells) {

TEST(polygonToCellsContainsPolygon_FullContainment) {
int64_t numHexagons;
t_assertSuccess(H3_EXPORT(maxPolygonToCellsSize)(
t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)(
&sfGeoPolygon, 4, CONTAINMENT_FULL, &numHexagons));
H3Index *hexagons = calloc(numHexagons, sizeof(H3Index));

Expand All @@ -321,7 +321,7 @@ SUITE(polygonToCells) {

TEST(polygonToCellsContainsPolygon_Overlapping) {
int64_t numHexagons;
t_assertSuccess(H3_EXPORT(maxPolygonToCellsSize)(
t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)(
&sfGeoPolygon, 4, CONTAINMENT_OVERLAPPING, &numHexagons));
H3Index *hexagons = calloc(numHexagons, sizeof(H3Index));

Expand Down Expand Up @@ -356,7 +356,7 @@ SUITE(polygonToCells) {
someHexagon.numHoles = 0;

int64_t numHexagons;
t_assertSuccess(H3_EXPORT(maxPolygonToCellsSize)(
t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)(
&someHexagon, 9, CONTAINMENT_CENTER, &numHexagons));
H3Index *hexagons = calloc(numHexagons, sizeof(H3Index));

Expand Down Expand Up @@ -416,7 +416,7 @@ SUITE(polygonToCells) {
// Prime meridian case
expectedSize = 4228;
int64_t numHexagons;
t_assertSuccess(H3_EXPORT(maxPolygonToCellsSize)(
t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)(
&primeMeridianGeoPolygon, 7, CONTAINMENT_CENTER, &numHexagons));
H3Index *hexagons = calloc(numHexagons, sizeof(H3Index));

Expand All @@ -431,7 +431,7 @@ SUITE(polygonToCells) {
// This doesn't exactly match the prime meridian count because of slight
// differences in hex size and grid offset between the two cases
expectedSize = 4238;
t_assertSuccess(H3_EXPORT(maxPolygonToCellsSize)(
t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)(
&transMeridianGeoPolygon, 7, CONTAINMENT_CENTER, &numHexagons));
H3Index *hexagonsTM = calloc(numHexagons, sizeof(H3Index));

Expand All @@ -444,7 +444,7 @@ SUITE(polygonToCells) {

// Transmeridian filled hole case -- only needed for calculating hole
// size
t_assertSuccess(H3_EXPORT(maxPolygonToCellsSize)(
t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)(
&transMeridianFilledHoleGeoPolygon, 7, CONTAINMENT_CENTER,
&numHexagons));
H3Index *hexagonsTMFH = calloc(numHexagons, sizeof(H3Index));
Expand All @@ -456,7 +456,7 @@ SUITE(polygonToCells) {
countNonNullIndexes(hexagonsTMFH, numHexagons);

// Transmeridian hole case
t_assertSuccess(H3_EXPORT(maxPolygonToCellsSize)(
t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)(
&transMeridianHoleGeoPolygon, 7, CONTAINMENT_CENTER, &numHexagons));
H3Index *hexagonsTMH = calloc(numHexagons, sizeof(H3Index));

Expand Down Expand Up @@ -484,7 +484,7 @@ SUITE(polygonToCells) {
GeoPolygon polygon = {.geoloop = geoloop, .numHoles = 0};

int64_t numHexagons;
t_assertSuccess(H3_EXPORT(maxPolygonToCellsSize)(
t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)(
&polygon, 4, CONTAINMENT_CENTER, &numHexagons));

H3Index *hexagons = calloc(numHexagons, sizeof(H3Index));
Expand Down Expand Up @@ -536,7 +536,7 @@ SUITE(polygonToCells) {
polygon.numHoles = 0;

int64_t numHexagons;
t_assertSuccess(H3_EXPORT(maxPolygonToCellsSize)(
t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)(
&polygon, 9, CONTAINMENT_CENTER, &numHexagons));
H3Index *hexagons = calloc(numHexagons, sizeof(H3Index));

Expand All @@ -560,21 +560,21 @@ SUITE(polygonToCells) {

TEST(invalidFlags) {
int64_t numHexagons;
for (uint32_t flags = 3; flags <= 32; flags++) {
for (uint32_t flags = CONTAINMENT_INVALID; flags <= 32; flags++) {
t_assert(
H3_EXPORT(maxPolygonToCellsSize)(
H3_EXPORT(maxPolygonToCellsSizeExperimental)(
&sfGeoPolygon, 9, flags, &numHexagons) == E_OPTION_INVALID,
"Flags other than polyfill modes are invalid for "
"maxPolygonToCellsSize");
"maxPolygonToCellsSizeExperimental");
}
t_assertSuccess(H3_EXPORT(maxPolygonToCellsSize)(
t_assertSuccess(H3_EXPORT(maxPolygonToCellsSizeExperimental)(
&sfGeoPolygon, 9, CONTAINMENT_CENTER, &numHexagons));
H3Index *hexagons = calloc(numHexagons, sizeof(H3Index));
for (uint32_t flags = 3; flags <= 32; flags++) {
for (uint32_t flags = CONTAINMENT_INVALID; flags <= 32; flags++) {
t_assert(H3_EXPORT(polygonToCellsExperimental)(
&sfGeoPolygon, 9, flags, hexagons) == E_OPTION_INVALID,
"Flags other than polyfill modes are invalid for "
"polygonToCells");
"polygonToCellsExperimental");
}
free(hexagons);
}
Expand Down
1 change: 1 addition & 0 deletions src/h3lib/include/bbox.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ bool bboxContains(const BBox *bbox, const LatLng *point);
bool bboxContainsBBox(const BBox *a, const BBox *b);
bool bboxOverlapsBBox(const BBox *a, const BBox *b);
bool bboxEquals(const BBox *b1, const BBox *b2);
CellBoundary bboxToCellBoundary(const BBox *bbox);
H3Error bboxHexEstimate(const BBox *bbox, int res, int64_t *out);
H3Error lineHexEstimate(const LatLng *origin, const LatLng *destination,
int res, int64_t *out);
Expand Down
5 changes: 5 additions & 0 deletions src/h3lib/include/polyfill.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,11 @@ DECLSPEC void iterDestroyPolygon(IterCellsPolygon *iter);
DECLSPEC H3Error H3_EXPORT(polygonToCellsExperimental)(
const GeoPolygon *polygon, int res, uint32_t flags, H3Index *out);

DECLSPEC H3Error H3_EXPORT(polygonToCellsExperimental)(
const GeoPolygon *polygon, int res, uint32_t flags, H3Index *out);
DECLSPEC H3Error H3_EXPORT(maxPolygonToCellsSizeExperimental)(
const GeoPolygon *polygon, int res, uint32_t flags, int64_t *out);

H3Error cellToBBox(H3Index cell, BBox *out, bool coverChildren);
H3Index baseCellNumToCell(int baseCellNum);

Expand Down
3 changes: 2 additions & 1 deletion src/h3lib/include/polygon.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ typedef enum {
CONTAINMENT_CENTER = 0, ///< Cell center is contained in the shape
CONTAINMENT_FULL = 1, ///< Cell is fully contained in the shape
CONTAINMENT_OVERLAPPING = 2, ///< Cell overlaps the shape at any point
CONTAINMENT_INVALID = 3 ///< This mode is invalid and should not be used
CONTAINMENT_OVERLAPPING_BBOX = 3, ///< Cell bounding box overlaps shape
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this purely an internal detail, or should be exposed to the user?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think users could potentially benefit from this - it's a faster (2-3x) version of CONTAINMENT_OVERLAPPING that may contain false positives, but no false negatives. Not appropriate for most use cases, but some users might find it useful.

CONTAINMENT_INVALID = 4 ///< This mode is invalid and should not be used
} ContainmentMode;

// 1s in the 4 bits defining the polyfill containment mode, 0s elsewhere
Expand Down
10 changes: 10 additions & 0 deletions src/h3lib/lib/bbox.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,16 @@ bool bboxEquals(const BBox *b1, const BBox *b2) {
b1->east == b2->east && b1->west == b2->west;
}

CellBoundary bboxToCellBoundary(const BBox *bbox) {
// Convert bbox to cell boundary, CCW vertex order
CellBoundary bboxBoundary = {.numVerts = 4,
.verts = {{bbox->north, bbox->east},
{bbox->north, bbox->west},
{bbox->south, bbox->west},
{bbox->south, bbox->east}}};
return bboxBoundary;
}

/**
* _hexRadiusKm returns the radius of a given hexagon in Km
*
Expand Down
Loading
Loading