concaveHull test fails with current GEOS main branch

[strk]

$ regress/run_test.pl regress/core/concave_hull -v
PATH is /home/strk/.local/bin:/home/strk/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games:/snap/bin:/home/strk/bin:/home/strk/go/bin:/opt/bitcoin/bin:/home/strk/bin:/home/strk/go/bin:/opt/bitcoin/bin
Checking for shp2pgsql ... found
Checking for pgsql2shp ... found
TMPDIR is /tmp/pgis_reg
Creating database 'postgis_reg' 
Loading unpackaged components from /home/src/postgis/postgis/regress/../regress/00-regress-install/share/contrib/postgis
Loading PostGIS into 'postgis_reg' 
PostgreSQL 13.5 (Ubuntu 13.5-0ubuntu0.21.04.1) on x86_64-pc-linux-gnu, compiled by gcc (Ubuntu 10.3.0-1ubuntu1) 10.3.0, 64-bit
  Postgis 3.3.0dev - (3.2.0-455-ga8de642a3) - 2022-02-02 15:08:04
  scripts 3.3.0dev 3.2.0-455-ga8de642a3
  GEOS: 3.11.0dev-CAPI-1.16.0
  PROJ: 7.2.1

Running tests

 regress/core/concave_hull .. failed (diff expected obtained: /tmp/pgis_reg/test_1_diff)
-----------------------------------------------------------------------------
--- regress/core/concave_hull_expected  2022-01-13 01:49:49.626748322 +0100
+++ /tmp/pgis_reg/test_1_out    2022-02-02 16:22:19.599616286 +0100
@@ -1,3 +1,3 @@
 ST_ConcaveHull MultiPolygon 0.95|t|t
-ST_ConcaveHull Lines 0.80|t|t
-ST_ConcaveHull Lines 0.80 holes|t|t
+ST_ConcaveHull Lines 0.80|f|t
+ST_ConcaveHull Lines 0.80 holes|f|f
-----------------------------------------------------------------------------
 uninstall .. ok (4831)

Run tests: 2
Failed: 1

[mdavis]

Occurs after ​this change.

Probably due to changing arg area_ratio float8 to param_pctconvex float, since this is incompatible with the underlying C signature.

[pramsey]

You are running a test that is meant to be run only with GEOS < 3.11 against the main of GEOS. If you check the Makefile you'll see that the concave hull tests only run for older GEOS. I don't entirely believe that testing the behaviour of our upstream carefully is super healthy, although I could add some tests along the lines of "function turns over and doesn't crash".

[pramsey]

I've added in some simple proof-of-function tests for the GEOS implementation at 6b8fec212

[strk]

Is it true that my signature change is incompatible with the underlying C signature ? I had the dubt, when changing float8 to float, when fixing #5078 — if this is the case I guess we'd want the C function to expect float (hoping it's not architecture dependent)

[strk]

So I confirm geos311 test runs successfully for me, but it looks like Martin is right about the incompatible change in my signature update as now runnign the concavehull test yelds:

 regress/core/concave_hull .. failed (diff expected obtained: /tmp/pgis_reg/test_1_diff)
-----------------------------------------------------------------------------
--- regress/core/concave_hull_expected  2022-01-13 01:49:49.626748322 +0100
+++ /tmp/pgis_reg/test_1_out    2022-02-02 22:47:16.733539749 +0100
@@ -1,3 +1,3 @@
-ST_ConcaveHull MultiPolygon 0.95|t|t
-ST_ConcaveHull Lines 0.80|t|t
-ST_ConcaveHull Lines 0.80 holes|t|t
+ERROR:  lwgeom_concavehull: GEOS Error: IllegalArgumentException: Edge length ratio must be in range [0,1]e
+ERROR:  lwgeom_concavehull: GEOS Error: IllegalArgumentException: Edge length ratio must be in range [0,1]e
+ERROR:  lwgeom_concavehull: GEOS Error: IllegalArgumentException: Edge length ratio must be in range [0,1]e
-----------------------------------------------------------------------------

[strk]

I'm reopening #5078 as well, as my change clearly broke the C version

[pramsey]

You are running a test (regress/core/concave_hull) that only should be run when GEOS < 3.11. So this is not a problem. The harmonized signature you did works correctly. I added a regress/core/geos311 test that (gently) exercises concave hull, so both cases (new and old GEOS) are exercised correctly.