Convergence criterion for Lloyd's algorithm

[foxyseta]

Should solve #22. I picked relative maximum movement difference instead of absolute maximum movement difference. This way, I can pick a reasonable default tolerance value, since it is independent from the mesh's sizes.

All tests are still passing. The only compiler I get were already present in previous builds. They're about missing/incomplete XML comments.

Edits by maintainers are welcome!

[wo80]

The iteration won't start, due to the way you are initializing prevMax and currMax:

double a = Double.PositiveInfinity;
double b = Double.PositiveInfinity;
Math.Abs(b - a) <= 1.0 * b; // -> NaN <= Infinity == false

I suggest changing to a while loop and running the first iteration outside the loop to have a correct starting value. Something like:

public int Smooth(IMesh mesh, int limit = 10, double tol = .01)
{
    var smoothedMesh = (Mesh)mesh;

    var mesher = new GenericMesher(config);
    var predicates = config.Predicates();

    // The smoother should respect the mesh segment splitting behavior.
    this.options.SegmentSplitting = smoothedMesh.behavior.NoBisect;

    // The maximum distances moved from any site at the previous and
    // current iterations.
    double prevMax = double.PositiveInfinity;
    double currMax = Step(smoothedMesh, factory, predicates);

    // Take a few smoothing rounds (Lloyd's algorithm). The stopping
    // criteria are the maximum number of iterations and the convergence
    // criterion.
    int i = 1;
    while (i < limit && Math.Abs(currMax - prevMax) > tol * currMax)
    {
        prevMax = currMax;
        currMax = Step(smoothedMesh, factory, predicates);

        // Actually, we only want to rebuild, if the mesh is no longer
        // Delaunay. Flipping edges could be the right choice instead 
        // of re-triangulating...
        smoothedMesh = (Mesh)mesher.Triangulate(Rebuild(smoothedMesh), options);

        factory.Reset();

        i++;
    }

    smoothedMesh.CopyTo((Mesh)mesh);

    return i;
}

Other changes proposed:

• Remove ISmoother interface and have only one Smooth method with default parameters (no DEFAULT_TOL needed)
• Return number of iterations taken.
• Correct spelling errors (creteria, criterium, maximumum)

[foxyseta]

Whoops, sorry, silly mistake of mine. Apparently running the tests wasn't enough. Since I'm returning the number of iterations now, should I check them in the unit tests?

[wo80]

Since I'm returning the number of iterations now, should I check them in the unit tests?

Yes.

Though I guess it's fair to assume that if a user calls Smooth, he/she want's to do smoothing, I think we should check for limit <= 0 and return 0 if true. Then you can test for number of iterations > 0 in SimpleSmootherTest.cs

[foxyseta]

I guess it's fair to assume that if a user calls Smooth, he/she want's to do smoothing, I think we should check for limit <= 0 and return 0 if true.

So, in this scenario, asking for a non-positive number of iterations would result in 0 iterations begin performed (and 0 being returned), right?

[wo80]

Correct. It has no cost and is more consistent than doing one iteration though none was requested.

[wo80]

The Smooth methods <returns> comment needs an update. Then we're ready to merge I guess. 👍

[foxyseta]

Hopefully the wording is clear.

[wo80]

Thanks a bunch, your contribution is very much appreciated!

I'll do a squash and merge since it's a single feature, affecting a single file.