> On Nov 28, 2020, at 12:44 PM, Joris Van den Bossche 
> <jorisvandenboss...@gmail.com> wrote:
> 
> Thanks for trying to reproduce it in C/C++. One obvious difference that I can 
> spot is that we use an integer for the "item" that gets inserted, and not the 
> geometry itself, but I wouldn't expect that to influence the result. 
> Although, trying to update your test case to do that, the test fails. But 
> that might also be an issue on my side due to my limited C++ experience (it 
> already fails on the "geoms.size()" check):

Nope still not seeing it... one small mistake in your query, trying to cast the 
int to a void, instead of passing in a the pointer to the address, here's one 
that works. Unfortunately that leaves us no closer to knowing why the 
SimpleSTRtree is unhappy in the python context. I fear I may just have to 
revert the CAPI to the old tree.


// querying tree with box
template<>
template<>
void object::test<9>
()
{
    GEOSSTRtree* tree = GEOSSTRtree_create(10);

    GEOSGeometry* g = GEOSGeomFromWKT("POINT (2 3)");
    int payload = 876;
    GEOSSTRtree_insert(tree, g, &payload);

    GEOSGeometry* q = GEOSGeomFromWKT("POLYGON ((0 0, 10 0, 10 10, 0 10, 0 
0))");

    typedef std::vector<int*> IList;
    IList items;
    ensure_equals(items.size(), 0);
    GEOSSTRtree_query(
        tree,
        q,
        [](void* item, void* userdata) {
            IList* items = (IList*)userdata;
            items->push_back((int*)item);
        },
        &items);

    ensure_equals(items.size(), 1);

    ensure_equals(*(items[0]), payload);

    GEOSGeom_destroy(q);
    GEOSGeom_destroy(g);
    GEOSSTRtree_destroy(tree);
}




> 
> --- a/tests/unit/capi/GEOSSTRtreeTest.cpp
> +++ b/tests/unit/capi/GEOSSTRtreeTest.cpp
> @@ -268,10 +268,11 @@ void object::test<8>
>  {
>      GEOSSTRtree* tree = GEOSSTRtree_create(10);
>      GEOSGeometry* g = GEOSGeomFromWKT("POINT (2 3)");
> -    GEOSSTRtree_insert(tree, g, g);
> +    int idx = 0;
> +    GEOSSTRtree_insert(tree, g, (void*)idx);
>      GEOSGeometry* q = GEOSGeomFromWKT("POLYGON ((0 0, 10 0, 10 10, 0 10, 0 
> 0))");
>  
> -    typedef std::vector<GEOSGeometry*> GList;
> +    typedef std::vector<int> GList;
>      GList geoms;
>      ensure_equals(geoms.size(), 0);
>      GEOSSTRtree_query(
> @@ -279,23 +280,16 @@ void object::test<8>
>          q,
>          [](void* item, void* userdata) {
>              GList* geoms = (GList*)userdata;
> -            geoms->push_back((GEOSGeometry*)item);
> +            geoms->push_back(*((int *)item));
>          },
>          &geoms);
>  
>      ensure_equals(geoms.size(), 1);
> -    const GEOSCoordSequence* seq = GEOSGeom_getCoordSeq(geoms[0]);
> -
> -    double x = -1;
> -    double y = -1;
> -    GEOSCoordSeq_getXY(seq,  0, &x, &y);
> -    ensure_equals(x, 2.0);
> -    ensure_equals(y, 3.0);
> +    ensure_equals(geoms.at(0), 0);
>  
> 
> On Sat, 28 Nov 2020 at 20:55, Paul Ramsey <pram...@cleverelephant.ca> wrote:
> 
> 
> > On Nov 28, 2020, at 8:11 AM, Joris Van den Bossche 
> > <jorisvandenboss...@gmail.com> wrote:
> > 
> > Hi all,
> > 
> > On the CI of PyGEOS we have a build testing against GEOS master, and 
> > somewhere in the last 4 days, a lot of the STRtree tests started failing 
> > (see eg https://github.com/pygeos/pygeos/runs/1465460418#step:9:86). 
> > Looking at the commits of the last days, this might be related to the 
> > SimpleSTRtree work?
> > 
> > A small (python) example of a tree consisting of a single point, which no 
> > longer is returned when querying the tree with a big polygon that certainly 
> > contains the point:
> > 
> > Using released version of GEOS:
> > 
> > >>> import pygeos
> > >>> pygeos.geos_version
> > (3, 8, 1)
> > >>> point = pygeos.Geometry("POINT (2 3)") 
> > >>> tree = pygeos.STRtree([point]) 
> > >>> tree.query(pygeos.box(0, 0, 10, 10)) 
> > array([0])
> > 
> > This is correctly returning the index of the single point. But when running 
> > with the latest GEOS master, the query doesn't find any point of the tree:
> > 
> > >>> import pygeos
> > >>> pygeos.geos_version
> > (3, 9, 0)
> > >>> point = pygeos.Geometry("POINT (2 3)")
> > >>> tree = pygeos.STRtree([point])
> > >>> tree.query(pygeos.box(0, 0, 10, 10))
> > array([], dtype=int64)
> > 
> > Are there changes expected in how the STRtree C API functions or required 
> > changes in user code? Or maybe we are using it in some incorrect/unexpected 
> > way? (code is at https://github.com/pygeos/pygeos/blob/master/src/strtree.c)
> 
> There are changes, I don't think you're mis-using anything. I swapped the 
> CAPI to use the SimpleSTRtree, figuring it would be good to share the 
> performance win with downstream. However, I can swap it back to the original 
> STRtree if this remains a problem.
> 
> One thing I noticed when trying to construct GEOS envelopes directly was that 
> annoyingly they were xmin xmax, ymin ymax, but I doubt that would be a 
> problem in your pre-existing working test. 
> 
> I just reconstructed your test in the GEOS CAPI suite, and it works as one 
> would expect. (Namely, it finds the one point.) So I'm not sure why your test 
> is getting different results.
> 
> 
> // querying tree with box
> template<>
> template<>
> void object::test<8>
> ()
> {
>     GEOSSTRtree* tree = GEOSSTRtree_create(10);
>     GEOSGeometry* g = GEOSGeomFromWKT("POINT (2 3)");
>     GEOSSTRtree_insert(tree, g, g);
>     GEOSGeometry* q = GEOSGeomFromWKT("POLYGON ((0 0, 10 0, 10 10, 0 10, 0 
> 0))");
> 
>     typedef std::vector<GEOSGeometry*> GList;
>     GList geoms;
>     ensure_equals(geoms.size(), 0);
>     GEOSSTRtree_query(
>         tree,
>         q,
>         [](void* item, void* userdata) {
>             GList* geoms = (GList*)userdata;
>             geoms->push_back((GEOSGeometry*)item);
>         },
>         &geoms);
> 
>     ensure_equals(geoms.size(), 1);
>     const GEOSCoordSequence* seq = GEOSGeom_getCoordSeq(geoms[0]);
> 
>     double x = -1;
>     double y = -1;
>     GEOSCoordSeq_getXY(seq,  0, &x, &y);
>     ensure_equals(x, 2.0);
>     ensure_equals(y, 3.0);
> 
>     GEOSGeom_destroy(q);
>     GEOSGeom_destroy(g);
>     GEOSSTRtree_destroy(tree);
> }
> 
> 
> 
> 
> 
> > 
> > Best,
> > Joris
> > 
> > 
> > On Wed, 25 Nov 2020 at 00:44, Paul Ramsey <pram...@cleverelephant.ca> wrote:
> > Hey all, just truing up what's underway and nearly there...
> > 
> > - Am I right that Z coordinates are nearly done? What's the status there?
> > 
> > I've been trying to address some performance issues, with some success and 
> > some ... other things. 
> > 
> > The success is the SimpleSTRtree, which is just the current STRtree but 
> > without the inheritance structure and with the nodes stored all next to 
> > each other in contiguous memory for more locality. For at least one use 
> > case I've seen 20% speed-ups on overlays, using the SimpleSTRtree in place 
> > of the STRtree inside the MCIndexNoder. I have not seen any slow-downs. I 
> > have pushed the SimpleSTRtree into master.
> > 
> > While I have implemented the nearestNeighbor() functionality on the 
> > SimpleSTRtree, I haven't hooked it up to anything yet. It could go into the 
> > IndexedFacetDistance, if anyone is super enthusiastic about it. From there 
> > it would affect searching in PreparedGeometry of various sorts.
> > 
> > I also tried using a similar trick with the MonotoneChainBuilder that sits 
> > inside the MCIndexNoder, replacing individual heap allocations with slabs 
> > by putting objects onto a std::deque, and incidentally stripping out some 
> > book-keeping. While that seems to pick up about 3-5% speedwise, 
> > unfortunately something about my implementation is incorrect (and in a 
> > wonderfully subtle way) as it fails testing on some platforms (not mine). 
> > https://github.com/pramsey/geos/tree/monotone-chain-builder
> > 
> > I've put that work to the side for now.
> > 
> > All the performance talk is mostly because JTS still runs a lot faster than 
> > GEOS for some bulk processing. My current test is a big union of watershed 
> > boundaries, about 6MB of data, which takes about 20s under GEOS and about 
> > 25% of that under JTS.  It's a big gap, and in theory the two code bases 
> > are pretty aligned right now. Same overlayNG engine, etc. So I figure there 
> > has to be a big implementation ball of performance hiding under the covers 
> > somewhere. No luck thus far.
> > 
> > I think we're close, looking forward to release :)
> > 
> > P
> > _______________________________________________
> > geos-devel mailing list
> > geos-devel@lists.osgeo.org
> > https://lists.osgeo.org/mailman/listinfo/geos-devel
> > _______________________________________________
> > geos-devel mailing list
> > geos-devel@lists.osgeo.org
> > https://lists.osgeo.org/mailman/listinfo/geos-devel
> 
> _______________________________________________
> geos-devel mailing list
> geos-devel@lists.osgeo.org
> https://lists.osgeo.org/mailman/listinfo/geos-devel
> _______________________________________________
> geos-devel mailing list
> geos-devel@lists.osgeo.org
> https://lists.osgeo.org/mailman/listinfo/geos-devel

_______________________________________________
geos-devel mailing list
geos-devel@lists.osgeo.org
https://lists.osgeo.org/mailman/listinfo/geos-devel

Reply via email to