Freedom-of-Form-Foundation / anatomy3d

A CAD tool for humanoid anatomy alterations. See the anatomy3d-blender repository for more recent work.
https://freedomofform.org/1856/3d-anatomy-project-scope-phase-1-focus-on-a-limb-joint/
GNU General Public License v2.0
7 stars 5 forks source link

Improve "close enough" test for numerical equality #40

Closed zennithn closed 3 years ago

zennithn commented 3 years ago

Just in case this hasn't been discussed yet on GitHub - I had an idea that follows on the intent of comparing numerical values for being "close enough" within some tolerance of floating point error. The idea is basically just that the permitted error should be relatively tight for comparing pairs of small floating-point values, and scale to be relatively larger for larger floating-point values. So, perhaps the permitted error should be a % or other factor that scales with the values.

I believe the most formally correct way to do it would be to take the geometric average of both values, and then permit some geometric upper-bound and a matching geometric lower-bound.

However, a suitable way to do it should just be to take the arithmetic average of both values, and define the upper and lower bounds as some percentage of that average.

Based on a quick glance at anatomy3d/engine/Real.cs -> Near(), there does seem to be a difference from existing implementation, which appears to only consider a set slop value. However, if this has already been addressed somewhere in the code, sorry!

Also, if this is useless minutiae, please ignore!

AdamNorberg commented 3 years ago

What you're looking for is Real.IsRelativelyNear(Real, Real): https://github.com/Freedom-of-Form-Foundation/anatomy3d/blob/57fa59c7f436d983d59cfa02539a1b126e8265f6/engine/Real.cs#L492

The current implementation of Near (for testing) uses IsNear, which is the "most lax" implementation of a nearness check - it matches if relative or absolute error is within the specified range. Lathreas' code relies on special handling for values near zero, using absolute threshholds, so I felt like allowing an absolute amount of error as well as a relative amount was more in keeping with the rest of the code. It's easy enough to swap out if we'd like to use a different standard.

zennithn commented 3 years ago

Got it. I think that sounds good enough, thanks for the explanation!