On Sat, 28 Nov 2020 at 21:53, Paul Ramsey <pram...@cleverelephant.ca> wrote:
>
>
> > 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
Also not if you try my original code *with* the mistake?
Because your version indeed passes for me as well, but I *think* the
version I wrote resembles more closely the PyGEOS code (so it might be an
issue in our C code on how we use the tree).
> , 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
>
_______________________________________________
geos-devel mailing list
geos-devel@lists.osgeo.org
https://lists.osgeo.org/mailman/listinfo/geos-devel
