Handle shorter reference curves (in Rectangle algorithm only)

[beutlich]

This is my attempt to fix #65. Not sure if good enough, but it fixes the test case.

FYI @casella @GallLeo

[casella]

Eventually I hope we'll have some annotation to specify shorter simulations for verification and testing, so this may no longer be necessary in the future, but in the meantime this is the only way we have to handle chaotic system verification properly.

[uschna]

Dear @beutlich, I must remember the sense of the code. After that the most changes are clear. Only some remarks:

I don't know, why

using System.Linq;
using System.Text;

is (not) needed.

I see, you use in https://github.com/modelica-tools/csv-compare/blob/master/Modelica_ResultCompare/CsvFile.cs reference.X[reference.X.Length - 1] as last value, in https://github.com/modelica-tools/csv-compare/blob/master/Modelica_ResultCompare/CurveCompare/Algorithms/Rectangle.cs is the last value reference.X[reference.Count - 1]

I see, [minX, maxX] is the minimal interval of reference and compareCurve, the interval for the lower and upper tube curve before your changes is [reference.X[0] - size.X, reference.X[reference.Count - 1] + size.X] and you delete all points not in the interval [minX - size.X / 2.0, maxX + size.X / 2.0] At first, I'm not sure: If compareCurve.X[0] >= reference.X[0] and compareCurve.X[compareCurve.X.Length - 1] <= reference.X[reference.X.Length - 1] nothing should be changed. Then we can use the interval [reference.X[0] - size.X, reference.X[reference.X.Length - 1] + size.X] or [reference.X[0], reference.X[reference.X.Length - 1]] In case of compareCurve.X[0] < reference.X[0] or compareCurve.X[compareCurve.X.Length - 1] > reference.X[reference.X.Length - 1] and 1 point is in the interval of the lower and upper tube curve but not in the interval of the reference curve then it would be better to take the interval [reference.X[0], reference.X[reference.X.Length - 1]] At second, I would take minX and maxX at begin of the algorithm in CalculateLower and CalculateUpper or, if this is to complicated, at end add (for lower tube curve)

if(LX[LX.Count - 1] < maxX)
{
LX.Add(maxX);
LY.Add(LY[LY.Count - 1]);
}

and for LX[0] analogously

if(LX[0] > minX)
{
LX.Insert(0, minX);
LY.Insert(LY[0]);
}

and also for the upper tube curve. Or better, we begin and end with a half rectangular, that means, begin with LX.Add(reference.X[b]); / UX.Add(reference.X[b]); instead of LX.Add(reference.X[b] - size.X); / UX.Add(reference.X[b] - size.X); and end with LX.Add(reference.X[reference.Count - 1]); / UX.Add(reference.X[reference.Count - 1]); instead of LX.Add(reference.X[reference.Count - 1] + size.X); / UX.Add(reference.X[reference.Count - 1] + size.X);

Then we don't need minX and maxX.

[beutlich]

Or better, we begin and end with a half rectangular, that means, begin with

Doing so, results in index exceptions in the RemoveLoop algorithm

while ([j + 1] < X[i - 1]) // X[j + 1] < X[i - 1] => (i - 1 > 0 && Y[i - 2] <= Y[i - 1]) (in case of lower)
    i--;

and (if avoided by not letting become i < 1) in strange tubes at maxX, for example

instead of

Even the second chart (this pull request) is not good in all cases because

• the upper tube abscissa value exceeds maxX (being 0.4)
• the lower tube ordinate value should probably not be > -20.

Handling of begin and end tube is rather tricky. For me it is not straight-forward to constain the tubes between minX and maxX in an obvious manner. I did not check the RemoveLoop algorithm.

[uschna]

Dear @beutlich, yesterday in the evening, I found out that it is not so easy. But I found a solution: As I remarked: We can start with LX.Add(reference.X[b]); / UX.Add(reference.X[b]); instead of LX.Add(reference.X[b] - size.X); / UX.Add(reference.X[b] - size.X); and end with LX.Add(reference.X[reference.Count - 1]); / UX.Add(reference.X[reference.Count - 1]); instead of LX.Add(reference.X[reference.Count - 1] + size.X); / UX.Add(reference.X[reference.Count - 1] + size.X); and don't need minX and maxX. The problem are the related first ([0]) and last ([*Y.Count - 1]) LY- and UY-Values:

LY[0]: Find all reference.X[i] with reference.X[0] <= reference.X[i] <= reference.X[0]+size.X and (usually per interpolation): reference.X[0]+size.X compute the minimum of the related reference.Y and compute - size.Y from this.

UY[0]: Find all reference.X[i] with reference.X[0] <= reference.X[i] <= reference.X[0]+size.X and (usually per interpolation): reference.X[0]+size.X compute the maximum of the related reference.Y and add size.Y to this.

LY[LY.Count - 1]: Find all reference.X[i] with reference.X[reference.Count - 1]-size.X <= reference.X[i] <= reference.X[reference.Count - 1] and (usually per interpolation): reference.X[reference.Count - 1]-size.X compute the minimum of the related reference.Y and compute - size.Y from this.

UY[UY.Count - 1]: Find all reference.X[i] with reference.X[reference.Count - 1]-size.X <= reference.X[i] <= reference.X[reference.Count - 1] and (usually per interpolation): reference.X[reference.Count - 1]-size.X compute the maximum of the related reference.Y and add size.Y to this.

You get the same solution if you make your Remove-algorithm with reference.X[0] instead of minX - size.X / 2.0 and reference.X[reference.Count - 1] instead of maxX + size.X / 2.0 and interpolate the values of LY and UY at these two points and replace the last removed points with these points.

[beutlich]

@uschna Thank you very much for your review comment. I updated this pull request with your idea, see f3c399ee0058b086fdaa7792060664bc663813f5

The example chart now looks as follows

For LY[0] and UY[0] should we take the the interval [ reference[0], reference[0] + size.X ] into account when calculating the new minY / maxY values to gain a more relaxed tube at the left boundary? And similar for the right boundary of course?

[uschna]

Dear @beutlich, if I understand your English answer correctly, you did not understand me correctly. Yes, it is correct:

For LY[0] and UY[0] should we take the the interval [ reference[0], reference[0] + size.X ] into account when calculating the new minY / maxY values to ... (get a better) tube at the left boundary? And similar for the right boundary of course?

But this relates to reference.Y, not to LY or UY. I suggest that you cut the two curves at reference.X[0] and reference.X[reference.Count - 1] I see this difference in the code https://github.com/modelica-tools/csv-compare/pull/73/files If the value of LY at reference.X[0] or reference.X[reference.Count - 1] is smaller than minY, then the first / last value is to big. If the value of LY is bigger, then the first / last value is to small. If the value of UY at reference.X[0] or reference.X[reference.Count - 1] is bigger than maxY, then the first / last value is to small. If the value of UY is smaller, then the first / last value is to big. If I understand https://github.com/modelica-tools/csv-compare/issues/65 correctly this is the case for BevelGear1D.revolute1.phi for UY at reference.X[reference.Count - 1]. The idea was not to calculate the minimum of the points of LY or maximum of the points of UY in the interval[reference[0] - size.X, reference[0]] or [reference.X[reference.Count - 1], reference.X[reference.Count - 1] + size.X]. Instead of

                double minY = reference.Y[0] - size.Y;
                while (LX.Count > 0 && LX[0] <= reference.X[0])
                {
                    minY = Math.Min(minY, LY[0]);
                    LX.RemoveAt(0);
                    LY.RemoveAt(0);
                }
                LX.Insert(0, reference.X[0]);
                LY.Insert(0, minY);

I think:

while (LX.Count > 0 && LX[0] < reference.X[0]) // if (LX[0] == reference.X[0]): no RemoveAt and Insert are needed
{
if (LX.Count > 1 && LX[1] > reference.X[0])
{
double minY = LY[0]+(LY[1]-LY[0])*(reference.X[0]-LX[0])/(LX[1]-LX[0]);
LX.RemoveAt(0);
LY.RemoveAt(0);
LX.Insert(0, reference.X[0]);
LY.Insert(0, minY);
}
else
{
LX.RemoveAt(0);
LY.RemoveAt(0);
}
}

I do not know, if there is a replace-function instead of RemoveAtand Insert.

[beutlich]

@uschna Thanks for the reply. I updated this pull request according to your comment. Here's how it looks this time:

[uschna]

Dear @beutlich, thank you. This is also possible. But why did you use for UX reference.X[0] + size.X instead of reference.X[0]?

[beutlich]

But why did you use for UX reference.X[0] + size.X instead of reference.X[0]?

Good catch. This was a leftover from my experiments and just got reverted.

[casella]

Great!

@beutlich as soon as you merge this, @GallLeo will be able to use it for the new round of regression testing. If it works fine, I guess you could make a new minor release, what do you think?

[casella]

@GallLeo, this is now ready for use.

[casella]

Thanks @beutlich for taking care of this 👍

[beutlich]

@casella Note, that it still is experimental since it only was tested against the one test data provided by https://github.com/modelica-tools/csv-compare/pull/73#issue-2532016226