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

SDA_spatialQuery() can fail because of 'area_ac' #343

Closed
dschlaep opened this issue Mar 22, 2024 · 5 comments · Fixed by #344
Closed

SDA_spatialQuery() can fail because of 'area_ac' #343

dschlaep opened this issue Mar 22, 2024 · 5 comments · Fixed by #344

Comments

@dschlaep
Copy link
Contributor

Requesting spatial intersections with map unit polygons via SDA_spatialQuery() can fail for some areas. In my case, this appears to be caused by the calculation of 'area_ac' that is added to the query by .SDA_geometrySelector() -- and not by the spatial query itself.

My example below creates a small polygon in eastern Washington for which I attempt to query the spatial intersections with SSURGO map unit polygons. This produces an error. However, it successfully returns the spatial intersections if I manually remove the request for 'area_ac' from the query.

I don't know why SDA fails with the original query. Maybe there is a real fix to this, SDA_query(q) in the example below suggests "Use MakeValid to convert the instance to a valid instance" -- not sure what that means... One option that avoids this error may be to add a new argument to SDA_spatialQuery(), which then is passed on to .SDA_geometrySelector(), that allows a user to remove 'area_ac' from the query? Thanks!

library("soilDB")
print(packageVersion("soilDB"))
#> [1] '2.8.1'

# Polygon in eastern Washington
poly <- "POLYGON ((-118.7458 47.2125, -118.7458 47.25417, -118.7875 47.25417, -118.7875 47.2125, -118.7458 47.2125))"
g <- sf::st_as_sfc(poly, crs = 4326) |> sf::st_make_valid()

# Error: Bad Request (HTTP 400).
SDA_spatialQuery(geom = g, what = "mupolygon", db = "SSURGO", geomIntersection = TRUE)
#> [1] "Error in try(httr::stop_for_status(r), silent = TRUE) : \n  Bad Request (HTTP 400).\n"
#> attr(,"class")
#> [1] "try-error"
#> attr(,"condition")
#> <http_400 in doTryCatch(return(expr), name, parentenv, handler): Bad Request (HTTP 400).>

# Demonstrate that request for 'area_ac' is the problem:
# Obtain query string
q <- SDA_spatialQuery(geom = g, what = "mupolygon", db = "SSURGO", geomIntersection = TRUE, query_string = TRUE)

# Remove request for 'area_ac'
q2 <- sub(
  ",\nGEOGRAPHY::STGeomFromWKB(\n    geom.STUnion(geom.STStartPoint()).STAsBinary(), 4326).STArea() * 0.000247105 AS area_ac",
  replacement = "",
  x = q,
  fixed = TRUE
)

# Pass query string without 'area_ac' to SDA
SDA_query(q2) |> processSDA_WKT()
#> single result set, returning a data.frame
#> Simple feature collection with 34 features and 1 field
#> Geometry type: GEOMETRY
#> Dimension:     XY
#> Bounding box:  xmin: -118.7875 ymin: 47.2125 xmax: -118.7458 ymax: 47.25417
#> Geodetic CRS:  WGS 84
#> First 10 features:
#>    mukey                           geom
#> 1  68209 POLYGON ((-118.7509 47.2331...
#> 2  68202 POLYGON ((-118.7504 47.2128...
#> 3  68238 POLYGON ((-118.7874 47.2201...
#> 4  68210 POLYGON ((-118.7627 47.2125...
#> 5  68210 MULTIPOLYGON (((-118.7514 4...
#> 6  68295 POLYGON ((-118.7477 47.2282...
#> 7  68202 MULTIPOLYGON (((-118.7607 4...
#> 8  68238 POLYGON ((-118.7843 47.2125...
#> 9  68216 POLYGON ((-118.7524 47.2272...
#> 10 68204 POLYGON ((-118.7831 47.2368...

Created on 2024-03-22 with reprex v2.1.0

brownag added a commit that referenced this issue Mar 22, 2024
@brownag
Copy link
Member

brownag commented Mar 22, 2024

Thanks for reporting this @dschlaep

I think this can be resolved by using geometry.MakeValid() on the result geometry that is used to calculate area i.e. 0f5510b

I don't think there is any problem with the input geometry per se, which is why sf::st_make_valid() has no effect.

I think it might be a good idea to, as you suggest, allow for the calculation of area to be omitted via an argument as it is usually easy enough to calculate this independently.

library(soilDB)
library(sf)
#> Linking to GEOS 3.11.2, GDAL 3.7.2, PROJ 9.3.0; sf_use_s2() is TRUE

sf::sf_use_s2(FALSE)
#> Spherical geometry (s2) switched off

# Polygon in eastern Washington
poly <- st_as_sfc("POLYGON ((-118.7458 47.2125, -118.7458 47.25417, -118.7875 47.25417, -118.7875 47.2125, -118.7458 47.2125))",
                  crs = 4326)
res <- SDA_spatialQuery(geom = poly, what = "mupolygon", db = "SSURGO", geomIntersection = TRUE)

st_area(poly)
#> 14628614 [m^2]
sum(st_area(res))
#> 14628619 [m^2]

Note that in order to calculate st_area(res) s2 geometry backend needs to be turned off. The intersection geometry result does not seem to play well with that--a common problem with SSURGO polygons in general in my experience.

Also I would like to look into returning a better error message for things like this, ideally you would not have to dig so deep to find the problem

@brownag
Copy link
Member

brownag commented Mar 22, 2024

I also note that calling geometry.MakeValid() on the result geometry itself (before calculating area) does not fix the problem with s2 geometry area calculation:

> sum(st_area(res))
Error in wk_handle.wk_wkb(wkb, s2_geography_writer(oriented = oriented,  : 
  Loop 1 is not valid: Edge 0 crosses edge 14

Nor can it replace the usage of geometry.MakeValid() on the geometry used to calculate area. The geom.STUnion(geom.STStartPoint()) bit seems to create a geometry with its own "problems" (depending on what the result contains for the input area)

GEOGRAPHY::STGeomFromWKB(geom.STUnion(geom.STStartPoint()).STAsBinary(), 4326).MakeValid().STArea() * 0.000247105 AS area_ac

@brownag
Copy link
Member

brownag commented Mar 22, 2024

One more thought: should we call sf::st_make_valid() on the geometry result when SDA_spatialQuery() returns geometry? I suppose if we do this we might want to have the ability to turn it off.

While it seems we can't fix the geometry within the SQL for use in s2, it does "work" to fix it with {sf} before returning the object.

This would fix above "subissue" I raised RE: some areas returning geometry that cannot be operated on with s2 backend without first making valid.

library(soilDB)
library(sf)
#> Linking to GEOS 3.11.2, GDAL 3.7.2, PROJ 9.3.0; sf_use_s2() is TRUE

sf_use_s2(TRUE)

# Polygon in eastern Washington
poly <- st_as_sfc("POLYGON ((-118.7458 47.2125, -118.7458 47.25417, -118.7875 47.25417, -118.7875 47.2125, -118.7458 47.2125))",
                  crs = 4326)
res <- SDA_spatialQuery(geom = poly, what = "mupolygon", db = "SSURGO", geomIntersection = TRUE)

st_area(poly)
#> 14588471 [m^2]
sum(st_area(res))
#> Error in wk_handle.wk_wkb(wkb, s2_geography_writer(oriented = oriented, : Loop 1 is not valid: Edge 0 crosses edge 14

res2 <- st_make_valid(res)
sum(st_area(res2))
#> 14588476 [m^2]

@dschlaep
Copy link
Contributor Author

That's great, the fix works for me! Many thanks!

should we call sf::st_make_valid() on the geometry result when SDA_spatialQuery() returns geometry?

-> Not sure to whom the question was addressed. I feel that may be a design choice for soilDB. I'm fine either way. One approach could be to follow sf's approach which appears to put the user in charge of making a returned object valid if needed. For that, I briefly searched the code base of sf. It appears that sf calls st_make_valid() only from two other functions: st_as_sfc.SpatialPolygons() and st_interpolate_aw.sf().

@brownag
Copy link
Member

brownag commented Mar 28, 2024

Great! Thanks @dschlaep. The question was more or less open to anyone who may read these issues, and just me thinking out loud to some degree, thanks for your input.

Your point is a good one about how st_make_valid() is (not) used in sf.

I think my preference is to leave validity fixes up to the user, and have soilDB stick to providing the data more or less as it exists in the "source" (however "invalid" it may be at times).

I will leave this issue open until I add the argument for turning off acre calculations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants