Skip to content

Conversation

@cbracken
Copy link
Member

@cbracken cbracken commented Aug 30, 2020

Description

Several of our tests make use of numbers without an exact floating point representation (frequently 0.x where x!=5) which, when scaled, also scale the error. The end result is that some of these tests currently implicitly rely on an implementation detail of floating point math and are sensitive to differences in the ~15th decimal place.

This patch reduces the sensitivity of some of these tests, checking values up to the precisionErrorTolerance from the foundation layer rather than requiring en exact match.

Related patches

flutter/engine#20879 fixes a bug in lerpDouble such that when the a, b endpoint parameters differ significantly in value, 0.0 is always returned rather than an intermediate value. As part of that patch, we change the implementation of the calculation to an algebraically-equivalent form which reduces the floating point error, but which produces small differences in the least-significant bits of the double representation, particularly when the arguments in the test have no exact floating-point representation (such as 0.2, 0.3, 0.6, 0.8 in several of these particular tests).

@cbracken cbracken requested a review from Hixie August 30, 2020 20:58
@flutter-dashboard flutter-dashboard bot added f: cupertino flutter/packages/flutter/cupertino repository framework flutter/packages/flutter repository. See also f: labels. labels Aug 30, 2020
@cbracken
Copy link
Member Author

cbracken commented Aug 30, 2020

An alternative is that we change some tests to use values with exact floating point representations, such as 0.25, 0.5 or other numbers that can be represented as sums of inverse powers of two, but that feels like a bit of an unfair constraint to place on test authors. Probably better to continue using the values we like, and continue migrating any cases we run into where we're doing exact equality checks on values that have been scaled/etc. that aren't using already using closeTo().

@cbracken cbracken changed the title Match lerped values to the 12th decimal place Match lerped values using closeTo Aug 30, 2020
@cbracken cbracken requested a review from tvolkert August 30, 2020 21:56
@Hixie
Copy link
Contributor

Hixie commented Aug 30, 2020

We normally use "moreOrLessEquals" which abstracts out the epsilon.

Several of our tests make use of numbers without an exact floating point
representation (frequently 0.x where x!=5) which, when scaled, also
scale the error. The end result is that some of these tests currently
implicitly rely on an implementation detail of floating point math and
are sensitive to differences in the ~15th decimal place.

This patch reduces the sensitivity of some of these tests, checking
values up to the precisionErrorTolerance from the foundation layer
rather than requiring en exact match.
@cbracken
Copy link
Member Author

cbracken commented Aug 31, 2020

TIL. Done!

Also sent out #64914, which migrates other uses of closeTo that involve precisionErrorTolerance.

@cbracken cbracken changed the title Match lerped values using closeTo Match lerped values using moreOrLessEquals Aug 31, 2020
@cbracken cbracken merged commit 8fa5c55 into flutter:master Aug 31, 2020
@cbracken cbracken deleted the lerp-closeto branch August 31, 2020 02:05
mingwandroid pushed a commit to mingwandroid/flutter that referenced this pull request Sep 6, 2020
Several of our tests make use of numbers without an exact floating point
representation (frequently 0.x where x!=5) which, when scaled, also
scale the error. The end result is that some of these tests currently
implicitly rely on an implementation detail of floating point math and
are sensitive to differences in the ~15th decimal place.

This patch reduces the sensitivity of some of these tests, checking
values using `moreOrLessEquals` from the flutter_test package
rather than requiring en exact match.
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 31, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

f: cupertino flutter/packages/flutter/cupertino repository framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants