spatial-framework-for-hadoop icon indicating copy to clipboard operation
spatial-framework-for-hadoop copied to clipboard

ST_Aggr_ConvexHull: fix convex hull of single geometry

Open wgtmac opened this issue 3 years ago • 7 comments

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

wgtmac avatar Nov 03 '21 08:11 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.

randallwhitman avatar Nov 03 '21 15:11 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.

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!

wgtmac avatar Nov 04 '21 02:11 wgtmac

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

climbage avatar Nov 04 '21 12:11 climbage

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
  • #41 & #163

randallwhitman avatar Nov 04 '21 15:11 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());
    }

randallwhitman avatar Dec 21 '21 00:12 randallwhitman

@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.

JordanMLee avatar Dec 21 '21 13:12 JordanMLee

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.

randallwhitman avatar Dec 21 '21 16:12 randallwhitman