-
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
Refactor distances between geometries #98
Comments
What should Also, from a naming perspective, why was |
For example, I could have seen |
Exactly. The distance between the point and the farthest point in the geometry.
I tried to follow the Distances.jl API so that whenever we pass points as inputs we can call |
Here are two cents from my perspective:
|
All very good points @lassepe. Can you submit a PR to each item separately so that we can more easily review and brainstorm? That would be great. There are lot of breaking changes, and we want to make sure we are not missing anything in the transition. |
In view of @juliohm's comment in #108
I would argue that we should not implement for M in (Distances.metrics..., Distances.weightedmetrics...)
@eval @inline (dist::$M)(a, b) = dist(coordinates(a), coordinates(b))
end I think this is not desirable because the tuples (dist::PreMetric)(a::Point, b::Point) = dist(coordinates(a), coordinates(b)) However, this inevitably creates an ambiguity error since the Therefore, I would suggest to deprecate |
Instead of thinking in terms of distance metrics, maybe we should think in terms of the underlying geometric objects involved. For example, the minimum distance between two geometries is the length of the arc connecting these geometries. In a Euclidean space, this arc is a Based on this viewpoint, we should consider addressing this issue in steps:
What do you think? |
I like the idea, but there are situations where such an arc isn't unique. I don't know if it would be guaranteed to exist either. In general, I am quite uncomfortable with the idea of discussing We can then have something like my_distance = Euclidean()
my_distance(p, nearest(p, g, distance)) The drawback is, we lose the default option for |
Very good thoughts as usual @serenity4 and I fully support the idea of encoding somehow the metric space as a type that we can pass to algorithms. I wonder if we should open a separate issue to discuss this metric space representation before we come back to this issue on distances. We could also discuss it here directly since distances and metric spaces are intimately connected. I started thinking about the |
In the picture I linked above, if you take a Manhattan/taxicab metric, you have multiple shortest paths (all red/orange/blue paths yield the same distance). It's a figure on a discrete domain, but in the continuous case it's the same; just that you have an infinite number of shortest paths. Perhaps you could first compute one possible shortest arc (even if multiple may exist) according to a heuristic, and take the measure of that arc to return a distance; I think that's doable. But maybe we'd have more direct formulas rather than trying to figure out this arc in the first place? I am also wondering whether we should use Distances.jl or reimplement metrics. The library has been defined to work very well for computations with iterators as was pointed out in this issue (it's like most of the library really), and (correct me if I'm wrong) we don't need that. The small effort we would spend in reimplementing their core types/formulas for points could be balanced out with not having to interface with this external library and having plain control over the type hierarchy.
From my point of view, having a way to define a metric space first is the more natural way. Once we have access to a metric, no need to ask for a metric parameter for evaluating distances. If one wants to do geometry processing in Euclidean space and then compute e.g. a spherical distance at some time, he/she should just construct another metric (e.g. from Distances.jl or Meshes.jl if we reimplement them) and use it directly. I think it's best to open a separate issue, for there will surely be some brainstorming required. |
FWIW: I ended up drafting the |
Yes, that's not going to be easy. If we just want distances it's fine, but otherwise it is more involved. Hopefully we'll be able to rely on primitives for the most part where solutions are easy to find. Just as a quick note for later, we may want to define something like |
Currently we have a single
evaluate(distance, g1, g2)
but as pointed out by @lassepe over Zulip, we could refactor this into at least two types of distances between geometries:mindistance
andmaxdistance
. We could also consider Hausdorff distances in the future.I think we can preserve
evaluate(distance, p1, p2)
wherep1
andp2
are points, and also provide thecolwise
andpairwise
variants like in Distances.jl.The text was updated successfully, but these errors were encountered: