ST_Aggr_ConvexHull: fix convex hull of single geometry

[wgtmac]

The convex hull of a single geometry should be itself. However, the result without this patch returns an empty multi polygon.

[randallwhitman]

Hi @wgtmac - thanks for contributing to Spatial Framework for Hadoop. However -

The convex hull of a single geometry should be itself.

No, that's not correct in general. The straightforward counterexample is a concave polygon; its convex hull is a convex polygon, not itself.

[wgtmac]

Hi @wgtmac - thanks for contributing to Spatial Framework for Hadoop. However -

The convex hull of a single geometry should be itself.

No, that's not correct in general. The straightforward counterexample is a concave polygon; its convex hull is a convex polygon, not itself.

Thanks @randallwhitman for the explanation. I am not a geospatial expert. The problem on our side arises when we tried to aggregate multiple points to get the convex hull based on the Hive UDAF. If the partial aggregation has one single point as the input, the point is lost due to the empty multi-polygon output. Could you help fix this issue? Thanks!

[climbage]

This makes sense. The aggregator is assuming a convex hull can be created from a partial result.

Thinking out loud about cases where that may not hold true:

• single point (this issue)
• stacked [coincident] points
• colinear points
• straight lines

[randallwhitman]

It will be useful if we can put together a unit test for at least the case of single geometry. We used to have automatic tests of most if not all the UDF/UDAF, using a framework that is no longer available. Have started [re-]writing unit tests, but still incomplete. I might later update this comment with some links.

• https://github.com/Esri/spatial-framework-for-hadoop/tree/master/hive/src/test/java/com/esri/hadoop/hive
• https://github.com/Esri/spatial-framework-for-hadoop/tree/master/hive/test
• Remove dependency on Hive PDK as it is not supported in Hive 0.11+ #41 & Test with Hive Mini-Cluster #163

[randallwhitman]

Please comment on this draft of a test: @wgtmac @climbage @JordanMLee

    public void TestStAggrConvexHullPoint() throws HiveException {
        ST_Point stPt = new ST_Point();
        BytesWritable bwGeom = stPt.evaluate(new DoubleWritable(1.2), new DoubleWritable(3.4));
        ST_Aggr_ConvexHull.AggrConvexHullBinaryEvaluator convexHullEval = new ST_Aggr_ConvexHull.AggrConvexHullBinaryEvaluator();
        ST_Aggr_ConvexHull.AggrConvexHullBinaryEvaluator other1 = new ST_Aggr_ConvexHull.AggrConvexHullBinaryEvaluator();
        ST_Aggr_ConvexHull.AggrConvexHullBinaryEvaluator other2 = new ST_Aggr_ConvexHull.AggrConvexHullBinaryEvaluator();
        convexHullEval.init();
        convexHullEval.iterate(bwGeom);
        BytesWritable partial = convexHullEval.terminatePartial();
        other1.merge(partial);
        convexHullEval.merge(other2.terminatePartial());
        BytesWritable result1 = other1.terminate();
        BytesWritable result2 = convexHullEval.terminate();
        OGCGeometry geom1 = GeometryUtils.geometryFromEsriShape(result1);
        OGCGeometry geom2 = GeometryUtils.geometryFromEsriShape(result2);
        assertFalse(geom1.isEmpty());
        assertFalse(geom2.isEmpty());
    }

[JordanMLee]

@randallwhitman the test above looks alright to me. When computing the aggregate convex hull, we shouldn't be losing geometries when computing the partial result or when merging. Both geom1 and geom2 should be degenerate polygons, since ST_Aggr_ConvexHull will always produce a polygon.

[randallwhitman]

When computing the aggregate convex hull, we shouldn't be losing geometries when computing the partial result or when merging. Both geom1 and geom2 should be [non-empty] degenerate polygons, since ST_Aggr_ConvexHull will always produce a polygon.

Thanks. In fact that test is failing for me; the result is empty. Which leaves us the question, which part of the code results empty rather than non-empty-degenerate.