OpenJSCAD.org: Problems with use of Math.Floor and with comparisons of inexact data.

General Bug Report

I recently experiment with making my CSharp port of JSCAD using single precision arithmetic. (This was in hopes of using some special Microsoft features that would speed things like Transformations, but only used Single precision.) This experiment failed, but it did uncover some general bugs in JSCAD. These all fall into the realm of side effects of floating point precision.

I’ll start with a detailed explanation of the first issue I encountered. This was AFTER the cos/sin (with rezero) changes that were recently made. I have removed those changes from my code base, this new way seems more robust and the rezero functions are no longer needed. So I immediately hit three bugs in ellipse.cs… I’m showing you what it would look like in ellipse.js. The first one (line 58) is a totally inappropriate use of Math.floor. The problem is simple… I was getting a 4.9999… value that was getting chopped to 4. The others are basically incorrect comparisons of a calculated value (thus inprecision) with Math.PI*2. Thus I created the “Equalish” routines. Basically before doing a floor it checks to see if the value is “Equalish” to the ceiling, choose Ceiling if it is, otherwise it does the floor. Likewise lessThanish… first checks “Equalish” then if it is not Equalish it does the < comparison. I have include my CSharp versions of these routines at the end. Also a complete list of the changes I made in the primitives and extrusions… I’m sure I’ll find more and will let you know when I do.

ellipse.js: 58
segments = Math.floor(segments * (rotation / (Math.PI * 2)))
segments = floorish(segments * (rotation / (Math.PI * 2)))
--
ellipse.js: 64
segments = (rotation < Math.PI * 2) ? segments + 1 : segments
segments = lessThanish(rotation, Math.PI * 2) ? segments + 1 : segments
--
ellipse.js: 71
if (rotation < Math.PI * 2) points.push(centerv)
if (lessThanish(rotation, Math.PI * 2)) points.push(centerv)

— Here’s the complete list of the “ish” changes’ I’ve made so far (translated to javasscript and your current line numbers.) — Note, I just discovered some changes had been made to extrudeRotate since I last pulled. line might be off a few… please let me know if I need to disambiguate!—

extrudeRotate.js: 50
if (totalRotation <= 0.0) totalRotation = (Math.PI * 2)
if (lessThanOrEqualish(totatalRotation, 0.0) totalRotation = (Math.PI * 2)
--
extrudeRotate.js: 55
segments = Math.floor(Math.abs(totalRotation) / anglePerSegment)
segments = floorish(Math.abs(totalRotation) / anglePerSegment)
--
cylinderElliptic.js: 67
const slices = Math.floor(segments * (rotation / (Math.PI * 2)))
const slices = floorish(segments * (rotation / (Math.PI * 2)))
--
cylinderElliptic.js: 120
if (rotation < (Math.PI * 2)) {
if (lessThanish(rotation < (Math.PI * 2))) {
--
ellipse.js: 58
segments = Math.floor(segments * (rotation / (Math.PI * 2)))
segments = floorish(segments * (rotation / (Math.PI * 2)))
--
ellipse.js: 64
segments = (rotation < Math.PI * 2) ? segments + 1 : segments
segments = lessThanish(rotation, Math.PI * 2) ? segments + 1 : segments
--
ellipse.js: 71
if (rotation < Math.PI * 2) points.push(centerv)
if (lessThanish(rotation, Math.PI * 2)) points.push(centerv)
--
roundedCylinder.js: 64
const qsegments = Math.floor(0.25 * segments)
const qsegments = floorish(0.25 * segments)
--
// My best guess on this one is that they had the "floorish" sort of issue and used
// Math.round to "fix" it, but that really isn't the appropriate way.
// And it could fail on edge cases.
ellipsoid.js: 41
const qsegments = Math.round(segments / 4)
const qsegments = floorish(segments / 4)

— Below is my C# version of the routines… you shouldn’t have trouble making the JS version. — — The value C.EPS is 1e-5 which you generally reference as EPS everywhere. —

    public static bool Equalish(double a, double b, double epsilon = C.EPS)
    {
        if (a == b)
        { // shortcut, also handles infinities
            return true;
        }

        var absA = Math.Abs(a);
        var absB = Math.Abs(b);
        var diff = Math.Abs(a - b);
        if (double.IsNaN(diff))
        {
            throw new ValidationException("Should never have NaN.");
        }
        return diff <= epsilon;
    }

    public static bool GreaterThanOrEqualish(double a, double b, double epsilon = C.EPS) {
        if (Equalish(a, b, epsilon)) {
            return true;
        }
        return a >= b;
    }

    public static bool GreaterThanish(double a, double b, double epsilon = C.EPS) {
        if (Equalish(a, b, epsilon)) {
            return false;
        }
        return a > b;
    }

    public static bool LessThanOrEqualish(double a, double b, double epsilon = C.EPS) {
        if (Equalish(a, b, epsilon)) {
            return true;
        }
        return a <= b;
    }

    public static bool LessThanish(double a, double b, double epsilon = C.EPS) {
        if (Equalish(a, b, epsilon)) {
            return false;
        }
        return a < b;
    }

    public static int Floorish(double a, double epsilon = C.EPS)
    {
        var a_ceil = Math.Ceiling(a);
        if (Equalish(a, a_ceil, epsilon)) {
            return (int)a_ceil;
        }
        return (int)Math.Floor(a);
    }