-
Notifications
You must be signed in to change notification settings - Fork 60
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Testcase "[PASS] Floating point numbers are compared with a 1e-6 tolerance 1-4/4" #78
Comments
For this one I used the 1+-1*1e-6 equation provided by @aothms - can confirm? |
#36 (comment) is the comment by @aothms which informed this testcase. Happy to modify but I'll need an expert to confirm the approach :) |
Exactly, we have to be adaptive to both large and small values (especially since we only use SI units, so the range from @Moult maybe it would be a bit more illustrative if we use large and small values in the test case, but obviously 42 will also do fine.. ;) |
Ok, "1e-6 tolerance" means "6 significant digits", if I understand aothom's statement correctly. I would say 6 significant digits are not enough. E.g. values from Swiss coordinate system has 10 significant digits for millimeter - and then we don't talk about round problems with float numbers yet. Or one centimeter in degress for geografic coordinate systems is something like 1E-9 °. So we need more significant digits. |
Oh, I see aothms answers in the meantime. But my statements seems to remain valid. |
That's definitely a valid comment. 6 is rather pessimistic, it's based on the number of significant digits that can safely survive a roundtrip as 32-bit float. (generally this is 7, but there are a handful of cases where only 6 can be preserved). We can also say, all implementations should use 64bit (I'm not aware of applications using 32bit, you'd generally get into tolerance issue on the geometry side. In that case we can choose any value <= 15. See https://en.cppreference.com/w/cpp/types/numeric_limits/digits10 |
Also note, we're not trying to solve actual construction tolerances, users can do that using minInclusive maxInclusive. We're just trying to fight rounding errors. |
I would say it would be good if you could define your own tolerances anyway, even if there was a default one.
maybe in some other way. Value and tolerance is in some ways a nicer representation than min/max limits. |
One suggestion from @CBenghi is to add reasonable tolerances to the measures table. |
An objection to this proposal: closed list of tolerances for measures do not work well for all contexts (e.g. when an ifcLenghtMeasure is used in the context of rail lenght or a screw thread, it would not be appropriate to use the same tolerance). Speaking with @aothms, I now understand that the proposed solution was more subtle that what I initially understood and works ad follows: The actual tolerance has two components:
The second one is significant for large values, while the first one kicks in for values around 0, as the following table demonstrates:
Maybe I was the only one that did not understand this, but I leave this here for future discussion. This solution seems sensible to me, but me must develop a reasonable spread of use cases to make sure that the two components work across the range of IDS use cases. |
I think some detail was lost in the way the solution was presented.
So there is a relative component and fixed component (for 0.) This means:
I don't think this can work. A length measure can still be the thickness of a screw thread or the length of a railway track.
I don't disagree, but I think that can wait for a next version if there is end user demand for it. |
using the solution presented by @aothms in all the 0.9.7 tollerance test they pass |
@andyward Sharp! Excellent that you bring this up. But I think there is a bigger problem hiding underneath this. For values under -1, the lowerbound is actually above the upper bound. So the problem I'd say is not so much that the absolute difference between the bounds touches 0, but that the signed difference between higher and lower is a linear function that crosses the x axis and becomes negative.
^ What do you think of this? |
I think we ended up just inverting the logic when testing -ve numbers, at least when testing for equality: XIDS Code Will have a look at whether your Abs() approach makes more sense. It will probably be clearer It gets more complex when you get into RangeConstraints as we found you need to account for whether it's the Min or Max you're apply the range tolerance to. I'm not even sure if this is sensible, but given a requirement that:
... obviously this will be satisfied by (100 - 1e-7), but should we expect this be satisfied by (100+1e-7)? Now what about Then you get similar fun and games with boundaries when testing -ve values ranges. I can see there may be real world use cases where a user may have range requirements where they expect real values with these precision artefacts to be handled. E.g. "LCA CO2e/kg/m2 should be <=600". 600.000001 would probably satisfy, as should a net -ve value. Do we need to consider for IDS v1.0? We had a stab at it. Out initial assumption on RangeConstraints is that applying any precision tolerance on Reals is only sensible when MinInclusive = true. 100.000001 can never be < 100. But this opens up the question about whether 99.999999998 is really < 100 bearing in mind it is within 1e-6 tolerance of 100. Bottom line, adding tolerance to Range checks opens up all kinds of interesting edge cases. Some tests and notes here in XIDS Tests. Just one example from the tests: // TODO: Consider semantic paradox where:
// 41.999958d satisfies being in the range 42-50 inclusive, but also satisfies being < 42 exclusive I guess the main question is: Before we finesse the exact tolerance algorithm, do we need to consider tolerance on ranges as well? |
An additional consideration there could be that maybe a range has been used by the end user to actually incorporate precision on a single value and then we do it again in the IDS implementation. Also, in general the inclusive/exclusive distinction really just doesn't work well on floating point, because the distinction between inclusive/exclusive is either infinitesimally small (real number line), really really tiny (an ieee754 float value) or somewhat significant (our tolerance padding). And then there are corner cases like negative and positive zero, where we have multiple ieee754 float value that compare equal? My inclination would be:
|
Yes this is what we're thinking. We stretch the upper/lower bounds slightly on ranges when the value being tested is Real. But if we do apply this to exclusive ranges don't we risk breaking some pretty fundamental user assumptions? Use case: "The value must be greater than 0 [and less than...]" i.e. If we stretch the bounds that would accept float values > -0.00001 (i.e. negative), as well as 0 itself. I suspect some folks would be 🤯! They may also be raising eyebrows that 0.00000001 would satisfy the requirement - but that's more understandable. I'm now thinking that Exclusive ranges should actually be shrunk, not stretched, to match the semantic intent of the requirement. So
My assumption is ranges are meant for real world tolerances. E.g. measures, costs, product performance characteristics, where a spread of values is permittable, but there is some real world minima, maxima that requires enforcing. This double-handling, of precision feels like a separate edge-case that should be handled by documentation & auditing rules. If we end up with lots of rules specifying 0.999999 < x < 1.000001 we've failed here - or the author is compensating unnecessarily since this is the exact FP issue we're discussing we hide from the spec writer? E.g. if a numeric RangeConstraint's spread is < [agreed-calculation] we should raise a warning in the audit tool @CBenghi ? |
Yeah, but in a way that issue still exists the other way around:
minExclusive > 0.0 would disallow 0.00000000001 if you shrink by the
precision factor. I don't think there are perfect answers to this. But I
can live with your suggestion of shrinking for min/maxExcl.
Sent from a mobile device, excuse my brevity. Kind regards, Thomas
Op do 21 mrt 2024 16:35 schreef Andy Ward ***@***.***>:
… Apply the tolerance padding as for single values, they slightly stretch
the domain of accepted values, better be a bit on the permissive side here
than on the restrictive side.
Yes this is what we're thinking. We stretch
<https://github.com/CBenghi/Xbim.Xids/blob/6f8c034f8244b9d5d7c6255a81a587cf662090d3/Xbim.InformationSpecifications/Values/RangeConstraint.cs#L132>
the upper/lower bounds slightly on ranges when the value being tested is
Real.
But if we do apply this to exclusive ranges don't we risk breaking some
pretty fundamental user assumptions? Use case: *"The value must be
greater than 0 [and less than...]"* i.e. <minExclusive value="0"/>
If we stretch the bounds that would accept float values > -0.00001 (i.e.
negative), as well as 0 itself. I suspect some folks would be 🤯! They may
also be raising eyebrows that 0.00000001 would satisfy the requirement -
but that's more understandable.
I'm now thinking that Exclusive ranges should actually be *shrunk*, not
*stretched*, to match the semantic intent of the requirement. So > 0
actually becomes > 1e-6; while Inclusive ranges are stretched - >= 0
becomes >= -1e-6. (using your proposed algorithm)
An additional consideration there could be that maybe a range has been
used by the end user to actually incorporate precision on a single value
and then we do it again in the IDS implementation.
My assumption is ranges are meant for real world tolerances. E.g.
measures, costs, product performance characteristics, where a spread of
values is permittable, but there is some real world minima, maxima that
requires enforcing.
This double-handling, of precision feels like a separate edge-case that
should be handled by documentation & auditing rules. If we end up with lots
of rules specifying 0.999999 < x < 1.000001 we've failed here - or the
author is compensating unnecessarily since this is the exact FP issue we're
discussing we hide from the spec writer?
E.g. if a numeric RangeConstraint's spread is < [agreed-calculation] we
should raise a warning in the audit tool @CBenghi
<https://github.com/CBenghi> ?
—
Reply to this email directly, view it on GitHub
<#78 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAILWV2W7JOVKHBSQI4U5XLYZL433AVCNFSM6AAAAAAQKPRVOCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMJSGY3TEOJWGE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Agreed. I'd probably make the argument that to a typical end-user a value of 0.00000000001 is to all intents and purposes 'zero'. We know that 999.9+ times in 1000 it's just an artificial artifact introduced by tooling using Floats for numbers, when they probably should have been using fixed precision decimals; or it's a derived measure based off FP numbers/geometry. And either way the requirement intent was for a meaningful positive number. |
Following the conversation in the call on 3rd May 2024 we are inviting all interested parties in producing a succinct proposal for the management of rounding that can be cover a variety of use cases. The proposal should address as many possible of the following criteria:
This should be accomplished with no changes to the current xsd schema; however, it is possible to take into consideration different behaviours depending on the contextual information such as:
Please write your proposal in a documentation markdown file and propose a PR targeting this issue. We will use the github review mechanism to come to an consensus in the coming weeks. Thanks for your contribution. |
The commit adds a new file "tolerance.md" to the Documentation directory. This file summarizes the discussions and comments made in issues buildingSMART#78 and buildingSMART#36 regarding tolerance in IDS
Hi all Interesting discussion so far. I encountered a scenario where the tolerance factor is necessary, as shown in the image. The IFC file was exported by a software, and the saved value is -8.744999999999999. Another approach would be to add and subtract a very small number. However, this approach also presents the issue of deciding how small the number should be. Regardless, I would greatly appreciate it if you could set the tolerance in the IDS file and ensure that it can handle negative values. |
I think the principal is that the IDS author doesn't need to specify a tolerance at all for this kind of precision issue - it needs an implementors agreement rather than an IDS schema change. i.e. the software doing the testing must allow for tolerance of these small amounts occurring on FP numbers. That exact agreement/algorithm is what we need to agree here e.g. in #294 |
Here's another proposal: what if the number was rounded to the same number of decimal places as what the IDS author specifies? E.g. if the author asks for a length of 1m, then |
I would not agree to this proposal - very difficult to explain to end users and potentially conflicting with standard implementations. In my understanding it would be best to keep IDS V1.0 as it is (1e-6 tolerance to avoid computational rounding errors in BIM authoring software), and add proper tolerance handling, e.g., using percentage or construction tolerances as fixed values (like +/- 2cm) in a future version. |
While I can see the initial logic, it feels like this going to raise as many problems as it solves?
Sounds like a big can of worms 😄 |
[PASS] Floating point numbers are compared with a 1e-6 tolerance 1/4
#23=IFCPROPERTYSINGLEVALUE('Foo',$,IFCREAL(42.000042),$);
is PASS with a 1e-4 tolerance but not with 1e-6 tolerance?Dito [PASS] Floating point numbers are compared with a 1e-6 tolerance 2/4
#23=IFCPROPERTYSINGLEVALUE('Foo',$,IFCREAL(41.999958),$);
-> 41.99999958And maybe in [FAIL] Floating point numbers are compared with a 1e-6 tolerance 3/4
you wanted write
#23=IFCPROPERTYSINGLEVALUE('Foo',$,IFCREAL(42.00000084),$);
? (FAIL it is anyway.)And analog in [FAIL] Floating point numbers are compared with a 1e-6 tolerance 4/4
The text was updated successfully, but these errors were encountered: