Skip to content
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

Drop comparison /w PHP_FLOAT_EPSILON #4972

Merged
merged 1 commit into from Sep 14, 2022
Merged

Drop comparison /w PHP_FLOAT_EPSILON #4972

merged 1 commit into from Sep 14, 2022

Conversation

mvorisek
Copy link
Contributor

@mvorisek mvorisek commented May 21, 2022

fixes #4966

The removed code is residual from 136fae3 and even a difference by PHP_FLOAT_EPSILON should be reported as a failure.

The PHP_FLOAT_EPSILON constant equals to 2.220446049250313E-16 (see 3v4l link in a post below) and is usable only when the abs(a - b) is done on normalized values, but when done so, the comparison is useless or satisfied because of the normalization rounding error solely. On the other side, imagine comparing very small numbers like 1e-100 and 1e-200. The difference between these two normalized values is astronomically large, but currently the phpunit handles such numbers as same!

@mvorisek mvorisek marked this pull request as draft May 21, 2022 21:12
@mvorisek
Copy link
Contributor Author

mvorisek commented May 21, 2022

marking as draft, the one failing test should fail - https://3v4l.org/rIJBO,

/**
 * Cast float to string with lossless precision.
 */
final public static function castFloatToString(float $value): string
{
    $precisionBackup = ini_get('precision');
    ini_set('precision', '-1');
    try {
        return (string) $value;
    } finally {
        ini_set('precision', $precisionBackup);
    }
}

must be put everywhere where float to string is casted (for print, for weak comparison, ...)

@mvorisek mvorisek marked this pull request as ready for review September 14, 2022 13:50
@mvorisek
Copy link
Contributor Author

PR is done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants