fix: f32→f64 precision artifact in temperature causes provider 400 errors - #1418
Boomboomdunce wants to merge 1 commit into
Conversation
…rors Direct f32-as-f64 preserves the binary representation, producing values like 0.699999988079071 instead of 0.7. Some OpenAI-compatible providers (e.g. Zhipu GLM-5) reject these with a 400 error. Add round_f32_to_f64() that formats to 6 decimal places before parsing back to f64.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves a critical compatibility issue with OpenAI-compatible providers caused by floating-point precision artifacts during Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request addresses an issue where converting an f32 temperature value to f64 introduces precision artifacts, causing 400 errors from some LLM providers. The fix introduces a round_f32_to_f64 helper function that rounds the value by formatting it to a string with 6 decimal places and parsing it back to an f64. This is a clever solution to the floating-point representation problem. The change is well-tested. My only suggestion is to make the parsing logic more robust by using expect() instead of unwrap_or(), to ensure any unexpected parsing failures are caught immediately rather than silently falling back to the old, problematic behavior.
| /// the artifact while preserving all meaningful precision for temperature/top_p. | ||
| fn round_f32_to_f64(val: f32) -> f64 { | ||
| let s = format!("{:.6}", val); | ||
| s.parse::<f64>().unwrap_or(val as f64) |
There was a problem hiding this comment.
The use of unwrap_or with a fallback to val as f64 could silently re-introduce the precision artifact issue if s.parse::<f64>() were to fail for some unforeseen reason. Since format!("{:.6}", val) should always produce a string that can be parsed back into a float, it's safer to treat this as an infallible operation. Using .expect() would cause a panic if parsing fails, making any unexpected behavior immediately obvious during development and testing, which is preferable to silently using the incorrect value.
| s.parse::<f64>().unwrap_or(val as f64) | |
| s.parse::<f64>().expect("formatted f32 should always be parsable to f64") |
References
- Prefer
expect()to explicitly fail with a clear message if an operation's setup or expected outcome is not met, especially whenunwrap_or()could silently lead to incorrect logic. This principle extends beyond tests to general code robustness, ensuring immediate detection of unexpected behavior.
…ression-check] Co-Authored-By: Boomboomdunce <liweizhu0708@gmail.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Thanks for the work on this, @Boomboomdunce! Great catch on the f32→f64 precision artifact. I've picked up your changes and continued them in #1450. The new PR includes your original work plus:
You're credited as co-author on the commit. Feel free to review the new PR! |
…rors (#1450) * fix: f32→f64 precision artifact in temperature causes provider 400 errors Direct f32-as-f64 preserves the binary representation, producing values like 0.699999988079071 instead of 0.7. Some OpenAI-compatible providers (e.g. Zhipu GLM-5) reject these with a 400 error. Add round_f32_to_f64() that formats to 6 decimal places before parsing back to f64. * fix: address clippy redundant_closure lint (takeover #1418) [skip-regression-check] Co-Authored-By: Boomboomdunce <liweizhu0708@gmail.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: use numeric rounding, update doc comment, remove duplicate assertion [skip-regression-check] Address review feedback on #1450: - Replace format!+parse with numeric rounding to avoid allocation - Update doc comment to only mention temperature (not top_p) - Remove duplicate assert_eq in test Co-Authored-By: Boomboomdunce <liweizhu0708@gmail.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Boomboomdunce <liweizhu0708@gmail.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…rors (#1450) * fix: f32→f64 precision artifact in temperature causes provider 400 errors Direct f32-as-f64 preserves the binary representation, producing values like 0.699999988079071 instead of 0.7. Some OpenAI-compatible providers (e.g. Zhipu GLM-5) reject these with a 400 error. Add round_f32_to_f64() that formats to 6 decimal places before parsing back to f64. * fix: address clippy redundant_closure lint (takeover #1418) [skip-regression-check] Co-Authored-By: Boomboomdunce <liweizhu0708@gmail.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: use numeric rounding, update doc comment, remove duplicate assertion [skip-regression-check] Address review feedback on #1450: - Replace format!+parse with numeric rounding to avoid allocation - Update doc comment to only mention temperature (not top_p) - Remove duplicate assert_eq in test Co-Authored-By: Boomboomdunce <liweizhu0708@gmail.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Boomboomdunce <liweizhu0708@gmail.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…rors (#1450) * fix: f32→f64 precision artifact in temperature causes provider 400 errors Direct f32-as-f64 preserves the binary representation, producing values like 0.699999988079071 instead of 0.7. Some OpenAI-compatible providers (e.g. Zhipu GLM-5) reject these with a 400 error. Add round_f32_to_f64() that formats to 6 decimal places before parsing back to f64. * fix: address clippy redundant_closure lint (takeover #1418) [skip-regression-check] Co-Authored-By: Boomboomdunce <liweizhu0708@gmail.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: use numeric rounding, update doc comment, remove duplicate assertion [skip-regression-check] Address review feedback on #1450: - Replace format!+parse with numeric rounding to avoid allocation - Update doc comment to only mention temperature (not top_p) - Remove duplicate assert_eq in test Co-Authored-By: Boomboomdunce <liweizhu0708@gmail.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Boomboomdunce <liweizhu0708@gmail.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…rors (nearai#1450) * fix: f32→f64 precision artifact in temperature causes provider 400 errors Direct f32-as-f64 preserves the binary representation, producing values like 0.699999988079071 instead of 0.7. Some OpenAI-compatible providers (e.g. Zhipu GLM-5) reject these with a 400 error. Add round_f32_to_f64() that formats to 6 decimal places before parsing back to f64. * fix: address clippy redundant_closure lint (takeover nearai#1418) [skip-regression-check] Co-Authored-By: Boomboomdunce <liweizhu0708@gmail.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: use numeric rounding, update doc comment, remove duplicate assertion [skip-regression-check] Address review feedback on nearai#1450: - Replace format!+parse with numeric rounding to avoid allocation - Update doc comment to only mention temperature (not top_p) - Remove duplicate assert_eq in test Co-Authored-By: Boomboomdunce <liweizhu0708@gmail.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Boomboomdunce <liweizhu0708@gmail.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…rors (nearai#1450) * fix: f32→f64 precision artifact in temperature causes provider 400 errors Direct f32-as-f64 preserves the binary representation, producing values like 0.699999988079071 instead of 0.7. Some OpenAI-compatible providers (e.g. Zhipu GLM-5) reject these with a 400 error. Add round_f32_to_f64() that formats to 6 decimal places before parsing back to f64. * fix: address clippy redundant_closure lint (takeover nearai#1418) [skip-regression-check] Co-Authored-By: Boomboomdunce <liweizhu0708@gmail.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: use numeric rounding, update doc comment, remove duplicate assertion [skip-regression-check] Address review feedback on nearai#1450: - Replace format!+parse with numeric rounding to avoid allocation - Update doc comment to only mention temperature (not top_p) - Remove duplicate assert_eq in test Co-Authored-By: Boomboomdunce <liweizhu0708@gmail.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Boomboomdunce <liweizhu0708@gmail.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Problem
f32 as f64preserves the binary representation, producing values like0.699999988079071instead of0.7. Some OpenAI-compatible providers (e.g. Zhipu GLM-5) reject these with a 400 error ("API 调用参数有误").Fix
Added
round_f32_to_f64()helper that formats to 6 decimal places before parsing back tof64. Applied to thetemperaturefield inbuild_rig_request().Test
Regression test
test_round_f32_to_f64_no_precision_artifactsverifies:round_f32_to_f64(0.7_f32) == 0.7(no artifact)0.7_f32 as f64 != 0.7(confirms the original bug exists)Impact
This affects all providers using
RigAdapter(OpenAI, Anthropic, Ollama, Tinfoil, OpenAI-compatible). The fix is purely cosmetic for providers that tolerate the artifact, and unblocks providers that don't.