Skip to content

float: pin erf/erfc(1.0) per platform libm - #101

Open
brandonros wants to merge 1 commit into
malisper:mainfrom
brandonros:fix/macos-erf-ulp-test
Open

float: pin erf/erfc(1.0) per platform libm#101
brandonros wants to merge 1 commit into
malisper:mainfrom
brandonros:fix/macos-erf-ulp-test

Conversation

@brandonros

@brandonros brandonros commented Sep 6, 2026

Copy link
Copy Markdown

`math_domains_and_live_pg_values` fails on every macOS checkout: Apple's libm differs from glibc by one ulp at `erf(1.0)` and `erfc(1.0)`. Pin both arms by `target_os` so a libm regression on either platform is still caught, the way the existing `lgamma` arms already do.

Verified: `cargo test -p adt_float math_domains` passes on Darwin 25.6.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Tests
    • Updated mathematical accuracy tests to account for minor platform-specific differences in erf(1.0) and erfc(1.0) results on macOS versus other operating systems.

Apple's libm differs from glibc by one ulp at erf(1.0) and erfc(1.0),
so math_domains_and_live_pg_values failed on every macOS checkout.
Pin both arms by target_os so a libm regression on either platform is
still caught.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 6, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: 481dc079-5ddb-4133-8043-33989920df5d

📥 Commits

Reviewing files that changed from the base of the PR and between 4c2e49f and 88f7aa2.

📒 Files selected for processing (1)
  • crates/backend/utils/adt/float/src/tests.rs

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

The float math test now uses target-specific expected values for erf(1.0) and erfc(1.0). macOS uses values that differ by one ulp from the values used on other platforms.

Changes

Float assertion portability

Layer / File(s) Summary
Platform-specific erf assertions
crates/backend/utils/adt/float/src/tests.rs
The test uses macOS-specific expected values and retains the existing values for non-macOS targets. A comment documents the libm difference.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Merge Risk: ⚪ Minimal · up to 88f7a

This updates float-test expectations for the documented one-ULP macOS libm difference while retaining the existing values elsewhere. No production behavior changes, and the change is ready to merge.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the platform-specific pinning of erf(1.0) and erfc(1.0) test values, which is the main change.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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.

1 participant