Conversation
|
I am so sorry. I must have missed this PR completely. I am a bit concerned about all the tests that were rewritten in this PR. What is the reason behind that? |
The behavior changed, which required some tests to be modified. I also find the current implementation of the tests cleaner - there is less duplicate code. Some tests were written in such a way that they didn't actually test anything, for example because they tested the equality of identical numbers (implementation using == would also pass the test). |
| assert_relative_eq!(0.0f32, 1e-40f32, epsilon = 1e-40f32); | ||
| assert_relative_eq!(1e-40f32, 0.0f32, epsilon = 1e-40f32); | ||
| assert_relative_eq!(0.0f32, -1e-40f32, epsilon = 1e-40f32); | ||
| assert_relative_eq!(-1e-40f32, 0.0f32, epsilon = 1e-40f32); | ||
| assert_relative_eq!(0.0f32, 1e-20f32, epsilon = 1e-20f32); | ||
| assert_relative_eq!(1e-20f32, 0.0f32, epsilon = 1e-20f32); | ||
| assert_relative_eq!(0.0f32, -1e-20f32, epsilon = 1e-20f32); | ||
| assert_relative_eq!(-1e-20f32, 0.0f32, epsilon = 1e-20f32); | ||
|
|
||
| assert_relative_ne!(1e-40f32, 0.0f32, epsilon = 1e-41f32); | ||
| assert_relative_ne!(0.0f32, 1e-40f32, epsilon = 1e-41f32); | ||
| assert_relative_ne!(-1e-40f32, 0.0f32, epsilon = 1e-41f32); | ||
| assert_relative_ne!(0.0f32, -1e-40f32, epsilon = 1e-41f32); |
There was a problem hiding this comment.
This looks like a precision regression to me.
There was a problem hiding this comment.
1e-40 is denormalized number, it is not safe to compare denormalized numbers using relative comparison. Therefore they are marked equal by absolute comparison.
| } | ||
|
|
||
| fn compare_near_numbers(number: f32) { | ||
| let rel_eq_number = number * 1.0000001f32; |
There was a problem hiding this comment.
this multiplication has an internal error associated with it. Consider this little program:
fn main() {
let a = 1E-20;
let b = 1E-10;
println!("{}", a * b);
println!("{}", 1E-30);
}Output
0.0000000000000000000000000000009999999999999999
0.000000000000000000000000000001
How do we ensure that this multiplicative error does not invalidate our test (either via false positive or false negative).
There was a problem hiding this comment.
I dont understand.
-
How can you tell if the result is correct? We are talking about approximate comparisons, there is no strict border between correct and incorrect behavior.
-
Multiplication is defined by IEEE754 to return always correctly rounded result, so with given numbers it is just different way how to "hardcode" a number. In this case the result seems to be not correct but that is because input numbers are rounded.
1e-20 is stored as 9.9999999999999994515327145421e-21
1e-10 is stored as 1.0000000000000000364321973155e-10
Their product is most accurately represented as 9.99999999999999908174112566984e-31, not by 1e-30 which is stored as 1.00000000000000008333642060759E-30.
jonaspleyer
left a comment
jonaspleyer
left a comment
There was a problem hiding this comment.
I have left some comments which I would like to discuss.
I think that there is nothing wrong with testing cases which would be true via Your implementation of the tests is different since it involves a multiplication to calculate the numbers that should be compared. |
kitt159
force-pushed
the
relative_eq-small-values-fix
branch
from
1f4d408 to
bc35d0f
Compare
kitt159
force-pushed
the
relative_eq-small-values-fix
branch
from
bc35d0f to
2b9646e
Compare
2 participants
Change is described here
Fixes #65