Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Different Validation Behavior (is_simple) in Geos 3.8.x #218

Closed
jdalt opened this issue Apr 13, 2020 · 11 comments · Fixed by #228
Closed

Different Validation Behavior (is_simple) in Geos 3.8.x #218

jdalt opened this issue Apr 13, 2020 · 11 comments · Fixed by #228

Comments

@jdalt
Copy link

jdalt commented Apr 13, 2020

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:
geojson_io

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.

@ykessler
Copy link

ykessler commented Apr 15, 2020

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

@ykessler
Copy link

ykessler commented Apr 15, 2020

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

@jdalt
Copy link
Author

jdalt commented Apr 17, 2020

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

@ykessler
Copy link

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.

@ricobonfim
Copy link

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.

@ricobonfim
Copy link

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!

@aleciavogel
Copy link

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?

@keithdoggett
Copy link
Member

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

@BuonOmo BuonOmo added this to High priority in Prioritized issues Oct 27, 2020
@BuonOmo BuonOmo moved this from High priority to Low priority in Prioritized issues Oct 27, 2020
@BuonOmo BuonOmo moved this from Low priority to High priority in Prioritized issues Oct 27, 2020
@keithdoggett keithdoggett self-assigned this Oct 27, 2020
@keithdoggett keithdoggett added this to High priority in Prioritized Issues Oct 28, 2020
keithdoggett added a commit that referenced this issue Oct 30, 2020
@keithdoggett
Copy link
Member

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.

Prioritized Issues automation moved this from High priority to Closed Nov 11, 2020
@ghost
Copy link

ghost commented Nov 22, 2020

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

ghost pushed a commit to dra11y/rgeo that referenced this issue Nov 22, 2020
@keithdoggett
Copy link
Member

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

keithdoggett added a commit that referenced this issue Apr 9, 2021
BuonOmo pushed a commit that referenced this issue Apr 16, 2021
BuonOmo pushed a commit that referenced this issue Jan 13, 2022
BuonOmo pushed a commit that referenced this issue Jan 13, 2022
BuonOmo pushed a commit that referenced this issue Mar 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Prioritized issues
High priority
Development

Successfully merging a pull request may close this issue.

5 participants