Tolerance in 1.0

[rubendel]

When implementing the tolerance check as described in https://github.com/buildingSMART/IDS/blob/development/Documentation/ImplementersDocumentation/tolerance.md a lot of the tolerance test cases fail (12). When modifying the logic a little bit (changing the > and < to inclusive comparators, >= and <=) the amount of errors goes down to only 2. So I am wondering whether the test cases are in line with the documentation for 1.0.

The remaining two test cases are:

• pass-comparison_tolerance_for_floating_point_negative_one_lower_bound (testing -1.0000020000 against -1.)
• pass-comparison_tolerance_for_floating_point_one_upper_bound (testing 1.000002 against 1.)

Based on their names these should pass, but they do not, tested in both a Java and a Python implementation.

[andyward]

These two tests are pretty much the canonical example we use for Real tolerance in IDS: (real values between 0.999998 and 1.000002 satisfy 1.0). So they should pass.

I'd be debugging the implementation. Another issue might be that the values are being treated as float rather than doubles?

For reference this is xbim's implementation which does pass these tests: https://github.com/CBenghi/Xbim.Xids/blob/main/Xbim.InformationSpecifications/Values/ValueConstraint.cs#L582

[rubendel]

Thank you for your response Andy. Looking at https://github.com/CBenghi/Xbim.Xids/blob/main/Xbim.InformationSpecifications/Values/ValueConstraint.cs#L593C11-L593C62 it seems your implementation is using >= and <= for the comparison too, which is not in line with the documentation:

x == v ⇒ (v - abs(v) × ϵ - ϵ) < x < (v + abs(v) × ϵ + ϵ)

So that kind of answers my first question, at least one other implementation is assuming >= and <= should be used :).

I am using double precision for all values/calculations.

It looks like the rounding to 15 digits is what makes the tests pass in your case, but that's not really something I have found in the IDS documentation, perhaps I am not looking in the correct places. It took me some time also to figure out the correct type of rounding (UP, DOWN, CEILING, FLOOR, HALF_UP, HALF_DOWN, HALF_EVEN), turns out that the .Net function you are calling is using HALF_EVEN by default.

My suggestions (I'll do a pull request for them after some positive responses to this issue):

• Update the forumula in the documentation to use <= and >=, assuming that is the intention
• Add information to the documentation about the rounding to 15 decimals and the rounding method

[andyward]

My suggestions (I'll do a pull request for them after some positive responses to this issue):

• Update the forumula in the documentation to use <= and >=, assuming that is the intention

Yes, I suspect when these 'boundary' test cases were created there's actually been a subtle change to whether it's inclusive or exclusive. Inclusive is probably conceptually simpler to communicate?

• Add information to the documentation about the rounding to 15 decimals and the rounding method

I think the reason we added the rounding is that these Real tolerance tests are also affected by inherent imprecision in floating point numbers in the implementator's tech platform - i.e basically the same issue we're mitigating for when testing data for in the first place.

E.g. in your first test case IFCREAL(-1.0000020000) as an IEEE 64 bit double is actually something like -1.00000019999999989473. As these are the boundary case tests, depending on direction the fraction error goes in you'll fail some tests. Rounding to 15 decimals seemed a pragmatic approach to tolerating this albeit at the expense of being able to test values < 1e-15

[rubendel]

Yes I think inclusive comparisons are easier to communicate/understand/implement, and since the test cases already interpret it like this I think it's just the documentation that needs updating.

Personally I would prefer to skip the rounding to 15 decimals and have test cases that pass/fail based on IEEE 64 bit double implementations, since that standard is wel defined, referred to in the documentation and implemented by all major programming languages. It would make implementation by others more straightforward I think. Since IDS 1.0 is final, my proposal would be to do that in a newer version.