Skip to content

fix(math): fix double rounding in Math.f16round#5238

Draft
ParthMozarkar wants to merge 2 commits intoboa-dev:mainfrom
ParthMozarkar:fix-math-f16round-double-rounding
Draft

fix(math): fix double rounding in Math.f16round#5238
ParthMozarkar wants to merge 2 commits intoboa-dev:mainfrom
ParthMozarkar:fix-math-f16round-double-rounding

Conversation

@ParthMozarkar
Copy link
Contributor

@ParthMozarkar ParthMozarkar commented Mar 23, 2026

Hey! I noticed Math.f16round was failing the test262 value-conversion.js
test and dug into why.

The issue is that the float16 crate's from_f64 converts through f32
internally, which causes double rounding for certain values.

For example Math.f16round(1.00048828125000022204) was returning 1
instead of the correct 1.0009765625.

I fixed it by switching to the half crate's f16::from_f64 which is
already in the dependency tree and converts directly from f64 to f16
without any intermediate f32 conversion.

All existing tests pass.

Fixes #5237

@github-actions github-actions bot added Waiting On Review Waiting on reviews from the maintainers C-Dependencies Pull requests that update a dependency file C-Builtins PRs and Issues related to builtins/intrinsics and removed Waiting On Review Waiting on reviews from the maintainers labels Mar 23, 2026
@github-actions github-actions bot added this to the v1.0.0 milestone Mar 23, 2026
@github-actions github-actions bot added the Waiting On Review Waiting on reviews from the maintainers label Mar 23, 2026
@github-actions
Copy link

Test262 conformance changes

Test result main count PR count difference
Total 52,963 52,963 0
Passed 50,545 50,545 0
Ignored 1,426 1,426 0
Failed 992 992 0
Panics 2 2 0
Conformance 95.43% 95.43% 0.00%

Tested main commit: 66a1cd268ce6006918032b5d0f98d3a63c81b188
Tested PR commit: a76e227ce36c8ccc2ade29523eacad1e5f160c78
Compare commits: 66a1cd2...a76e227

@codecov
Copy link

codecov bot commented Mar 23, 2026

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 59.88%. Comparing base (6ddc2b4) to head (a76e227).
⚠️ Report is 916 commits behind head on main.

Files with missing lines Patch % Lines
core/engine/src/builtins/math/mod.rs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #5238       +/-   ##
===========================================
+ Coverage   47.24%   59.88%   +12.64%     
===========================================
  Files         476      582      +106     
  Lines       46892    63460    +16568     
===========================================
+ Hits        22154    38006    +15852     
- Misses      24738    25454      +716     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-Builtins PRs and Issues related to builtins/intrinsics C-Dependencies Pull requests that update a dependency file Waiting On Review Waiting on reviews from the maintainers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Math.f16round gives wrong results for some values

1 participant