st_getFaceGeometry return null #7
Comments
|
ST_BuildArea uses only the edges you pass to it, so clearly it won't work if you don't give it ALL the edges forming a face. My recommendation here is to add the reference grid to the topology, so to make sure every single face of the topology will be covered exactly by one and one only grid cell. This would imply adding all lines of the grid (horizontal lines, vertical lines) with TopoGeo_addLinestring.
The count you obtain above is the count of so called "dangling edges" (if one of the end nodes is shared with other edges) or "isolated edges" (if both end nodes are only connected to it). Having those kind of edges may be the result of different things, they do not necessarely represent a malformed topology so are allowed (non-areal features would result in such kind of edges). If you are sure you always ever passed full valid polygons or rings to the topology population functions, then the only reason for dangling or isolated edges to appear would be dimensional collapse due to either snapping or robustness (impossibility to represent an intersection point without moving a line so much to collapse to another one). Catching these occurrences as early as possible would help building over those errors. |
|
Sorry, I just noticed you mention |
|
The problem with https://postgis.net/docs/manual-3.0/ValidateTopology.html is that it can not be run for given area so I am not able to use it on big data sets. I need to bee able i verify/fix for a selected area if this should work I belive. |
|
I just added ST_BuildArea to test if all the edges that are needed to build the the simple feature polygon are in the egde and that seems to be the case , also when st_GeoFaceGeometry is null and mbr i not null. So it does not look like any edges area missing. This does not happing where often. |
|
I did a test where I added the edges that did not very well with big datasets. I got more error and it took quite a time to remove the cell borders. I also I had to keep track the part of the cell bords that overlapped with the edges from the input and that should not be removed. Here is the branch where I did the test https://github.com/larsop/resolve-overlap-and-gap/tree/add_postgis_topology_use_cell_borders |
This could be a good improvement, although by definition a topology is a set of related features so the amount of features overlapping a given area may very well extend outside of it and eventually get to require looking at the whole topology (imagine specifying an area which contains the external edge of a big country, in order to verify correctness of those edges the code will still need to walk the whole external ring of the country, which would span the whole topology). That said, it could be useful to have functions to check specific validities. |
|
On Fri, Apr 24, 2020 at 01:56:29AM -0700, Lars Aksel Opsahl wrote:
I just added ST_BuildArea to test if all the edges that are needed
to build the the simple feature polygon are in the egde and that seems to be the case,
also when st_GeoFaceGeometry is null and mbr i not null.
So it does not look like any edges area missing.
The ST_GetFaceGeometry function is using ST_BuildArea with a
collection of edges having the specified face recorded as either being
on their left or right side:
https://git.osgeo.org/gitea/postgis/postgis/src/branch/master/liblwgeom/lwgeom_topo.c#L2732-L2777
So if your ST_BuildArea works and ST_GetFaceGeometry does not, we may
be in presence of an error with the values of `left_face` and
`right_face` for some of the edges.
Can you provide a minimal dataset and accompanying query showing the problem ?
The dataset would be a dump of the topology (reduced as much as
possible).
|
Yes that's the problem , when I checked the last time some edges referring to a very big face and not the ones close by.
I will find an example |
|
Here is example where we have a dangling egde which should not be dangling
|
|
So basically what I need to after after all cells are glued together, I need find not truly dangling edges (like the one above) and update these with the correct left and right face . Is this possible/difficult ? |
|
What you should find out is HOW those not-truly dangling edges were allowed in the database. |
|
Updating the left and right face for an edge could be as simple as running an |
Yes this is may main problem I was sure about how about figure out what face to use in a simple way. |
I belive this happens in this way .
Usually this goes ok and we get face with a valid geometry based in this edges, but sometimes this fail. I have not been able catch when happens.
Yes I can use this plugin , but when I run I see logs faces that have this problem when I created for instance a simple feature layer based this Topology layer.
I have not been able to produce a simpe test case here and it seems to vary from time to time and layer to layer. |
One possible cause of it could be 2 transactions running at the same time and the last one committing work which overrides the work performed by the other. This is what you want to avoid and can surely happen. If this is the case you should NEVER be able to see such behaviour when ensuring a single client ever edits the Topology (I know you're trying to avoid this, but worth verifying). One other possible cause (if running single-thread) could be that when you did split the original polygon into multiple edges, some edge gets threated (snapped, due to tolerance) in a way that it ends up NOT being connected anymore with its "other side", resulting in an open ring.
The TopologyViewer can be used to just inspect edge table, which gives more information here. Keep the "faces" layers off, just see edges and their attributes, as they give a visual indication of which edge has which face on which side, and which edges are supposed to be connected to each edge, via next_left, next_right edge attributes, these two classes of attributes can be inspected-with-your-eyes to verify their correctness (togheter with the direction of edges). |
This sound very nice , I have some problems to use the way describe . I have upgraded qgis 3.10 and in the DB Manager I see this. Is it any user documentation for how to use this with Postgis Topology. |
I did this test now and it does not seem to be a snap to problem . ` `
|
I did a test https://github.com/larsop/resolve-overlap-and-gap/tree/test_single_thread_issues_7 where I used a single thread for inserting the glue lines and that did not help. Here also added code where thread to thread remove and add the egde but that did not help , because probably need to healed the line first. I will run another test where I do a union with exiting lines before I add the glue line. |
This failed also. Tested at https://github.com/larsop/resolve-overlap-and-gap/tree/test_union_with_existing_edges_issues_7 It seems like splitting up the edges that crosser the cell borders are make too many error for some data sets. I will no do another test where I do not splitt the lines that cross the cell borders, but keep them as they are. |
I'm afraid not, but it what that plugin does is only creating a layer group containing various useful layers to inspect different things. It's not recommended to turn visibility of all layers ON. Rather you'd only enable one after the other, in turn, to see which information you find useful and which not. What I see from the screenshot is that you have the "face MBR" layer visible, which you probably don't want. I would actually turn off all layers under the "face" group and only focus on the layers under the "Edge" group. I don't fully see those groups in your screenshot so I wonder if something is not working as expected. I'll build QGIS-3.10 and see what I get from it. |
|
On Fri, Apr 24, 2020 at 11:26:41PM -0700, Lars Aksel Opsahl wrote:
select topology.TopoGeo_addLinestring('topo_sr16_mdata_02',geom,1)
from (
select
ST_RemEdgeModFace('topo_sr16_mdata_02',edge_id),
geom
from topo_sr16_mdata_02.edge_data
where edge_id = 756267) as r;
topogeo_addlinestring
-----------------------
1106439
Sorry I don't understand how the above query is a test for what I was
saying about snapping. If I understand your query correctly what you
are doing is:
1. Remove one dangling edge from the topology
2. Add the Linestring of that edge back in the topology
3. Verifying that the insertion of given Linestring results
again in a single edge, with same start/end node, same
next left/right edges.
One thing I note in the result of that operation is that while
the original edge was marked as "dangling" (same face on both sides)
it is now instead reported to bind 2 different faces. I don't see
as such behaviour could happen if not due to malformation of the
original topology: that edge must have bound 2 different faces
from the start, and that information must have gone lost somehow,
maybe due to parallel update of its attributes.
|
|
I did a test now where used a single thread for inserting the glue lines and that did not help.
Perfect, we have much higher chances of handling such case (easier to
reproduce). Can you reduce the test to provide a start topology and
operations that do break it ?
Please also when referring to code (ie: "inserting glue line") can you
point me to the exact code snippet doing that ? I'd like to better
understand the operations you are using to do this.
It seems like splitting up the edges that crosser the cell borders
are make too many error for some data sets.
Splitting edges can always create problems, because unless edges are
split at one of their existing vertices, single segments are bound to
move slightly to be able to represent e _new_ vertex, and such
movement could result in an open ring. All these occurrences are
supposedly handled by the splitting performed by the topology
functions themselves, and for this reason I suggested to insert the
edges of your defined grid to the topology, so that they are found
by the topology routing and used to do "controlled" splitting of the
edges you insert.
|
Yes you are reading it correctly and values for faces are fixed when deleting and the edge add again. I also did a test with where used tolerance like 0 and that worked. The problem also seems to be also when running i single thread. Can the problem be something like this ?
The problem seems be that dangling edge is not always been notified/updated about new face. I will try make a test for this with a smaller dataset. |
I tried to do something like this here where I also added the cell borders https://github.com/larsop/resolve-overlap-and-gap/tree/add_postgis_topology_use_cell_borders/src/main/extern_pgtopo_update_sql But the performance was very poor and I also needed needed to removed a lot of edges in a post operation because they where only cell borders and not a line from the input layer, so went I stopped testing this branch. I may try to see if it's possible to develop this further. |
|
I have been able to make a small test that fails almost every second I run. By failing I mean we get dangling edges. SELECT ST_length(geom), edge_id , start_node, next_left_edge , next_right_edge, abs_next_right_edge, right_face from test_topo_t3.edge_data where left_face = right_face If I try to check faces with st_getFaceGeometr If I check with this cmd I find where catch errors
|
|
Can the problem be something like this ?
1) A edge is added which is dangling at stage one and that is correct. The face may be very big.
2) More edges are added then at at certain stage we get a new face.
The problem seems be that dangling edge is not always been notified/updated about new face.
Edges to be notified about the new faces are the edges that formed a
new ring. Ring is determined by following the "next_left_edge" and
"next_right_edge" attributes of the "edge" table. This is what
https://postgis.net/docs/GetRingEdges.html also does.
On the liblwgeom level, a callback is used to do the edge walking,
which boils down to a RECURSIVE CTE. Such recursive CTE, in absence
of a LIMIT paremeter (it's invoked with no LIMIT, from liblwgeom)
may enter an infinite loop, when edge linking is incorrect (it may
or may not be what is happening in my `make check` run, for instance).
The code of the plpgsql function:
https://git.osgeo.org/gitea/postgis/postgis/src/tag/3.0.1/topology/sql/query/GetRingEdges.sql.in#L50-L61
The code of the C callback:
https://git.osgeo.org/gitea/postgis/postgis/src/tag/3.0.1/topology/postgis_topology.c#L1182-L1192
They should be equivalent queries, and should probably be merged:
https://trac.osgeo.org/postgis/ticket/4675
So, if "edge is not always been notified/updated", there are two
options:
1) GetRingEdges does NOT find that edge (meaning edge linking is
broken == already invalid topology)
2) UPDATE is ineffective (for another update in another process
overriding the previous one?)
I've been thinking for some time now that we should implement some
form of database level locks to avoid second scenario, but if you
mention you can see this happening in a single thread my guess is
that we're in presence of the first scenario instead.
|
|
On Thu, Apr 30, 2020 at 09:49:30PM -0700, Lars Aksel Opsahl wrote:
I have been able to make a small test that fails almost every second I run. By failing I mean we get dangling edges.
Dangling edges from input without dangling edges may still not be
a failure, as long as the resulting topology is valid (dangling edges
are not invalid, but natural way to develop a ring in chunks).
SELECT ST_length(geom), edge_id , start_node, next_left_edge , next_right_edge, abs_next_right_edge, right_face from test_topo_t3.edge_data where left_face = right_face
;
st_length | edge_id | start_node | next_left_edge | next_right_edge | abs_next_right_edge | right_face
--------------------+---------+------------+----------------+-----------------+---------------------+------------
16638.511442709078 | 7151 | 5625 | -6957 | -4029 | 4029 | 0
15668.515454601618 | 7157 | 2706 | 3522 | -7651 | 7651 | 1465
9558.525838860027 | 7177 | 2809 | 7182 | 2815 | 2815 | 1471
46869.15086689703 | 7269 | 1471 | 3214 | 1482 | 1482 | 1484
15683.408435026411 | 7408 | 3856 | -3964 | -7410 | 7410 | 1546
These records do not necessarely represent an invalid topology.
Each of those edges could be the body of the hourglass in the
following ASCII drawing:
```
*-----*
\ /
\ /
*
|
| <- this
|
*
/ \
/ \
*-----*
```
If I try to check faces with st_getFaceGeometr
select * from test_topo_t3.face where st_getFaceGeometry('test_topo_t3',face_id) is null;
ERROR: XX000: SQL/MM Spatial exception - universal face has no geometry
You have to explicitly skip universal face (0):
```
SELECT id
FROM test_topo_t3.face
WHERE face_id != 0
AND st_getFaceGeometry('test_topo_t3',face_id) is null;
```
If I check with this cmd I find where catch errors
select count(*) from test_topo_t3.face where topo_update.get_face_geo('test_topo_t3',face_id,0) is null;
count
-------
23
(1 row)
Interesting, this does indeed sound like a bug.
To make this state I used the branch
https://github.com/larsop/resolve-overlap-and-gap/tree/test_faling_left_like_right_face_issues_7
[...]
I will try to follow these steps, but I can't avoid to note it's still
really too many steps, and high complexity (dblink involved).
Attaching a dump of the resulting topology may also be a good idea.
|
|
https://github.com/larsop/resolve-overlap-and-gap/tree/test_faling_left_like_right_face_issues_7
and ran the tests './run_test.pl --verbose --spatial_ref_sys -topology resolve_overlap_and_gap;'
I'm assuming you mean to run that command from the
`src/test/sql/regress` directory, in order to find `./run_test.pl`.
But running as you specify fails with:
Unable to find /usr/src/nibio/resolve-overlap-and-gap/src/test/sql/regress/00-regress-install/share/contrib/postgis/postgis.sql
Adding the `--extension` switch makes the tester us PostGIS via
EXTENSIN, fixing this problem, although the warning fixed in
master and reported in #6
is back, and the test still never completes for me, due to what I
reported in #12
I'm afraid I'll need to focus on #12 before being able to follow
instructions implying running those tests.
|
Here is dump from postgres 12 og POSTGIS="3.0.0 r17983" [EXTENSION] PGSQL="120" GEOS="3.8.0-CAPI-1.13.1 " PROJ="6.3.0" LIBXML="2.9.10" LIBJSON="0.13.1" LIBPROTOBUF="1.3.2" WAGYU="0.4.3 (Internal)" TOPOLOGY The number null faces and faces where left_face= right_face is a kind of random from time to time. 'pg_dump -Z9 -f test_faling_left_like_right_face_issues_7.sql.gz resolve_og' |
|
Here is a dump of the schema only. pg_dump -Z9 -n test_topo_t3 -f test_topo_t3.sql.gz resolve_og |
|
Ok I've imported the dump, then registered the topology with:
Note I used 0 as the tolerance, because the dump does not contain any information about what's the actual tolerance was. The imported topology, according to ValidateTopology, has many invalidities: So next we shall find out HOW such invalidities were introduced. Picking the first "face has no rings" record (face_id 1460), I see that there's a single edge marked as having that face on a side: edge_id 7166, having face 1460 on its left side and face 1462 on its right side. The face on the other side (1462) is ALSO found as being a "face has no ring" error, and is also only known by that edge. So we have a single edge (edge_id 7166) marked as having 2 different faces on its sides, NOT closed (end_node != start_node), NOT isolated (abs_next_left_edge != 7166, abs_next_right_edge != 7166) BUT exclusively knowing about those faces (face_id 1460 and 1462). Here's a picture of that situation, extracted from QGIS-3.12:
The GetRingEdges function tells us that edge 7166 is part of a proper and closed ring: You can see the next (3498) and previous (6982) edges in the map image above. So what's happening is most likely that the face on the left of edge 7166 should be face_id=1724 (the face on that side registered on next and previous edges), and the face on the right of edge 7166 should be face_id=1671 (the face on that side registered on next and previous edges). Updating the edge accordingly should (if it's the only issue) fix that specific problem, which results in GetFaceGeometry returning NULL also for faces 1724 and 1671 missing a portion of their ring (see https://trac.osgeo.org/postgis/ticket/4680#ticket for a possible enhancement to GetFaceGeometry that would make it work also in these cases). I've also filed https://trac.osgeo.org/postgis/ticket/4681#ticket to enhance ST_GetFaceGeometry so it prints a WARNING when such cases are found. So, to recap:
It remains to understand WHY the topology turned invalid in the first place, but I'd do this in a different ticket, because the title of this one is specifically addressing ST_GetFaceGeometry returning NULL. If you agree please file another ticket about "Topology construction routine results invalid topology" or similar. |
|
I have filled new ticket here I have also been doing some more and it seems closely related how many threads and how complex the input are. The best result seems to be when using branch. What I do here is that I do not cut the lines that cross the cell borders using the cell border, by using this code. When doing this we way don't need put together those lines connecting the cell at later stage. Another change is at the stage 3 line 163 where where we merge all the cells together I also run this as one single thread. We stills get some missing geometries but in one test case we reduced the number lost significant geometries from maybe around 400 hundred to less than 10 |
|
One update I check the results from last night using branch where I do not cut lines crossing the cell wall and it seems like the lines where left equal right face are that not connected to cell borders any more. I used this Here is SQL that show big differences. With the no cut line branch and a single thread we get 8 problems related to lines crossing cell borders. But using a branch where we cut the lines and merge them in a multithread environment we get almost 3000 error . SELECT count(*) from topo_sr16_trl_09.edge_data e, (select ST_ExteriorRing(geo) as geo from topo_sr16_trl_09.trl_2019_test_segmenter_renska_grid) as r where ST_Intersects(r.geo,e.geom) and st_length(geom) > 10 and left_face != 0 and right_face = left_face;
|
|
I've filed PR #15 to run the topology validation step as part of the existing test. |
|
Can we close this ticket as completed ? |
|
Yes this solved as long as we run in a single thread in step 3 where we insert lines crossing cell borders. We will also do a test with row level lock to see if that can help to run in parallel. |
When I add data i break everything into cells and glue together at later stage. Sometimes faces and the edges are handled incorrectly at the cell borders. If I have face where this is the case I can often run this command to rebuild the simple feature geo,
command_string = FORMAT('select ST_BuildArea(ST_Collect(e.geom)) from %1$s.face f, %1$s.edge_data e where ST_Covers(ST_Expand(f.mbr,%3$s),e.geom) and face_id = %2$s', _atopology, _face_id,tolerance); execute command_string into face_geo;The reason why we get a null geometry from st_getFaceGeometry is that that one or more of the edges often belong to very big face, and then left and right are often equal.
Im now doing some testing different smaller datasets than ar5 to be able to this the code in reasonable time.
Running this sql and getting a big number and long lines usually indicates a lot of errors.
SELECT count(*) from topo_sr16_mdata_01.edge_data where right_face = left_face;
Why is this happening ?
How can we prevent from happening ?
Is is possible to fix it if it happens ?
The text was updated successfully, but these errors were encountered: