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

Rename h3SetToLinkedGeo to cellsToLinkedMultiPolygon #571

Merged
merged 1 commit into from
Feb 1, 2022
Merged

Rename h3SetToLinkedGeo to cellsToLinkedMultiPolygon #571

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 5 additions & 5 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ set(OTHER_SOURCE_FILES
src/apps/testapps/testLinkedGeo.c
src/apps/testapps/mkRandGeo.c
src/apps/testapps/testH3Api.c
src/apps/testapps/testH3SetToLinkedGeo.c
src/apps/testapps/testCellsToLinkedMultiPolygon.c
src/apps/testapps/testH3ToLocalIj.c
src/apps/testapps/testH3ToLocalIjExhaustive.c
src/apps/testapps/testGridDistance.c
Expand All @@ -227,7 +227,7 @@ set(OTHER_SOURCE_FILES
src/apps/fuzzers/fuzzerLatLngToCell.c
src/apps/fuzzers/fuzzerCellToLatLng.c
src/apps/fuzzers/fuzzerGridDisk.c
src/apps/fuzzers/fuzzerH3SetToLinkedGeo.c
src/apps/fuzzers/fuzzerCellsToLinkedMultiPolygon.c
src/apps/fuzzers/fuzzerDistances.c
src/apps/fuzzers/fuzzerCellArea.c
src/apps/fuzzers/fuzzerExactEdgeLength.c
Expand All @@ -243,7 +243,7 @@ set(OTHER_SOURCE_FILES
src/apps/fuzzers/fuzzerPolygonToCellsNoHoles.c
src/apps/benchmarks/benchmarkPolygonToCells.c
src/apps/benchmarks/benchmarkPolygon.c
src/apps/benchmarks/benchmarkH3SetToLinkedGeo.c
src/apps/benchmarks/benchmarkCellsToLinkedMultiPolygon.c
src/apps/benchmarks/benchmarkGridDiskCells.c
src/apps/benchmarks/benchmarkGridPathCells.c
src/apps/benchmarks/benchmarkDirectedEdge.c
Expand Down Expand Up @@ -469,7 +469,7 @@ if(BUILD_FUZZERS)
add_h3_fuzzer(fuzzerLatLngToCell src/apps/fuzzers/fuzzerLatLngToCell.c)
add_h3_fuzzer(fuzzerCellToLatLng src/apps/fuzzers/fuzzerCellToLatLng.c)
add_h3_fuzzer(fuzzerGridDisk src/apps/fuzzers/fuzzerGridDisk.c)
add_h3_fuzzer(fuzzerH3SetToLinkedGeo src/apps/fuzzers/fuzzerH3SetToLinkedGeo.c)
add_h3_fuzzer(fuzzerCellsToLinkedMultiPolygon src/apps/fuzzers/fuzzerCellsToLinkedMultiPolygon.c)
add_h3_fuzzer(fuzzerDistances src/apps/fuzzers/fuzzerDistances.c)
add_h3_fuzzer(fuzzerCellArea src/apps/fuzzers/fuzzerCellArea.c)
add_h3_fuzzer(fuzzerExactEdgeLength src/apps/fuzzers/fuzzerExactEdgeLength.c)
Expand Down Expand Up @@ -501,7 +501,7 @@ if(BUILD_BENCHMARKS)
add_h3_benchmark(benchmarkDirectedEdge src/apps/benchmarks/benchmarkDirectedEdge.c)
add_h3_benchmark(benchmarkVertex src/apps/benchmarks/benchmarkVertex.c)
add_h3_benchmark(benchmarkIsValidCell src/apps/benchmarks/benchmarkIsValidCell.c)
add_h3_benchmark(benchmarkH3SetToLinkedGeo src/apps/benchmarks/benchmarkH3SetToLinkedGeo.c)
add_h3_benchmark(benchmarkCellsToLinkedMultiPolygon src/apps/benchmarks/benchmarkCellsToLinkedMultiPolygon.c)
add_h3_benchmark(benchmarkPolygonToCells src/apps/benchmarks/benchmarkPolygonToCells.c)
if(ENABLE_REQUIRES_ALL_SYMBOLS)
add_h3_benchmark(benchmarkPolygon src/apps/benchmarks/benchmarkPolygon.c)
Expand Down
2 changes: 1 addition & 1 deletion CMakeTests.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ if(H3_IS_ROOT_PROJECT AND BUILD_TESTING)
add_h3_test(testCellToChildrenSize src/apps/testapps/testCellToChildrenSize.c)
add_h3_test(testH3Index src/apps/testapps/testH3Index.c)
add_h3_test(testH3Api src/apps/testapps/testH3Api.c)
add_h3_test(testH3SetToLinkedGeo src/apps/testapps/testH3SetToLinkedGeo.c)
add_h3_test(testCellsToLinkedMultiPolygon src/apps/testapps/testCellsToLinkedMultiPolygon.c)
add_h3_test(testH3SetToVertexGraph src/apps/testapps/testH3SetToVertexGraph.c)
add_h3_test(testLinkedGeo src/apps/testapps/testLinkedGeo.c)
add_h3_test(testPolygonToCells src/apps/testapps/testPolygonToCells.c)
Expand Down
16 changes: 9 additions & 7 deletions dev-docs/RFCs/v4.0.0/names_for_concepts_types_functions.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,10 +271,12 @@ discussion requiring benchmarking, so we will defer that to a

### Functions

| Current name | Proposed name | Notes |
|-----------------------------------|--------------------------|------------------------|
| `h3ToGeoBoundary` | `cellToBoundary` | returns `CellBoundary` |
| *DNE* | `cellToLoop` | returns `GeoLoop` |
| *DNE* | `loopToBoundary` | |
| *DNE* | `boundaryToLoop` | |
| `getH3UnidirectionalEdgeBoundary` | `directedEdgeToBoundary` | returns `CellBoundary` |
| 3.x name | Proposed name | Notes |
|-----------------------------------|-----------------------------|----------------------------|
| `h3ToGeoBoundary` | `cellToBoundary` | returns `CellBoundary` |
| *DNE* | `cellToLoop` | returns `GeoLoop` |
| *DNE* | `loopToBoundary` | |
| *DNE* | `boundaryToLoop` | |
| `getH3UnidirectionalEdgeBoundary` | `directedEdgeToBoundary` | returns `CellBoundary` |
| `h3SetToLinkedGeo` | `cellsToLinkedMultiPolygon` | returns `LinkedGeoPolygon` |
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we also rename LinkedGeoPolygon? This struct is part of the external API, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Per line 269 it looks like the rename should be LinkedGeoMultiPolygon? It's part of the external C API in h3api.h, although it is not needed for binary compatability

Copy link
Collaborator

@nrabinowitz nrabinowitz Jan 31, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

That shouldn't be necessary. The linked structures have a next pointer. So instead of a MultiPolygon with Polygon children, you have a Polygon that can optionally define a pointer to a Polygon sibling.

Maybe the rename isn't necessary - I thought we were dropping the Geo in GeoPolygon, so I was suggesting this should follow suit. But if not, this is fine - though I guess we could update LinkedGeoCoord to LinkedLatLng for consistency.

| `h3SetToToMultiPolygon` | `cellsToMultiPolygon` | bindings only |
19 changes: 10 additions & 9 deletions ...ps/benchmarks/benchmarkH3SetToLinkedGeo.c → ...arks/benchmarkCellsToLinkedMultiPolygon.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,19 +43,20 @@ int nestedDonutsCount = 24;
BEGIN_BENCHMARKS();
LinkedGeoPolygon polygon;

BENCHMARK(h3SetToLinkedGeoRing2, 10000, {
H3_EXPORT(h3SetToLinkedGeo)(ring2, ring2Count, &polygon);
H3_EXPORT(destroyLinkedPolygon)(&polygon);
BENCHMARK(cellsToLinkedMultiPolygonRing2, 10000, {
H3_EXPORT(cellsToLinkedMultiPolygon)(ring2, ring2Count, &polygon);
H3_EXPORT(destroyLinkedMultiPolygon)(&polygon);
});

BENCHMARK(h3SetToLinkedGeoDonut, 10000, {
H3_EXPORT(h3SetToLinkedGeo)(donut, donutCount, &polygon);
H3_EXPORT(destroyLinkedPolygon)(&polygon);
BENCHMARK(cellsToLinkedMultiPolygonDonut, 10000, {
H3_EXPORT(cellsToLinkedMultiPolygon)(donut, donutCount, &polygon);
H3_EXPORT(destroyLinkedMultiPolygon)(&polygon);
});

BENCHMARK(h3SetToLinkedGeoNestedDonuts, 10000, {
H3_EXPORT(h3SetToLinkedGeo)(nestedDonuts, nestedDonutsCount, &polygon);
H3_EXPORT(destroyLinkedPolygon)(&polygon);
BENCHMARK(cellsToLinkedMultiPolygonNestedDonuts, 10000, {
H3_EXPORT(cellsToLinkedMultiPolygon)
(nestedDonuts, nestedDonutsCount, &polygon);
H3_EXPORT(destroyLinkedMultiPolygon)(&polygon);
});

END_BENCHMARKS();
6 changes: 3 additions & 3 deletions src/apps/fuzzers/fuzzerH3SetToLinkedGeo.c → ...fuzzers/fuzzerCellsToLinkedMultiPolygon.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/
/** @file
* @brief Fuzzer program for h3SetToLinkedGeo
* @brief Fuzzer program for cellsToLinkedMultiPolygon
*/

#include "aflHarness.h"
Expand All @@ -26,10 +26,10 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
int sz = size / sizeof(H3Index);

LinkedGeoPolygon polygon;
H3Error err = H3_EXPORT(h3SetToLinkedGeo)(h3Set, sz, &polygon);
H3Error err = H3_EXPORT(cellsToLinkedMultiPolygon)(h3Set, sz, &polygon);

if (!err) {
H3_EXPORT(destroyLinkedPolygon)(&polygon);
H3_EXPORT(destroyLinkedMultiPolygon)(&polygon);
}
return 0;
}
Expand Down
70 changes: 41 additions & 29 deletions src/apps/testapps/testH3SetToLinkedGeo.c → .../testapps/testCellsToLinkedMultiPolygon.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,38 +20,40 @@
#include "test.h"
#include "utility.h"

SUITE(h3SetToLinkedGeo) {
SUITE(cellsToLinkedMultiPolygon) {
TEST(empty) {
LinkedGeoPolygon polygon;

t_assertSuccess(H3_EXPORT(h3SetToLinkedGeo)(NULL, 0, &polygon));
t_assertSuccess(
H3_EXPORT(cellsToLinkedMultiPolygon)(NULL, 0, &polygon));

t_assert(countLinkedLoops(&polygon) == 0, "No loops added to polygon");

H3_EXPORT(destroyLinkedPolygon)(&polygon);
H3_EXPORT(destroyLinkedMultiPolygon)(&polygon);
}

TEST(singleHex) {
LinkedGeoPolygon polygon;
H3Index set[] = {0x890dab6220bffff};
int numHexes = ARRAY_SIZE(set);

t_assertSuccess(H3_EXPORT(h3SetToLinkedGeo)(set, numHexes, &polygon));
t_assertSuccess(
H3_EXPORT(cellsToLinkedMultiPolygon)(set, numHexes, &polygon));

t_assert(countLinkedLoops(&polygon) == 1, "1 loop added to polygon");
t_assert(countLinkedCoords(polygon.first) == 6,
"6 coords added to loop");

H3_EXPORT(destroyLinkedPolygon)(&polygon);
H3_EXPORT(destroyLinkedMultiPolygon)(&polygon);
}

TEST(invalid) {
LinkedGeoPolygon polygon;
H3Index set[] = {0xfffffffffffffff};
int numHexes = ARRAY_SIZE(set);

t_assert(H3_EXPORT(h3SetToLinkedGeo)(set, numHexes, &polygon) ==
E_CELL_INVALID,
t_assert(H3_EXPORT(cellsToLinkedMultiPolygon)(
set, numHexes, &polygon) == E_CELL_INVALID,
"Invalid set fails");
}

Expand All @@ -60,13 +62,14 @@ SUITE(h3SetToLinkedGeo) {
H3Index set[] = {0x8928308291bffff, 0x89283082957ffff};
int numHexes = ARRAY_SIZE(set);

t_assertSuccess(H3_EXPORT(h3SetToLinkedGeo)(set, numHexes, &polygon));
t_assertSuccess(
H3_EXPORT(cellsToLinkedMultiPolygon)(set, numHexes, &polygon));

t_assert(countLinkedLoops(&polygon) == 1, "1 loop added to polygon");
t_assert(countLinkedCoords(polygon.first) == 10,
"All coords added to loop except 2 shared");

H3_EXPORT(destroyLinkedPolygon)(&polygon);
H3_EXPORT(destroyLinkedMultiPolygon)(&polygon);
}

// TODO: This test asserts incorrect behavior - we should be creating
Expand All @@ -77,7 +80,8 @@ SUITE(h3SetToLinkedGeo) {
H3Index set[] = {0x8928308291bffff, 0x89283082943ffff};
int numHexes = ARRAY_SIZE(set);

t_assertSuccess(H3_EXPORT(h3SetToLinkedGeo)(set, numHexes, &polygon));
t_assertSuccess(
H3_EXPORT(cellsToLinkedMultiPolygon)(set, numHexes, &polygon));

t_assert(countLinkedPolygons(&polygon) == 2, "2 polygons added");
t_assert(countLinkedLoops(&polygon) == 1,
Expand All @@ -89,7 +93,7 @@ SUITE(h3SetToLinkedGeo) {
t_assert(countLinkedCoords(polygon.next->first) == 6,
"All coords for one hex added to second polygon");

H3_EXPORT(destroyLinkedPolygon)(&polygon);
H3_EXPORT(destroyLinkedMultiPolygon)(&polygon);
}

TEST(contiguous3) {
Expand All @@ -98,13 +102,14 @@ SUITE(h3SetToLinkedGeo) {
0x8928308289bffff};
int numHexes = ARRAY_SIZE(set);

t_assertSuccess(H3_EXPORT(h3SetToLinkedGeo)(set, numHexes, &polygon));
t_assertSuccess(
H3_EXPORT(cellsToLinkedMultiPolygon)(set, numHexes, &polygon));

t_assert(countLinkedLoops(&polygon) == 1, "1 loop added to polygon");
t_assert(countLinkedCoords(polygon.first) == 12,
"All coords added to loop except 6 shared");

H3_EXPORT(destroyLinkedPolygon)(&polygon);
H3_EXPORT(destroyLinkedMultiPolygon)(&polygon);
}

TEST(hole) {
Expand All @@ -114,29 +119,30 @@ SUITE(h3SetToLinkedGeo) {
0x8928308288fffff, 0x89283082883ffff};
int numHexes = ARRAY_SIZE(set);

H3_EXPORT(h3SetToLinkedGeo)(set, numHexes, &polygon);
H3_EXPORT(cellsToLinkedMultiPolygon)(set, numHexes, &polygon);

t_assert(countLinkedLoops(&polygon) == 2, "2 loops added to polygon");
t_assert(countLinkedCoords(polygon.first) == 6 * 3,
"All outer coords added to first loop");
t_assert(countLinkedCoords(polygon.first->next) == 6,
"All inner coords added to second loop");

H3_EXPORT(destroyLinkedPolygon)(&polygon);
H3_EXPORT(destroyLinkedMultiPolygon)(&polygon);
}

TEST(pentagon) {
LinkedGeoPolygon polygon;
H3Index set[] = {0x851c0003fffffff};
int numHexes = ARRAY_SIZE(set);

t_assertSuccess(H3_EXPORT(h3SetToLinkedGeo)(set, numHexes, &polygon));
t_assertSuccess(
H3_EXPORT(cellsToLinkedMultiPolygon)(set, numHexes, &polygon));

t_assert(countLinkedLoops(&polygon) == 1, "1 loop added to polygon");
t_assert(countLinkedCoords(polygon.first) == 10,
"10 coords (distorted pentagon) added to loop");

H3_EXPORT(destroyLinkedPolygon)(&polygon);
H3_EXPORT(destroyLinkedMultiPolygon)(&polygon);
}

TEST(twoRing) {
Expand All @@ -152,13 +158,14 @@ SUITE(h3SetToLinkedGeo) {
0x89300628303ffff};
int numHexes = ARRAY_SIZE(set);

t_assertSuccess(H3_EXPORT(h3SetToLinkedGeo)(set, numHexes, &polygon));
t_assertSuccess(
H3_EXPORT(cellsToLinkedMultiPolygon)(set, numHexes, &polygon));

t_assert(countLinkedLoops(&polygon) == 1, "1 loop added to polygon");
t_assert(countLinkedCoords(polygon.first) == (6 * (2 * 2 + 1)),
"Expected number of coords added to loop");

H3_EXPORT(destroyLinkedPolygon)(&polygon);
H3_EXPORT(destroyLinkedMultiPolygon)(&polygon);
}

TEST(twoRingUnordered) {
Expand All @@ -174,13 +181,14 @@ SUITE(h3SetToLinkedGeo) {
0x893006283c7ffff};
int numHexes = ARRAY_SIZE(set);

t_assertSuccess(H3_EXPORT(h3SetToLinkedGeo)(set, numHexes, &polygon));
t_assertSuccess(
H3_EXPORT(cellsToLinkedMultiPolygon)(set, numHexes, &polygon));

t_assert(countLinkedLoops(&polygon) == 1, "1 loop added to polygon");
t_assert(countLinkedCoords(polygon.first) == (6 * (2 * 2 + 1)),
"Expected number of coords added to loop");

H3_EXPORT(destroyLinkedPolygon)(&polygon);
H3_EXPORT(destroyLinkedMultiPolygon)(&polygon);
}

TEST(nestedDonut) {
Expand All @@ -197,7 +205,8 @@ SUITE(h3SetToLinkedGeo) {
0x892830828b3ffff, 0x89283082887ffff, 0x89283082883ffff};
int numHexes = ARRAY_SIZE(set);

t_assertSuccess(H3_EXPORT(h3SetToLinkedGeo)(set, numHexes, &polygon));
t_assertSuccess(
H3_EXPORT(cellsToLinkedMultiPolygon)(set, numHexes, &polygon));

// Note that the polygon order here is arbitrary, making this test
// somewhat brittle, but it's difficult to assert correctness otherwise
Expand All @@ -215,7 +224,7 @@ SUITE(h3SetToLinkedGeo) {
t_assert(countLinkedCoords(polygon.next->first->next) == 6,
"Got expected inner loop");

H3_EXPORT(destroyLinkedPolygon)(&polygon);
H3_EXPORT(destroyLinkedMultiPolygon)(&polygon);
}

TEST(nestedDonutTransmeridian) {
Expand All @@ -232,7 +241,8 @@ SUITE(h3SetToLinkedGeo) {
0x897eb572287ffff, 0x897eb572283ffff, 0x897eb57229bffff};
int numHexes = ARRAY_SIZE(set);

t_assertSuccess(H3_EXPORT(h3SetToLinkedGeo)(set, numHexes, &polygon));
t_assertSuccess(
H3_EXPORT(cellsToLinkedMultiPolygon)(set, numHexes, &polygon));

// Note that the polygon order here is arbitrary, making this test
// somewhat brittle, but it's difficult to assert correctness otherwise
Expand All @@ -250,28 +260,30 @@ SUITE(h3SetToLinkedGeo) {
t_assert(countLinkedCoords(polygon.next->first->next) == 30,
"Got expected big inner loop");

H3_EXPORT(destroyLinkedPolygon)(&polygon);
H3_EXPORT(destroyLinkedMultiPolygon)(&polygon);
}

TEST(contiguous2distorted) {
LinkedGeoPolygon polygon;
H3Index set[] = {0x894cc5365afffff, 0x894cc536537ffff};
int numHexes = ARRAY_SIZE(set);

t_assertSuccess(H3_EXPORT(h3SetToLinkedGeo)(set, numHexes, &polygon));
t_assertSuccess(
H3_EXPORT(cellsToLinkedMultiPolygon)(set, numHexes, &polygon));

t_assert(countLinkedLoops(&polygon) == 1, "1 loop added to polygon");
t_assert(countLinkedCoords(polygon.first) == 12,
"All coords added to loop except 2 shared");

H3_EXPORT(destroyLinkedPolygon)(&polygon);
H3_EXPORT(destroyLinkedMultiPolygon)(&polygon);
}

TEST(negativeHashedCoordinates) {
LinkedGeoPolygon polygon;
H3Index set[] = {0x88ad36c547fffff, 0x88ad36c467fffff};
int numHexes = ARRAY_SIZE(set);
t_assertSuccess(H3_EXPORT(h3SetToLinkedGeo)(set, numHexes, &polygon));
t_assertSuccess(
H3_EXPORT(cellsToLinkedMultiPolygon)(set, numHexes, &polygon));

t_assert(countLinkedPolygons(&polygon) == 2, "2 polygons added");
t_assert(countLinkedLoops(&polygon) == 1,
Expand All @@ -283,6 +295,6 @@ SUITE(h3SetToLinkedGeo) {
t_assert(countLinkedCoords(polygon.next->first) == 6,
"All coords for one hex added to second polygon");

H3_EXPORT(destroyLinkedPolygon)(&polygon);
H3_EXPORT(destroyLinkedMultiPolygon)(&polygon);
}
}
2 changes: 1 addition & 1 deletion src/apps/testapps/testLinkedGeo.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ SUITE(linkedGeo) {

t_assert(countLinkedPolygons(polygon) == 2, "Polygon count correct");

H3_EXPORT(destroyLinkedPolygon)(polygon);
H3_EXPORT(destroyLinkedMultiPolygon)(polygon);
free(polygon);
}
}
Loading