fix(extract): deterministic decisecond rounding in extraction + tests#7627
Open
DennisOSRM wants to merge 9 commits into
Open
fix(extract): deterministic decisecond rounding in extraction + tests#7627DennisOSRM wants to merge 9 commits into
DennisOSRM wants to merge 9 commits into
Conversation
This reverts commit cedd640.
…t to detect platform rounding differences - Use long double floor-based rounding when converting segment.duration to integer deciseconds to avoid platform-dependent std::round behavior. - Add unit_tests/util/rounding_consistency.cpp to catch rounding mismatches between double std::round and long double floor method. - Revert temporary JavaScript fuzzy-tolerance test change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
DennisOSRM
changed the title
The long double + floor approach preserves binary representation errors of decimal fractions, causing incorrect rounding for values like 1.45 (rounds to 14 deciseconds instead of the correct 15). IEEE 754 double-precision correctly-rounded multiplication compensates for these errors: 1.45*10.0 snaps to exactly 14.5, and std::round(14.5)=15. The std::round approach is deterministic on all modern platforms (FLT_EVAL_METHOD=0 on x86-64 and ARM). Replaced the unit test that compared two inherently different rounding methods with proper tests that validate correct concrete decisecond values and verify determinism.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Make decisecond rounding deterministic by using long double floor-based rounding when converting segment.duration to integer deciseconds. Add a focused unit test (unit_tests/util/rounding_consistency.cpp) to detect platform-dependent differences between std::round on double and a long double floor-based rounding. Revert temporary JS fuzzy-tolerance change.
This PR aims to remove flaky 0.1s differences observed on Ubuntu 26.04 CI by ensuring deterministic rounding behavior across platforms.
CI confirms the fix, the debug logging added earlier will be removed.