Making operations generic over CoordFloat

[ianthetechie]

This library is not currently usable unless your geometry uses f64 coordinates.

It's arguable whether most projects should use f32 (IIUC most CPUs will still use 64-bit registers for 32-bit floating point math), but I think there is some value in considering support. If you are working with extremely large datasets in memory and don't need millimeter-level precision, f32 can save a ton of RAM, enabling you to work with even larger datasets. It also might enable better better SIMD optimizations in the future, since you can pack twice as much data in registers.

I'm opening this as a draft PR since there are a few open questions, including whether this is even a welcome change :) This is just a first whack at a working implementation that shows it's possible, but we probably want to do the following before merge:

• Performance testing to make sure this doesn't regress (shouldn't, but want to make sure). I see there are criterion benchmarks, but it doesn't look like they actually run in CI? Is there a strategy for approaching things like this within the project already?
• Figure out a better approach to testing than copy+paste. I can think of a macro that generates tests for multiple types... ideas welcome.

[michaelkirk]

Yikes, sorry this got lost for so long! If you're still interested in seeing it through, I think it would be reasonable to add.

Performance testing

There are some benchmarks. At least historically, I've found GH actions to be too noisy to be useful, so I typically just run a before/after benchmark locally and post the results in the PR. Very lo-fi, I know.

Figure out a better approach to testing than copy+paste.

My intuition is that duplicating the entire test suite and running it twice, once for f32 and again for f64 is over-testing. Instead, what are the interesting edge cases using f32 vs using f64? Maybe something brushing against numerical limits. Those are the only things we should be testing in my opinion, plus maybe a happy path test case. Others might feel differently though.

e.g. this isn't an interesting test wrt f32, and probably does not need to be duplicated: https://github.com/georust/polyline/pull/53/files#diff-b1a35a68f14e696205874893c07fd24fdb88882b47c23cc0e0c80a30c7d53759R326

[michaelkirk]

personal preference/nit: leave out the version bump, but please do add a BREAKING change log release.

It's too hard to remember if versions have already been bumped by someone else in another PR since release, so we just bump upon release, depending on what's in the changelog.

[ianthetechie]

Thanks for the review! No worries; GH notifications always get lost for me too...

I will revisit this once I get some time (should be fairly soon). I am indeed still interested in seeing this through, as for some large scale applications where the precision of f64 isn't necessarily better than the margin of human error anyways, the perf gains might be worth it.