Bugfix: Remove unintended view distance limit

[patowen]

The math::distance normalizes the input vectors as part of its computation, which results in a loss of precision when an input vector is large.

For situations where that matters, such as in traversal.rs, we use math::mip instead of math::distance. Note that, when the input vectors are already pre-normalized, the distance formula simplifies to (-mip(a, b)).acosh(), which is likely small enough to not need its own function.

Note that the actual bug this fixes is that distance computations in traversal::ensure_nearby were resulting in NaN, which caused the loop to continue on forever, continuing to expand out the graph until running out of memory.

[Ralith]

This makes me really want the stronger math types we've discussed before, so we can have distance take pre-normalized inputs and let callers normalize or not as needed.

[Ralith]

the actual bug this fixes is that distance computations in traversal::ensure_nearby were resulting in NaN

Should we add an assert or a unit test to catch this more easily in the future?

[patowen]

the actual bug this fixes is that distance computations in traversal::ensure_nearby were resulting in NaN

Should we add an assert or a unit test to catch this more easily in the future?

Perhaps, although that is nontrivial. An assert could be refactored away, and a unit test would cause the tests to exit non-gracefully if they fail (and potentially make GitHub mad at us for hogging a lot of RAM).

That being said, I think I agree with going the unit test route. That will cause such regressions to reliably be caught in CI, even if the way they're caught is not ideal (and I assume GitHub is smart about the resource pool allocations).