-
Notifications
You must be signed in to change notification settings - Fork 87
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Algorithms from geodesic geometry #989
Comments
Thanks for catching these missing methods. Please feel free to submit a PR. We can merge and release a patch quickly. |
@disberd please keep in mind that some methods aren't supposed to exist, for example |
Are there any other methods missing after this broadcast is applied? |
@juliohm my use case is to verify whether a given point is inside any of a subset of countries (so a subdomain out of the full dmn). Given that Is there a better approach to achieve what I want above? That being said |
any(geom -> point ∈ geom, dom) should do it right? The only methods for |
It works for me here: triangles = rand(Triangle, 100) |> Shadow("xy")
point = rand(Point) |> Shadow("xy")
point .∈ triangles |
But this is already what happens by default with |
Yes the Meshes.jl/src/predicates/in.jl Line 10 in e959de5
which apparently transforms the |
Oh I see, so we are missing a method of |
I will think about this specific method, to see if it makes sense to add a method to |
The problem is that we are restricting the method currently to Euclidean polygons: Meshes.jl/src/predicates/in.jl Line 113 in e959de5
Should this method get relaxed to also support curved polygons over the ellipsoid? I understand that your use case involves points with |
We need to think carefully about these geodesic variants later. Maybe the easiest thing to do now is to |
This is the current situation:
Meshes.jl/src/predicates/in.jl Line 160 in 097754c
Meshes.jl/src/predicates/in.jl Line 113 in e959de5
When you call Meshes.jl/src/predicates/in.jl Line 10 in 097754c
We need to figure out the correct solution here. We plan to look into geodesic geometry after we finish polishing the Euclidean geometry algorithms. |
I see, on my side it is fine to just keep getting the error for Point(LatLon) until we have some geodesic computations in place to be consistent. A potential fallback till then is to call In any case, I understand if you want this to error for LatLon (though it should probably then error for every type of polygon (even without holes) for consistency. (For my use case I can just flatten the coords myself before checking for inclusion, knowing I am doing approximation since distance on E{2} is not the same as distance on ellipsoid) |
An alternative method consists of converting flatpoint = point |> Proj(PlateCarree)
flattable = geotable |> Proj(PlateCarree)
flatpoint in flattable.geometry
That is a good point. We should probably throw an error when a geodesic geometry is given as input to the predicate. And then, when we are ready to start the work on geodesic algorithms, we can add methods one by one. |
I am trying to do some benchmarks to improve the performance of
sideof
(see #988), and I found method errors very early with the following example, probably caused by missing methods forsideof
:This throws a first error
which is caused by this fallback method not working when a single
Point
is provided:Meshes.jl/src/sideof.jl
Lines 161 to 168 in c0c9c2d
This can be easily fixed by creating a method for the single point that just wraps it in an iterable (here in the REPL for simplicity)
After fixing that method, another error appears though
which again seems to be caused by some missing method implemented
The text was updated successfully, but these errors were encountered: