Different Validation Behavior (is_simple) in Geos 3.8.x

Question

Different Validation Behavior (is_simple) in Geos 3.8.x

jdalt opened this issue 5 years ago · 11 comments

RGeo has different exception throwing behavior and returns different is_simple values when backed by different versions of the underlying geos library.

Specifically the RGeo::Geographic.simple_mercator_factory will throw an exception when parsing a self intersecting "hour glass" polygon in 3.8 while it will parse that polygon and return #is_simple? true in 3.7.

I think this is the root cause of this stackoverflow issue: https://stackoverflow.com/questions/58996305/getting-invalidgeometry-linearring-failed-ring-test-after-upgrading-rgeo-gem

Steps to reproduce

With geos-config --version 3.8.1:

RGeo::Geographic.simple_mercator_factory.parse_wkt('POLYGON((-93.572755591195 44.615834475162, -93.571098455347 44.613948476312, -93.571098455347 44.613948476313, -93.571098455347 44.613948476314, -93.571325365812 44.615739119667, -93.572627021193 44.613957229694, -93.572755591195 44.615834475162))')

This is a self-intersecting "hour glass" polygon:

Expected behavior

With geos-config --version 3.5.2, 3.6.4, or 3.7.4 (built from tarball with cmake):
This Polygon will be parsed. It returns true for #is_simple?.

Actual behavior

RGeo::Error::InvalidGeometry (LinearRing failed ring test)

System configuration

Ruby version:
ruby 2.5.7p206 (2019-10-01 revision 67816) [x86_64-darwin18]

OS:
Darwin jdalt-mbp.local 18.7.0 Darwin Kernel Version 18.7.0: Thu Jan 23 06:52:12 PST 2020; root:xnu-4903.278.25~1/RELEASE_X86_64 x86_64

Also replicated on travis-ci with Ubuntu xenial with a geos 3.8.1 built from source/tarball.

rgeo is version 2.1.1

Speculation

I think this is a difference in how is_simple is calculated in the CAPI. I believe all factories that use the CAPI to calculate is_simple are affected. I don't think the behavior of RGeo::Geographic.spherical_factory is affected.

Additional Thoughts

This is a breaking change, but for my own purposes I prefer the new behavior. I like it because it's more consistent with the implementation of RGeo::Geographic.spherical_factory which I converted my app to in order to avoid the issues outlined in #212. I tried #213 but it caused too much of a performance degradation to be viable for me.

However, I can't easily upgrade the underlying library in my production environment, so I'd like to get more insight into what's happening and what other mitigation options might be available to me. Ideally I'd be able to induce the 3.8 behavior in earlier versions of the library, or I'd be alerted to some other factory that's performant and consistent across versions.

I think others need to be aware that there are serious, breaking changes that exist in geos 3.8.x. This will become the default version of the library in Ubuntu focal. I imagine others who frequently hop back and forth between macOS (with the library installed and updated by Homebrew) and Linux ci and/or production environments will be similarly confused by this behavior.

Answer 1 · 2020-04-15T01:38:30.000Z

@jdalt Any thoughts on the scope of changes regarding is_simple?? It looks like for both self-intersecting and "pinched" polygons (e.g. #212 (comment)), is_simple? will return false with geos 3.8.x, and true in previous versions (I confirmed this with a pinched polygon). Do we think these are the only differences? And what types of polygons might cause is_simple? to return false with geos < 3.8.x?

Answer 2 · 2020-04-15T14:35:57.000Z

@jdalt Another thing I've noticed is that the same polygons for which is_simple? returns true for geos < 3.8.x and false for geos 3.8.x also cause .intersection() methods to return nil in 3.8.x, even when valid intersections exists, and .intersects? returns true. However, is_simple? is never called in that operation AFAICT, so looks like those polygons may be treated differently by other operations as well.

Answer 3 · 2020-04-17T17:42:50.000Z

@ykessler I don't have a good idea of the scope of the changes to is_simple.

I'm collecting stuff in my own test suite, but I'd need a good amount of time to deep dive and enumerate the problems.

I'm curious what it would look like to run the rgeo test suite on ci against 3.8.1 geos. I'd also love to see rgeo behave consistently across factories and across versions of geos for basic simple features violations. Right now changing factories is a pretty dangerous/large undertaking if you jump the line from the ruby factories to the CAPI backed ones. The CAPI factories can sometimes throw NoMethod errors when parsing wkt or decoding JSON.

I think the ci test matrix should probably be expanded to cover different versions of geos in addition to different versions of ruby.

Answer 4 · 2020-04-20T17:50:33.000Z

FWIW I ended up creating an "unpinch" function for RGeo::Geos::CAPIPolygonImpl, which simply creates a tiny separation between any non-contiguous points that have identical coordinates. Using this I can reliably bypass the LinearRing errors and related issues. It's a band-aid but works for now.

Answer 5 · 2020-04-22T18:55:30.000Z

This seems to be happening on earlier versions of GEOS too.
For me things are running smoothly on my development docker VM with Debian, but when I tried migrating some changes to my Staging Ubuntu server, everything went downhill. All libraries seemed to be at the same version (although from different repositories), except for PROJ4 which was version 5.2.0-1 on my dev machine, and 4.9 at staging. The only way I managed to upgrade PROJ4 was upgrading Ubuntu itself from bionic to eoan, unfortunately the problem persisted.

The WKT that is failing for me is: POLYGON ((-51.90477158413189 -23.38227371253858 0, -51.90478667415024 -23.38227088600153 0, -51.90451182169191 -23.38179368490476 0, -51.90471120333436 -23.38160910939104 0, -51.90503792850615 -23.38206462390997 0, -51.90477158413189 -23.38227371253858 0))
Which is both a "hour glass" and have very close coordinates.
I tested OP Polygon example and it failed too.

Some info about my Staging (failing) setup:

OS

$ lsb_release -a
No LSB modules are available.
Distributor ID:	Ubuntu
Description:	Ubuntu 19.10
Release:	19.10
Codename:	eoan

Ruby

$ ruby --version
ruby 2.7.0p0 (2019-12-25 revision 647ee6f091) [x86_64-linux]

Rails

$ rails --version
Rails 5.0.7.2

PostgreSQL 12

$ apt list postgresql* | grep installed
postgresql-12-postgis-3-scripts/eoan-pgdg,now 3.0.1+dfsg-2.pgdg19.10+1 all [installed,automatic]
postgresql-12-postgis-3/eoan-pgdg,now 3.0.1+dfsg-2.pgdg19.10+1 amd64 [installed]
postgresql-12/eoan-pgdg,now 12.2-2.pgdg19.10+1 amd64 [installed]

PostgreSQL 12 Postgis 3

SELECT PostGIS_full_version();
POSTGIS="3.0.1 ec2a9aa" [EXTENSION] PGSQL="120" GEOS="3.7.2-CAPI-1.11.2 b55d2125" PROJ="Rel. 5.2.0, September 15th, 2018" LIBXML="2.9.4" LIBJSON="0.13.1" LIBPROTOBUF="1.3.1" WAGYU="0.4.3 (Internal)"

GEOS

$ apt list libgeos*
Listing... Done
libgeos-3.7.2/eoan,now 3.7.2-1 amd64 [installed,automatic]
libgeos-c1v5/eoan,now 3.7.2-1 amd64 [installed]
libgeos-dev/eoan,now 3.7.2-1 amd64 [installed]

PROJ4

$ apt list libproj*
Listing... Done
libproj-dev/eoan,now 5.2.0-1 amd64 [installed]
libproj13/eoan,now 5.2.0-1 amd64 [installed,automatic]

Appreciate if anyone have suggestions on how to move on.

Answer 6 · 2020-04-28T05:29:26.000Z

To everyone that might come across this issue, downgrading ruby from 2.7.0 to 2.6.5 was enough solve this issue on my production environment.

So my working environment has the following setup:
Ubuntu 18.04
ruby 2.6.5
rails 5.0.7.2
postgres12 and postgis3
libgeos-3.7.1
libproj12

Good luck everyone!

Answer 7 · 2020-09-28T18:22:50.000Z

Encoding GeoJSON with the rgeo gem results in a linear ring test failure all of a sudden and I'm using the same data as before. I've been using ruby 2.6.5 this entire time, so @ricobonfim's solution won't work for me.

My similar specs are as follows...

Ubuntu 18.04 (via Heroku)
ruby 2.6.5
rails 6.0.3
postgres 12.4 and postgis 2.5
libproj12

In my development environment, I'm using postgres 11.2. Not sure if that makes a difference.

Is there a way for me to opt for lenient_assertions when encoding GeoJSON?

Answer 8 · 2020-09-28T21:20:12.000Z

@aleciavogel you can set a specific factory with the geo_factory option in RGeo::GeoJSON.deocde(string, geo_factory: factory).

You mention encoding in the issue, but as long as you have an RGeo geometry instance the encoder should accept it since that does not depend on a factory.

Answer 9 · 2020-11-11T23:34:51.000Z

This has been addressed in #228 and will eventually be released in version 3.0.0. If you'd like to use it sooner, it is in the v3-dev branch.

Answer 10 · 2020-11-22T23:40:22.000Z

EDIT: Still not working for me in v3-dev.

EDIT 2: In lib/rgeo/impl_helper/basic_line_string_methods.rb, replacing:

      def is_ring?
        is_closed? && is_simple?
      end

with:

      def is_ring?
        is_closed?
      end

fixed it for me. I don't know how this would affect edge cases, but it no longer throws an error for me when simply reading polygons from ActiveRecord. Perhaps the method is_simple? should be debugged, commented, and simplified.

This doesn't seem to have anything to do with the GEOS library in my case. I found the culprit in lib/rgeo/geographic/spherical_feature_methods.rb. What is all this gobbledegook? is_simple? returns false for perfectly valid polygons, resulting in a LinearRing failed ring test Error::InvalidGeometry error around line 187 of lib/rgeo/impl_helper/basic_line_string_methods.rb! All I am trying to do is call Shape.all[x].buffer in Rails console, where x is the index of the shape in my table that RGeo considers to be "invalid", but yet PostGIS says it is perfectly valid... all I am doing is READING data... why is it calling this validation on reading from ActiveRecord? Is this what is fixed in v3.0? Please clarify. Thanks.

      def is_simple?
        len = arcs.length
        return false if arcs.any?(&:degenerate?)
        return true if len == 1
        return arcs[0].s != arcs[1].e if len == 2
        arcs.each_with_index do |arc, index|
          nindex = index + 1
          nindex = nil if nindex == len
          return false if nindex && arc.contains_point?(arcs[nindex].e)
          pindex = index - 1
          pindex = nil if pindex < 0
          return false if pindex && arc.contains_point?(arcs[pindex].s)
          next unless nindex
          oindex = nindex + 1
          while oindex < len
            oarc = arcs[oindex]
            return false if !(index == 0 && oindex == len - 1 && arc.s == oarc.e) && arc.intersects_arc?(oarc)
            oindex += 1
          end
        end
        true
      end

Answer 11 · 2020-11-23T01:18:32.000Z

@tgrushka this issue is unrelated to the issue you're having. This is about GEOS having different definitions of a simple ring between versions, so this only affects CAPIFactories.

It's hard to tell without a specific example of your geometry, but I'm guessing this is related to #212. It's known that in small geometries, the spherical_factory loses accuracy due to floating-point errors.

Currently, most factories validate geometries on instantiation, which means that if the rings of the polygon intersect, is_simple? returns false, and the factory will raise an error. You can get around this by using the uses_lenient_assertions flag when creating your factory. Like so:

RGeo::Geographic.spherical_factory(uses_lenient_assertions: true)

This will disable validity checks.

Here's an example of how to configure the postgis adapter to use specific factories (#223 (comment)).

In v3 the default behavior might be changed so that errors are not automatically raised on instantiation, but uses_lenient_assertions is the best way to get around this currently.