Add XPath geofence() function

[tiritea]

Closes #TBD

What has been done to verify that this works as intended?

Have written unit tests comprising points inside and outside prescribed simple geoshape polygons. Included potential edge cases; specifically points with the exact same X or Y coordinate as vertices, since this can cause issues for some point-in-polygon algorithms.

Have not been able to do live testing of geoshape (and geopoint) acquired in a real form - this still needs to be tested in a live ODK Collect running a real form

Why is this the best possible solution? Were any other approaches considered?

I looked at several point-in-polygon geometry algorithms and this seems to be the 'best', it is historically well exercised (as best I can tell), and computationally efficient. It handles both convex and concave polygon perimeters.

I have not found a good pure geospatial solution, i.e. one that actually treats the surface as quasi-spherical. This is a pure Cartesian point-in-polygon algorithm.

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

This is an entirely new XPath function that will not impact any existing forms or behavior.

Do we need any specific form for testing your changes? If so, please attach one.

It will require a new form that captures a geopoint and a corresponding geopshape, and a new form definition that includes a calculation to exercise the new XPath function accordingly [which may consequently be rejected by pyxform due to the as-yet-unknown new XPath function...]. I am happy to provide an XForms form that does this, but as I've not been able to test this life yet I dont have one on hand :-(

Does this change require updates to documentation? If so, please file an issue here and include the link below.

This new Xpath function will need to be documented accordingly. Specific mention should be made to situations where the algorithm becomes degenerate. Specifically it cannot be used on areas that surround either pole. Also, if the target geoshape crossed the prime meridian then the coordinates specified for the geoshape should be adjusted to ensure they correctly map to corresponding Cartesian coordinates (ie you dont try to go from long 358deg to 2deg, but instead 358deg to 362deg) . And geoshapes with very long edges should probably be broken into shorter ones, due to fact great circles in spherical coordinates don't precisely map to straight lines in Cartesian coordinates. Failure to do so will result in points actually inside the geoshape - on a spherical surface - being incorrectly identified as outside the polygon - of a flat surface.

[lognaturel]

So exciting, thank you!

I’ll review in more detail shortly and make sure we have some good test forms. In parallel, I’d like to have a thread up on the forum to share the approach and function naming in case anyone has ideas to chime in with as well as for visibility. I think an Ideas thread that point to https://forum.getodk.org/t/odk-geofence-v1/18656/11 for history would be great. I’m happy to do that unless you would like to do so.

Naming-wise, did you consider any other options? Maybe more boolean-ish names like in-geofence(), in-polygon(), or inside()? I’m not loving any of those yet and maybe geofence() is our best bet. I’d find it helpful to briefly get a sense of what your thoughts on naming are!

[tiritea]

I'm happy to write a post about the new function once its released, describing the point-in-polygon algorithm used, and its caveats. Assuming of course y'all are happy with it and it gets a bit more real-life testing before being released into the wild... Note, for the next 2 weeks I'll be a bit busy transitioning jobs, so if you'd like to publish something before that then please dont wait for me!

I'll not too precious about naming it. FWIW I lean towards obvious and concise; "geofence" describes exactly what the function is performing, so I dont actually see any problem with using it. ODK also doesn't really have a well-established precedent for prefixing things with, say, 'is-foobar' to denote boolean results (eg we have "selected(), contains(), starts-with(), ...) so I dont think adding an in-geofence() really adds a lot. Also, its worth calling it something that will likely come up readily should somebody do the obvious and search for "ODK geofence". But, again, I really dont have a hugely strong opinion about it.

[lognaturel]

Thanks so much for that reasoning, it makes a lot of sense. We’ll start testing shortly!

Huge congrats on the new gig and wishing you a fun and exciting start. ✨

[tiritea]

Thanks for testing. As noted, there are a bunch of hardcoded unit tests; I've tried to cover what I think could be edge or degenerate cases for the point-in-polygon algorithm. eg points inside or and outside that are exactly in line with points or horizontal edges of the polygon (this is a problem for similar algorithms, but hopefully not this one...). Obviously, if you are exactly at one of the corners or exactly on an edge then the outcome is somewhat indeterminate as to whether you are 'inside' or 'outside'. I'm also unclear what will happen if you happen to have, say, degenerate self-intersecting polygons and the like, but again these are somewhat ill-defined to begin with. Mostly it needs testing with real geopoint locations against some real non-trivial convex & concave geoshapes, including inside and outside above, below, left, right of permutations.

[lognaturel]

Todo:

• rebase on master to pull in recent changes to distance
• support string geoshape in addition to reference Area and geofence functions: accept literal string argument and check for valid geoshape #769
• exception if first and last point don't match (I think we should explicitly only support single, closed polygons) Area and geofence functions: accept literal string argument and check for valid geoshape #769

[tiritea]

Todo:

* [ ]  support string geoshape in addition to reference

* [ ]  exception if first and last point don't match (I think we should explicitly only support single, closed polygons)

FYI I largely based the javarosa handling code on the existing area() function. So I think it would be good to keep these two aligned in the above regards. Both functions should therefore similarly support both raw string geoshape argument as well as a reference/nodeset. Likewise, both functions explicitly require a geoshape argument; this implies the first and last points are the same, there are at least 4 (or 3?) points, etc. It certainly doesnt do any harm performing some explicit checks for valid arguments [I'm not sure what the correct behavior should be if determined to be an invalid geoshape?...]. But, again, both these functions should probably behave the same around this.

[lognaturel]

Fine by me to track adding support for literal values and error checking in a separate issue that we do later for both area and geofence! If you’re up for doing the rebase that would be great.

[lognaturel]

Here's a simple testing form that works in pyxform today: https://docs.google.com/spreadsheets/d/1UKLC9ZBT5CdquUqmyMvf2Ofspl5IC2YRPBhV8ruo5bQ/edit#gid=1068911091

It does indeed get less accurate as the fence size gets bigger but we can certainly document those limitations.

[tiritea]

I'm about to lose my development environment while I transition jobs [I must return my current dev laptop and the new one is yet to arrive...] so I realistically wont get to look into rebasing anything till next week at the earliest.

[lognaturel]

Whoops, I didn't get my force push quite right since this work was done on master. I think maybe branch protection has kicked in. I pushed to a new branch so we can merge!