Opened 5 years ago

Closed 5 years ago

#4407 closed defect (fixed)

Test failure on 32bit: tickets ... #3719

Reported by: myon Owned by: pramsey
Priority: blocker Milestone: PostGIS 3.0.0
Component: postgis Version: master
Keywords: Cc:

Description

Building PostGIS 3alpha1 fails on i386 (32-bit intel) on all Debian/Ubuntu releases:

16:25:14  tickets .. failed (diff expected obtained: /tmp/pgis_reg/test_98_diff)
16:25:14 -----------------------------------------------------------------------------
16:25:14 --- tickets_expected	2019-05-26 12:16:34.000000000 +0000
16:25:14 +++ /tmp/pgis_reg/test_98_out	2019-05-31 14:25:14.318429553 +0000
16:25:14 @@ -305,7 +305,8 @@
16:25:14  #3709|t
16:25:14  NOTICE:  Hole lies outside shell at or near point 25495368.044100001 6671726.9312000005
16:25:14  #3719a|f
16:25:14 -#3719b|t
16:25:14 +NOTICE:  Self-intersection
16:25:14 +#3719b|f
16:25:14  #3774|t
16:25:14  #1014a|POINT(0 0)
16:25:14  #1014a|POINT(0 0)
16:25:14 -----------------------------------------------------------------------------

Build log: https://pgdgbuild.dus.dg-i.net/job/postgis-binaries/212/architecture=i386,distribution=xenial/console

Change History (19)

comment:1 by myon, 5 years ago

Problem persists in 3alpha2.

comment:2 by strk, 5 years ago

Summary: Test failure on 32bit: tickets ... /tmp/pgis_reg/test_98_diffTest failure on 32bit: tickets ... #3719

comment:3 by strk, 5 years ago

It sounds GEOS related, what version of GEOS are you running ? The test is for issue #3719

comment:4 by myon, 5 years ago

3.7.1-1

comment:5 by myon, 5 years ago

Priority: mediumblocker

Setting as blocker because the package doesn't build cleanly on apt.postgresql.org.

comment:6 by pramsey, 5 years ago

Looking at the #3719 ticket, that ticket is all about linearization, which for close-to-tolerance cases can produce invalid output from valid (in a curvey sense) input. So this is probably something in linearization, which seems to be a constantly churning part of the code base (everyone has their own defn of what a "correct" linearization (or de-linearization) looks like).

That said, blame shows no changes since 8 months ago when dbaston took a run at #3719. We haven't had 32 bit issues all this time, right?

comment:7 by robe, 5 years ago

Well this is a new test that Darafei introduced to test FULL JOIN.

and you know how much of a master he is at putting together tests that break everything you thought was working :)/

comment:8 by robe, 5 years ago

Sorry got this confused with - #4433

which is also a 32-bit error but different from this one.

This particular one I am not seeing on windows 32-bit or freebsd 32-bit so must be a different issue

comment:9 by robe, 5 years ago

I should add on 32-bit testing with GEOS 3.8 head and freebsd 32-bit is at GEOS: 3.6.3-CAPI-1.10.3 80c13047

So could still be an issue with GEOS 3.7.1 (and 32-bit). I'll try to do a 32-bit test on windows to try to replicate.

comment:10 by robe, 5 years ago

one more thing I just realized — myon is testing against pg12. My 32-bits are testing on PostgreSQL 10 (I don't build 32-bit on 11 and above since EDB stopped shipping 32-bit windows binaries so no audience for them).

Could this issue be related to the float changes in PG12?

comment:11 by Algunenano, 5 years ago

I can reproduce this with Debian buster 32 bits, using Postgresql 11.4 and GEOS 3.7.1.

comment:12 by sebastic, 5 years ago

Also fails to build on armel, armhf & x32:

https://buildd.debian.org/status/package.php?p=postgis&suite=experimental

Click the links in the Status column for the full buildlog.

comment:13 by Algunenano, 5 years ago

armel: Failing because of #4433 (already fixed, not included in last RC) armhf: Same as armel i386: Fails because of #4433 and this ticket. x32: Same as armel

So armel, armhf and x32 should be already ok with trunk, but i386 is still affected by this issue:

 #3719a|f
-#3719b|t
+NOTICE:  Self-intersection

comment:14 by Algunenano, 5 years ago

Comparing 64 vs 32 bits:

Select postgis_geos_noop('CURVEPOLYGON(COMPOUNDCURVE(CIRCULARSTRING(1.0441 2.9312,1.3959388 2.93601515,1.7478 2.9333), (1.7478 2.9333,1.0441 2.9312)))'::geometry);

64b

01030000000100000004000000BF0E9C33A2B4F03F174850FC18730740CE84A5E2C355F63F70C9CA82F57C0740492EFF21FDF6FB3FBADA8AFD65770740BF0E9C33A2B4F03F174850FC18730740

32b

01030000000100000005000000BF0E9C33A2B4F03F174850FC18730740CE84A5E2C355F63F6DC9CA82F57C07404E2EFF21FDF6FB3FB6DA8AFD65770740492EFF21FDF6FB3FBADA8AFD65770740BF0E9C33A2B4F03F174850FC18730740
Select ST_AsText(postgis_geos_noop('CURVEPOLYGON(COMPOUNDCURVE(CIRCULARSTRING(1.0441 2.9312,1.3959388 2.93601515,1.7478 2.9333), (1.7478 2.9333,1.0441 2.9312)))'::geometry));

64b:

POLYGON((1.0441 2.9312,1.39593876394092 2.93601514989239,1.7478 2.9333,1.0441 2.9312))

32b

POLYGON((1.0441 2.9312,1.39593876394092 2.93601514989239,1.7478 2.9333,1.7478 2.9333,1.0441 2.9312))

The 32b version is producing an extra point, which makes the polygon invalid.

comment:15 by Algunenano, 5 years ago

I've narrowed it down to lwgeom_stroke:

CREATE OR REPLACE FUNCTION __custom_segmentize(geom geometry, integer DEFAULT 32)
	RETURNS geometry
	AS '$libdir/postgis-3', 'LWGEOM_curve_segmentize'
	LANGUAGE 'c' IMMUTABLE STRICT PARALLEL SAFE;
	

Select ST_AsText(__custom_segmentize('CURVEPOLYGON(COMPOUNDCURVE(CIRCULARSTRING(1.0441 2.9312,1.3959388 2.93601515,1.7478 2.9333), (1.7478 2.9333,1.0441 2.9312)))'::geometry));

32b:

POLYGON((1.0441 2.9312,1.39593876394092 2.93601514989239,1.7478 2.9333,1.7478 2.9333,1.0441 2.9312))

64b

POLYGON((1.0441 2.9312,1.39593876394092 2.93601514989239,1.7478 2.9333,1.0441 2.9312))

BTW, I understand that LWGEOM_curve_segmentize should be replaced by the default deprecated C function as it's not used by the SQL code anymore.

comment:16 by Algunenano, 5 years ago

It appears than a tolerance of 0.0 to remove repeated points in lwcompound_linearize is too small in 32b systems.

Using DBL_EPSILON also doesn't work (kind of expected), but FLT_EPSILON works. I'm reviewing lwarc_linearize to see if we can do anything there to improve the precision of 32 bit systems; if not I'll just increase the tolerance to FLT_EPSILON.

comment:18 by Algunenano, 5 years ago

I've looked a bit deeper into it and the issue seems to be the workaround to avoid collapsing arc introduced in r16998.

In this case the angle described by the arc is 0.0428024008, and the increment calculated by the tolerance passed is 0.0490873852.

The workaround changes the increment to angle / 2, which is 0.0214012004 but at this point the floating point precision under 32b systems (probably using 64 doubles) is not enough and that means it adds an extra point:

1.04410000000000002807 2.93120000000000002771 <<< Start of segment
1.39593876394092175630 2.93601514989238987496 << Extra point in segment
1.74780000000000113047 2.93329999999999824212 << Extra point added in 32b (due to imprecision)
1.74780000000000002025 2.93330000000000001847 << End of segment

For this case where the tolerance is bigger than the arc angle and the 3 points are not colinear, I returning the input as the linearization is both correct and avoids floating point precision issues. What do you think @dbaston?

comment:19 by algunenano, 5 years ago

Resolution: fixed
Status: newclosed

In 17677:

lwarc_linearize: Iterate over the number of segments

Closes https://github.com/postgis/postgis/pull/458
Closes #4407
References #3719

Note: See TracTickets for help on using tickets.