Skip to content

Commit 0ad0cb0

Browse files
committed
Fix cmath.log error propagation and bigint log precision
cmath: Rewrite log(z, base) to match cmath_log_impl errno semantics. c_log always returns a value and separately reports EDOM for zero. c_quot returns EDOM and (0,0) for zero denominator instead of NaN. bigint: Add sticky bit to frexp_bigint for correct IEEE round-half-to-even, matching _PyLong_Frexp. Use mul_add for log/log2/log10 bigint to match loghelper fma usage. Add regression tests for remainder tie-to-even and signed zero.
1 parent 3ae1ee1 commit 0ad0cb0

File tree

5 files changed

+247
-32
lines changed

5 files changed

+247
-32
lines changed

src/cmath/exponential.rs

Lines changed: 97 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -177,40 +177,47 @@ pub(crate) fn ln(z: Complex64) -> Result<Complex64> {
177177
/// If base is Some(b), returns log(z) / log(b).
178178
#[inline]
179179
pub fn log(z: Complex64, base: Option<Complex64>) -> Result<Complex64> {
180-
// c_log always returns a value, but sets errno for special cases.
181-
// The error check happens at the end of cmath_log_impl.
182-
// For log(z) without base: z=0 raises EDOM
183-
// For log(z, base): z=0 doesn't raise because c_log(base) clears errno
184-
let z_is_zero = z.re == 0.0 && z.im == 0.0;
180+
let (log_z, mut err) = c_log(z);
185181
match base {
186-
None => {
187-
// No base: raise error if z=0
188-
if z_is_zero {
189-
return Err(Error::EDOM);
190-
}
191-
ln(z)
192-
}
182+
None => err.map_or(Ok(log_z), Err),
193183
Some(b) => {
194-
// With base: z=0 is allowed (second ln clears the "errno")
195-
let log_z = ln(z)?;
196-
let log_b = ln(b)?;
197-
// Use _Py_c_quot-style division to preserve sign of zero
198-
Ok(c_quot(log_z, log_b))
184+
// Like cmath_log_impl, the second c_log call overwrites
185+
// any pending error from the first one.
186+
let (log_b, base_err) = c_log(b);
187+
err = base_err;
188+
let (q, quot_err) = c_quot(log_z, log_b);
189+
if let Some(e) = quot_err {
190+
err = Some(e);
191+
}
192+
err.map_or(Ok(q), Err)
199193
}
200194
}
201195
}
202196

197+
/// c_log behavior: always returns a value, but reports EDOM for zero.
198+
#[inline]
199+
fn c_log(z: Complex64) -> (Complex64, Option<Error>) {
200+
let r = ln(z).expect("ln handles special values without failing");
201+
if z.re == 0.0 && z.im == 0.0 {
202+
(r, Some(Error::EDOM))
203+
} else {
204+
(r, None)
205+
}
206+
}
207+
203208
/// Complex division following _Py_c_quot algorithm.
204209
/// This preserves the sign of zero correctly and recovers infinities
205210
/// from NaN results per C11 Annex G.5.2.
206211
#[inline]
207-
fn c_quot(a: Complex64, b: Complex64) -> Complex64 {
212+
fn c_quot(a: Complex64, b: Complex64) -> (Complex64, Option<Error>) {
208213
let abs_breal = m::fabs(b.re);
209214
let abs_bimag = m::fabs(b.im);
215+
let mut err = None;
210216

211217
let mut r = if abs_breal >= abs_bimag {
212218
if abs_breal == 0.0 {
213-
Complex64::new(f64::NAN, f64::NAN)
219+
err = Some(Error::EDOM);
220+
Complex64::new(0.0, 0.0)
214221
} else {
215222
let ratio = b.im / b.re;
216223
let denom = b.re + b.im * ratio;
@@ -244,7 +251,7 @@ fn c_quot(a: Complex64, b: Complex64) -> Complex64 {
244251
}
245252
}
246253

247-
r
254+
(r, err)
248255
}
249256

250257
/// Complex base-10 logarithm.
@@ -325,6 +332,53 @@ mod tests {
325332
});
326333
}
327334

335+
fn test_log_error(z: Complex64, base: Complex64) {
336+
use pyo3::prelude::*;
337+
338+
let rs_result = log(z, Some(base));
339+
340+
Python::attach(|py| {
341+
let cmath = pyo3::types::PyModule::import(py, "cmath").unwrap();
342+
let py_z = pyo3::types::PyComplex::from_doubles(py, z.re, z.im);
343+
let py_base = pyo3::types::PyComplex::from_doubles(py, base.re, base.im);
344+
let py_result = cmath.getattr("log").unwrap().call1((py_z, py_base));
345+
346+
match py_result {
347+
Ok(result) => {
348+
use pyo3::types::PyComplexMethods;
349+
let c = result.cast::<pyo3::types::PyComplex>().unwrap();
350+
panic!(
351+
"log({}+{}j, {}+{}j): expected ValueError, got ({}, {})",
352+
z.re,
353+
z.im,
354+
base.re,
355+
base.im,
356+
c.real(),
357+
c.imag()
358+
);
359+
}
360+
Err(err) => {
361+
assert!(
362+
err.is_instance_of::<pyo3::exceptions::PyValueError>(py),
363+
"log({}+{}j, {}+{}j): expected ValueError, got {err:?}",
364+
z.re,
365+
z.im,
366+
base.re,
367+
base.im,
368+
);
369+
assert!(
370+
matches!(rs_result, Err(crate::Error::EDOM)),
371+
"log({}+{}j, {}+{}j): expected Err(EDOM), got {rs_result:?}",
372+
z.re,
373+
z.im,
374+
base.re,
375+
base.im,
376+
);
377+
}
378+
}
379+
});
380+
}
381+
328382
use crate::test::EDGE_VALUES;
329383

330384
#[test]
@@ -382,6 +436,29 @@ mod tests {
382436
}
383437
}
384438

439+
#[test]
440+
fn regression_c_quot_zero_denominator_sets_edom() {
441+
let (q, err) = c_quot(Complex64::new(2.0, -3.0), Complex64::new(0.0, 0.0));
442+
assert_eq!(err, Some(crate::Error::EDOM));
443+
assert_eq!(q.re.to_bits(), 0.0f64.to_bits());
444+
assert_eq!(q.im.to_bits(), 0.0f64.to_bits());
445+
}
446+
447+
#[test]
448+
fn regression_log_zero_quotient_denominator_raises_edom() {
449+
let cases = [
450+
(Complex64::new(2.0, 0.0), Complex64::new(1.0, 0.0)),
451+
(Complex64::new(1.0, 0.0), Complex64::new(1.0, 0.0)),
452+
(Complex64::new(2.0, 0.0), Complex64::new(0.0, 0.0)),
453+
(Complex64::new(0.0, 0.0), Complex64::new(1.0, 0.0)),
454+
(Complex64::new(0.0, 0.0), Complex64::new(0.0, 0.0)),
455+
];
456+
457+
for (z, base) in cases {
458+
test_log_error(z, base);
459+
}
460+
}
461+
385462
proptest::proptest! {
386463
#[test]
387464
fn proptest_sqrt(re: f64, im: f64) {

src/math/bigint.rs

Lines changed: 87 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -81,10 +81,17 @@ fn frexp_bigint(n: &BigInt) -> (f64, i64) {
8181
return (m, e as i64);
8282
}
8383

84-
// For large integers, extract top ~53 bits
85-
// Shift right to keep DBL_MANT_DIG + 2 = 55 bits for rounding
84+
// For large integers, extract top DBL_MANT_DIG + 2 = 55 bits for rounding
8685
let shift = bits - 55;
87-
let mantissa_int = n >> shift as u64;
86+
let mut mantissa_int = n >> shift as u64;
87+
88+
// Sticky bit: if any shifted-out bits were non-zero, set the LSB.
89+
// This ensures correct IEEE round-half-to-even when converting to f64.
90+
// See _PyLong_Frexp in longobject.c.
91+
if (&mantissa_int << shift as u64) != *n {
92+
mantissa_int |= BigInt::from(1);
93+
}
94+
8895
let mut x = mantissa_int.to_f64().unwrap();
8996

9097
// x is now approximately n / 2^shift, with ~55 bits of precision
@@ -119,7 +126,7 @@ pub fn log_bigint(n: &BigInt, base: Option<f64>) -> crate::Result<f64> {
119126
// Use frexp decomposition for large values
120127
// n ~= x * 2^e, so log(n) = log(x) + log(2) * e
121128
let (x, e) = frexp_bigint(n);
122-
let log_n = crate::m::log(x) + std::f64::consts::LN_2 * (e as f64);
129+
let log_n = crate::mul_add(crate::m::log(2.0), e as f64, crate::m::log(x));
123130

124131
match base {
125132
None => Ok(log_n),
@@ -150,7 +157,11 @@ pub fn log2_bigint(n: &BigInt) -> crate::Result<f64> {
150157
// Use frexp decomposition for large values
151158
// n ~= x * 2^e, so log2(n) = log2(x) + e
152159
let (x, e) = frexp_bigint(n);
153-
Ok(crate::m::log2(x) + (e as f64))
160+
Ok(crate::mul_add(
161+
crate::m::log2(2.0),
162+
e as f64,
163+
crate::m::log2(x),
164+
))
154165
}
155166

156167
/// Return the base-10 logarithm of a BigInt.
@@ -171,7 +182,11 @@ pub fn log10_bigint(n: &BigInt) -> crate::Result<f64> {
171182
// Use frexp decomposition for large values
172183
// n ~= x * 2^e, so log10(n) = log10(x) + log10(2) * e
173184
let (x, e) = frexp_bigint(n);
174-
Ok(crate::m::log10(x) + std::f64::consts::LOG10_2 * (e as f64))
185+
Ok(crate::mul_add(
186+
crate::m::log10(2.0),
187+
e as f64,
188+
crate::m::log10(x),
189+
))
175190
}
176191

177192
/// Compute ldexp(x, exp) where exp is a BigInt.
@@ -333,6 +348,33 @@ mod tests {
333348
});
334349
}
335350

351+
fn assert_exact_log_bigint_bits(n: &BigInt, func_name: &str, rs: crate::Result<f64>) {
352+
crate::test::with_py_math(|py, math| {
353+
let n_str = n.to_string();
354+
let builtins = pyo3::types::PyModule::import(py, "builtins").unwrap();
355+
let py_n = builtins
356+
.getattr("int")
357+
.unwrap()
358+
.call1((n_str.as_str(),))
359+
.unwrap();
360+
let py_f: f64 = math
361+
.getattr(func_name)
362+
.unwrap()
363+
.call1((py_n,))
364+
.unwrap()
365+
.extract()
366+
.unwrap();
367+
let rs_f = rs.unwrap();
368+
assert_eq!(
369+
py_f.to_bits(),
370+
rs_f.to_bits(),
371+
"{func_name}({n}): py={py_f} ({:#x}) vs rs={rs_f} ({:#x})",
372+
py_f.to_bits(),
373+
rs_f.to_bits()
374+
);
375+
});
376+
}
377+
336378
#[test]
337379
fn edgetest_log_bigint() {
338380
// Small values
@@ -410,6 +452,45 @@ mod tests {
410452
}
411453
}
412454

455+
#[test]
456+
fn regression_log_bigint_pow10_309() {
457+
let n = BigInt::from(10).pow(309);
458+
assert_exact_log_bigint_bits(&n, "log", log_bigint(&n, None));
459+
}
460+
461+
#[test]
462+
fn regression_log2_bigint_pow10_309() {
463+
let n = BigInt::from(10).pow(309);
464+
assert_exact_log_bigint_bits(&n, "log2", log2_bigint(&n));
465+
}
466+
467+
#[test]
468+
fn regression_log10_bigint_pow10_309() {
469+
let n = BigInt::from(10).pow(309);
470+
assert_exact_log_bigint_bits(&n, "log10", log10_bigint(&n));
471+
}
472+
473+
#[test]
474+
fn regression_log_bigint_sticky_bit_rounding() {
475+
// If frexp_bigint drops the sticky bit, this rounds 1 ULP low.
476+
let n = (BigInt::from(0x40000000000c02u64) << 970u32) + BigInt::from(1u8);
477+
assert_exact_log_bigint_bits(&n, "log", log_bigint(&n, None));
478+
}
479+
480+
#[test]
481+
fn regression_log2_bigint_sticky_bit_rounding() {
482+
// If frexp_bigint drops the sticky bit, this rounds 1 ULP low.
483+
let n = (BigInt::from(0x400000000010a2u64) << 970u32) + BigInt::from(1u8);
484+
assert_exact_log_bigint_bits(&n, "log2", log2_bigint(&n));
485+
}
486+
487+
#[test]
488+
fn regression_log10_bigint_sticky_bit_rounding() {
489+
// If frexp_bigint drops the sticky bit, this rounds 1 ULP low.
490+
let n = (BigInt::from(0x4000000000049au64) << 970u32) + BigInt::from(1u8);
491+
assert_exact_log_bigint_bits(&n, "log10", log10_bigint(&n));
492+
}
493+
413494
// ldexp_bigint tests
414495

415496
fn test_ldexp_bigint_impl(x: f64, exp: &BigInt) {

src/math/integer.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -730,8 +730,8 @@ mod tests {
730730
128, // Table boundary + 1
731731
// Powers of 2 and boundaries
732732
64,
733-
63, // 2^6 - 1
734-
65, // 2^6 + 1
733+
63, // 2^6 - 1
734+
65, // 2^6 + 1
735735
1024,
736736
65535, // 2^16 - 1
737737
65536, // 2^16

src/math/misc.rs

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -390,6 +390,64 @@ mod tests {
390390
}
391391
}
392392

393+
#[test]
394+
fn regression_remainder_halfway_even_cases() {
395+
// These cases exercise the half-way branch in CPython's custom
396+
// m_remainder implementation, where ties are resolved toward the
397+
// even multiple of y.
398+
let cases = [
399+
((6.0, 4.0), 0xc000_0000_0000_0000_u64), // -2.0
400+
((3.0, 2.0), 0xbff0_0000_0000_0000_u64), // -1.0
401+
((5.0, 2.0), 0x3ff0_0000_0000_0000_u64), // 1.0
402+
((-6.0, 4.0), 0x4000_0000_0000_0000_u64), // 2.0
403+
((-5.0, 2.0), 0xbff0_0000_0000_0000_u64), // -1.0
404+
((1.5, 1.0), 0xbfe0_0000_0000_0000_u64), // -0.5
405+
((2.5, 1.0), 0x3fe0_0000_0000_0000_u64), // 0.5
406+
((3.5, 1.0), 0xbfe0_0000_0000_0000_u64), // -0.5
407+
((4.5, 1.0), 0x3fe0_0000_0000_0000_u64), // 0.5
408+
((5.5, 1.0), 0xbfe0_0000_0000_0000_u64), // -0.5
409+
];
410+
411+
for &((x, y), expected_bits) in &cases {
412+
let r = remainder(x, y).unwrap();
413+
assert_eq!(
414+
r.to_bits(),
415+
expected_bits,
416+
"remainder({x}, {y}) = {r} ({:#x}), expected {:#x}",
417+
r.to_bits(),
418+
expected_bits
419+
);
420+
test_remainder(x, y);
421+
}
422+
}
423+
424+
#[test]
425+
fn regression_remainder_boundary_and_signed_zero_cases() {
426+
// These cases pin the sign of zero and the behavior immediately
427+
// around the half-way boundary.
428+
let just_below_half = f64::from_bits(0x3fdf_ffff_ffff_ffff);
429+
let just_above_half = f64::from_bits(0x3fe0_0000_0000_0001);
430+
let cases = [
431+
((4.0, 2.0), 0x0000_0000_0000_0000_u64), // +0.0
432+
((-4.0, 2.0), 0x8000_0000_0000_0000_u64), // -0.0
433+
((4.0, -2.0), 0x0000_0000_0000_0000_u64), // +0.0
434+
((just_below_half, 1.0), 0x3fdf_ffff_ffff_ffff_u64),
435+
((just_above_half, 1.0), 0xbfdf_ffff_ffff_fffe_u64),
436+
];
437+
438+
for &((x, y), expected_bits) in &cases {
439+
let r = remainder(x, y).unwrap();
440+
assert_eq!(
441+
r.to_bits(),
442+
expected_bits,
443+
"remainder({x}, {y}) = {r} ({:#x}), expected {:#x}",
444+
r.to_bits(),
445+
expected_bits
446+
);
447+
test_remainder(x, y);
448+
}
449+
}
450+
393451
#[test]
394452
fn edgetest_copysign() {
395453
for &x in crate::test::EDGE_VALUES {

0 commit comments

Comments
 (0)