Add a TGGeometry extension for fast inexact geometric predicates #271
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Mar 4, 2025
asinghvi17
force-pushed
the
as/tg
branch
2 times, most recently
from
d51369e to
3f223aa
Compare
This stack of pull requests is managed by Graphite. Learn more about stacking. |
This was referenced Mar 4, 2025
rafaqz
reviewed
Mar 8, 2025
rafaqz
reviewed
Mar 8, 2025
| """ | ||
| struct GEOS | ||
| struct GEOS <: CLibraryPlanarAlgorithm # SingleManifoldAlgorithm{Planar} | ||
| manifold::Planar | ||
| params::NamedTuple |
There was a problem hiding this comment.
Is there a reason to have theses as type unstable?
There was a problem hiding this comment.
Probably not, but I'm not sure what the effect of parametrizing these will be on compile times, since every time you change:
- kwarg order
- number of kwargs
this might force a recompilation since it's not the "same type anymore"
I guess it needs to be tested...
maybe some @nospecializeinfer might help, but that makes user/dev level method definitions horrible.
There was a problem hiding this comment.
We can figure that out later I think, if this ever becomes an issue. (I don't think it will be since everything is passed directly to C anyway)
There was a problem hiding this comment.
Or these could hold AbstractDict or namedtuple, if compile time is an issue.
There was a problem hiding this comment.
I guess we could also do OrderedCollections.jl's LittleDict...that would be a good middle ground at the cost of an extra ~20ms of load time (on my machine)
This was referenced Mar 18, 2025
asinghvi17
added a commit
that referenced
this pull request
Apr 3, 2025
This was factored out of the "dev branch" #259 and contains the subset of changes that apply to GeometryOpsCore, for easier review. Child PRs: #271 (TGGeometry) -> #275 (AdaptivePredicates) -> #273 (clipping algorithm type) -> #274 (trees) - Use [StableTasks.jl](https://github.com/JuliaFolds2/StableTasks.jl) in apply and applyreduce - its type-stable tasks save us some allocations! - Remove `Base.@assume_effects` on the low level functions, which caused issues on Julia v1.11 and was probably incorrect anyway - Add an algorithm interface with an abstract supertype `Algorithm{M <: Manifold}`, as discussed in #247. Also adds an abstract Operator supertype and some discussion in code comments, but no implementation or interface surface there yet. - Split out `types.jl` into a directory `types` with a bunch of files in it, for ease of readability / docs / use. - (out of context change): refactor CI a bit for cleanliness. TODOs for later (not this PR): - [ ] Add a `format` method that takes in an incompletely specified algorithm and some geometry as input, and returns a completely specified algorithm. What does this mean? Imagine I call `GO.intersection(FosterHormannClipping(), geom1, geom2)`. That `FosterHormannClipping()` should get expanded to `FosterHormannClipping(AutoAlgorithm(), AutoAccelerator())`. Then, `format` will take `format(alg, args...)` and: - get the `crstrait` of the two geometries, scan for incompatibilities, assign the correct manifold to the algorithm (maybe warn or emit debug info) - if no geometries available, get the manifold via `best_manifold(::Algorithm)`. - maybe inflate the accelerator by checking `npoint` and later preparations to see what's most efficient, maybe not - depends on what we want!
asinghvi17
force-pushed
the
graphite-base/271
branch
from
f802c60 to
87c2c7b
Compare
asinghvi17
force-pushed
the
as/tg
branch
2 times, most recently
from
80ef751 to
08f7ef4
Compare
- allow arbitrary legend kws - import GeoFormatTypes, WellKnownGeometry, CoordinateTransformations, ProgressMeter - make USA benchmarks print better - add benchmarks for coverage union - It turns out JTS (and also GEOS) use a special purpose noder and algorithm for coverage unions. We should also implement that at some stage - but it's a bit tough without the OverlayNG methodology. Maybe we can get an AI to implement that here :D
These aren't run yet but it is nice to have the code, both to confirm that GeometryOps is correct, and to confirm that TGGeometry actually works.
All these files, translated to JSON, are now in TGGeometry.jl, along with a reader that we can copy from there at some point. So there's no reason to keep those files in here.
rafaqz
reviewed
Apr 16, 2025
rafaqz
reviewed
Apr 16, 2025
Co-authored-by: Rafael Schouten <[email protected]>
asinghvi17
referenced
this pull request
Apr 17, 2025
* using GOCore * Fix errors found in docs * Set the current module in the Literate source This way, docstrings are resolved respective to where the page is. * add TGGeometry to docs project * Update docs/src/api.md
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Another spin out from #259
Parent: #269
Child PRs: #275 (AdaptivePredicates) -> #273 (clipping algorithm type) -> #274 (trees)
TGGeometry.jl is a Julia wrapper around the tg C library for planar geometric predicates. It doesn't use exact computation, but the indexing ideas there are extremely efficient and I thought it would be nice to at least have it accessible.
It's ridiculously fast - 100 ns for point in polygon if you convert the geometry to a TGGeometry first.
Accessible by
predicate(GO.TG(), geom1, geom2)for all GI/Simple Features planar geometry.This also refactors GEOS to work under the algorithm interface as well as laying the foundation for a future hypothetical PROJ algorithm, that we could use for reproject or segmentize.
Note
I'm deliberately not testing TG along with GO in the operations since I don't want to deal with our tests, that require exact predicates, returning the wrong values from the inexact
tg. However, I should and will add some tests elsewhere. This PR will also be a good proving ground for the new extension interface should there be one.This is IMO ready for merge since TGGeometry.jl is tested and it doesn't impact users. I also don't want to advertise it, at least initially.