Esri/geometry-api-java

Question: TestOGCGeometryCollection.testUnionLinestring

paulushub opened this issue · 9 comments

Context: I have ported this library to .NET/C# and testing it. Currently, I have passed all tests except 5 (excluding TestWkbImportOnPostgresST, TestSerialization) and this one needs some clarification. Here is the test:

assertUnion("LINESTRING (1 2, 3 4)", "LINESTRING (2 3, 2.1 3.1)",
	"LINESTRING (1 2, 2 3, 2.0999999999999996 3.0999999999999996, 3 4)");

How valid is this expected test result? The following is my failed test result:

Expected string length 65 but was 35. Strings differ at index 24.
Expected: "...STRING (1 2, 2 3, 2.0999999999999996 3.0999999999999996, 3 4)"
But was:  "...STRING (1 2, 2 3, 2.1 3.1, 3 4)"
----------------------------------^

In formatting (StringUtils), I cast the double to decimal type (straight forward in C#) and the output is more precise and does not require the later trimming of zeros method provided by the library.

@paulushub Those differences are insignificant. The only problem is that there is no 2.1 as double. Check that round-trip to string and back will produces equal geometry for you.

Those differences are insignificant.

True, but for unit tests, it makes a difference. May be asText should take a value that specifies the precision.

Why assertEquals on strings rather than assertTrue with Geometry.equals?

@paulushub When you explicitly assign 2.0999999999999996 to a double and convert that double to a string what do you get in your code?

@paulushub

May be asText should take a value that specifies the precision.

For that purpose you should use the geometry API itself, OperatorExportToWKT and specify precision in the exportFlags parameter. See WktExportFlags.

@randallwhitman

Why assertEquals on strings rather than assertTrue with Geometry.equals?

It does not matter in principle because Geometry.equals is an exact comparison.
If we had Geometry.equals(otherGeometry, 1e-12) with tolerance, then it would be more useful in the unit tests.

@paulushub When you explicitly assign 2.0999999999999996 to a double and convert that double
to a string what do you get in your code?

Using the following codes

double value1 = 2.0999999999999996;
double value2 = 2.1;

int precision = 17;

string format = "{0:f" + precision + "}";

Console.WriteLine("Using Format: " + format);
Console.WriteLine(string.Format(CultureInfo.InvariantCulture, format, value1));
Console.WriteLine(string.Format(CultureInfo.InvariantCulture, format, (decimal)value1));
Console.WriteLine(string.Format(CultureInfo.InvariantCulture, format, value2));
Console.WriteLine(string.Format(CultureInfo.InvariantCulture, format, (decimal)value2));


format = "{0:g" + precision + "}";

Console.WriteLine();
Console.WriteLine("Using Format: " + format);
Console.WriteLine(string.Format(CultureInfo.InvariantCulture, format, value1));
Console.WriteLine(string.Format(CultureInfo.InvariantCulture, format, (decimal)value1));
Console.WriteLine(string.Format(CultureInfo.InvariantCulture, format, value2));
Console.WriteLine(string.Format(CultureInfo.InvariantCulture, format, (decimal)value2));

You get the following results:

Using Format: {0:f17}
2.10000000000000000
2.10000000000000000
2.10000000000000000
2.10000000000000000

Using Format: {0:g17}
2.0999999999999996
2.1
2.1000000000000001
2.1

Ok. It seems that casting 2.0999999999999996 to the decimal and back to double adds a slight change. We try to have a round trip to WKT without change in values.

Otherwise if you see slight differences in the unit tests between c# and java, it is completely fine. I don't think you can expect different compilers to produce same floating point results. I've even seen differences when running exactly same binary on different processor architectures (in c++ though) because compilers may have compiled different execution paths for different processors.

@stolstov The differences is not really an issue, but the form of testing. I feel the results should be consistent and independent of the language used.

I think you added the RemoveTrailingZeros_ method for this purpose, but this will not have any effect on formatted text result like 2.0999999999999996 or 2.1000000000000001.
I added the cast to decimal before formatting to ensure consistent results.

Thanks for the time and the discussion, I really appreciate it.