Optimise line locate point

[AndrewLipscomb]

• [*] I agree to follow the project's code of conduct.
• I added an entry to CHANGES.md if knowledge of this change could be valuable to users.

---

See #1320

Two things here - first is optimising the LineLocatePoint algorithm.

A new bench shows this was the original timing

LineString line_locate_point
                        time:   [259.34 µs 259.57 µs 259.83 µs]
                        change: [-1.6333% -1.0309% -0.5978%] (p = 0.00 < 0.05)
                        Change within noise threshold.
Found 6 outliers among 100 measurements (6.00%)
  5 (5.00%) high mild
  1 (1.00%) high severe

After updates

LineString line_locate_point
                        time:   [73.201 µs 73.534 µs 74.106 µs]
                        change: [-71.777% -71.684% -71.525%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
  2 (2.00%) high mild
  1 (1.00%) high severe

Great success, very nice.

I've put some of this work out publicly under a public ComparableDistance trait. However I don't think that's necessarily the best idea.

I started off just working off of Boost Geometry's comparable_distance idea - but I think given

• This will be at least strictly Euclidean to start, which implies all "comparable distances" will just be the non-sqrt'ed versions which is kind of handy to know
• I have no idea what a ComparableDistance trait would look like for non-Euclidean spaces

I think it would be better exposed as an explicit DistanceSquared trait that only Euclidean implements - if I am writing my own algo's I'd like to have it reliably convertible to a real distance.

Keen for some feedback here - I still have some clean up here before I mark this as ready to go.

[michaelkirk]

This seems great! No notes. 👏👏 👏

I think it would be better exposed as an explicit DistanceSquared trait that only Euclidean implements - if I am writing my own algo's I'd like to have it reliably convertible to a real distance.

That makes sense to me. Though I'm also not super bothered by having a ComparableDistance trait which is currently only implemented for Euclidean.

[AndrewLipscomb]

Just FYI - I've gotten bogged down with work this week. Still aiming to fix this up but will need a weekend to do it

[urschrei]

No rush whatsoever!

[bennjii]

Awesome work @AndrewLipscomb, is there anything I can do to help this PR along? Happy to have a raise a PR in your fork to resolve those CI failures.

[michaelkirk]

This seems great and results in a nice speedup. We should merge it soon.

I know you've expressed a concern about the naming of the trait - I don't particularly share that concern. I'm fine either way.

If you have time to make those changes soon, I'll re-review, otherwise l'll fix up the docs and merge in a couple days.

[michaelkirk]

This comment seems spurious (maybe it was copied from the Distance trait which was implemented on more than just the Euclidean metric space).

[bennjii]

I've made fixes for the CI errors in AndrewLipscomb#1 if that helps :)

[michaelkirk]

I know this is an ancient PR (sorry it sat here so long), but I think it'd be good to have.

[michaelkirk]

Usually this is the same, and it's DRY to use our new comparable distance method here, but the old impl, using hypot should be more stable.