Skip to content

WIP: support for prepared geometries#84

Closed
brendan-ward wants to merge 1 commit intopygeos:masterfrom
brendan-ward:prepared
Closed

WIP: support for prepared geometries#84
brendan-ward wants to merge 1 commit intopygeos:masterfrom
brendan-ward:prepared

Conversation

@brendan-ward
Copy link
Copy Markdown
Contributor

Work toward supporting GEOSPreparedGeometry created from GEOSGeometry objects.

I'm new to doing Python C extensions with ufuncs, so I could be doing something obviously incorrect here.

This doesn't yet have the prepared geometry predicates, pending some questions / issues (and some segfaults I haven't been able to sort out):

  1. This approach has GEOSPreparedGeometry as a companion object to GEOSGeometry objects, with all of the associated wrappings and types. This roughly follows shapely's approach of having a PreparedGeometry object. For usage, this means that the user needs to specifically create prepared geometries before running a spatial operation that uses them (related questions below). Given how those operations are performed as ufuncs, I don't see an easy way to do just-in-time prepared geometries, but I could be missing something.

  2. It seems like there are implications around tracking references to the underlying geometry, and deallocating them when appropriate, that seem potentially problematic here. I'm new enough to how we are handling references here that I don't feel I can fully identify those issues.

  3. Is there a way to identify a prepared geometry from a regular geometry, so that in the higher-level API, we don't need to add specific API functions for prepared geometries - we just leverage the prepared geometry and run the associated predicate if we a passed a prepared geometry (this is what shapely does)? E.g., keep one intersects function, instead of adding a intersects_prepared function at the high level. Internally, we'd have separate functions.

  4. should conversion to prepared geometries be one way or two way? Meaning, should we allow the user to convert back from or otherwise get the underlying unprepared geometry? Or should prepared geometries basically be a dead end, only used as inputs for prepared geometry predicates?

.tp_basicsize = sizeof(PreparedGeometryObject),
.tp_itemsize = 0,
.tp_flags = Py_TPFLAGS_DEFAULT,
// .tp_new = PreparedGeometryObject_new, // TODO
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not clear if this is needed here, since the only way to create a prepared geometry is from an existing geometry?

/* in case of a missing value: return NULL (NaG) */
ret_ptr = NULL;
} else {
ret_ptr = func(context_handle, in1);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is currently crashing (on Mac, GEOS 3.8) with terminated by signal SIGSEGV (Address boundary error)

I think the way I've wrapped the GEOSPrepare function used here seems valid - it copies how this was done for other unary ufuncs above.

@caspervdw I'm probably missing something really obvious here that you're likely to spot quickly - are you able to point me in the right direction?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also get a segfault, but it seems it is not crashing here, but in PreparedGeometryObject_FromGEOSPreparedGeometry (called in OUTPUT_Yp below). And there, it seems the tp_alloc call segfaults. But not directly sure why this happens, since the PreparedGeometryObject seems very similar to GeometryObject.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for my late response, I have been very busy.

I don't see anything obvious going wrong, but I am thinking whether the GEOSPreparedGeometry is able to function when the related GEOSGeometry is already deallocated? Could a segfault be originating from that issue?

Copy link
Copy Markdown
Member

@jorisvandenbossche jorisvandenbossche left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for looking into this!

/* in case of a missing value: return NULL (NaG) */
ret_ptr = NULL;
} else {
ret_ptr = func(context_handle, in1);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also get a segfault, but it seems it is not crashing here, but in PreparedGeometryObject_FromGEOSPreparedGeometry (called in OUTPUT_Yp below). And there, it seems the tp_alloc call segfaults. But not directly sure why this happens, since the PreparedGeometryObject seems very similar to GeometryObject.

@brendan-ward
Copy link
Copy Markdown
Contributor Author

Obviated by #92

@brendan-ward brendan-ward deleted the prepared branch January 24, 2021 17:36
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 this pull request may close these issues.

3 participants