GrowExpansion() typo?

[demic312]

By

for (eindex = 0; eindex < e.Length; eindex++)

did you mean

for (eindex = 0; eindex < elen; eindex++) ?

I know it's not used further. Doesn't affect anything else. I was just reading through the code to understand it better.

[thinkadd]

You mean in here?

public static int GrowExpansion(int elen, double[] e, double b, double[] h)
{
    double Q;
    int eindex;

    Q = b;
    for (eindex = 0; eindex < e.Length; eindex++)
    {
        TwoSum(Q, e[eindex], out Q, out h[eindex]);
    }
    h[eindex] = Q;
    return eindex + 1;
}

I believe not. e is an array, e.Length is the array length. for (eindex = 0; eindex < e.Length; eindex++) basically means for each element in the array.

update: I took a look at it. Turns out this is only used in the tests, and in the caller, elen is exactly e.Length:
var n = GrowExpansion_Checked(e.Length, e, b, h);

So the two versions are logically equivalent. However, my personal preference is to just use e.Length, saving the effort of looking at the caller to find out what elen is.

[govert]

I suspect it is a bug.
See this assert in how it is called:

RobustGeometry.NET/RobustGeometry.Test/Predicates.Test/ExpansionTests.cs

Line 34 in dde86e1

Assert.IsTrue(e.Length >= elen);

This is long ago for me, but as I remember it, e will be a kind of buffer, and elen is tracking how many elements we're actually using.

Also note the GrowLength is not used other than some tests.

RobustGeometry.NET/RobustGeometry/Predicates/ExactArithmetic.cs

Line 371 in dde86e1

// NOTE: This algorithm is not actually used further, but good for checking the S. paper.

It would be good to confirm with some test or scenario that breaks with the current code.
Also to review for similar mistakes.

However, for me this is not an active project.

[govert]

I do note that .Length does not appear elsewhere in that file.

[demic312]

as i understand if input elen and e.Length happen to be equal then it wouldn't matter. But if, say, e was the output of a zero elim expansion then elen wouldn't be the same as e.Length

[demic312]

i don't think any other methods use .Length

[demic312]

i think elen is the equivalent of e.Count in a List<>. a lot of zero elim expansion operations seem to return an array that's bigger than the number of components in that expansion because the arrays are initialized before we know how many components are zero (and therefore eliminated)