fix(numtower): inexact presence per value, POW sign from exact parity, i128::MIN numerators (improvise-59nw.1)
Answers ChatGPT Codex's P1 and Copilot's two findings on PR #337; each has a regression test that failed at edbb50c4.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
This commit is contained in:
committed by
Edward Langley
parent
edbb50c426
commit
568f52165d
@@ -135,19 +135,21 @@ impl<'a> Operand<'a> {
|
||||
}
|
||||
}
|
||||
|
||||
/// The `i128` fast path's operands; `i128::MIN` cannot be negated, so it takes the bigint path.
|
||||
fn parts(self) -> Option<(i128, i128)> {
|
||||
match self {
|
||||
Operand::Dec(d) => Some((d.mantissa(), pow10(d.scale()))),
|
||||
Operand::Small(n, d) => Some((n, d)),
|
||||
Operand::Small(n, d) => (n != i128::MIN).then_some((n, d)),
|
||||
Operand::Big(_) | Operand::Float(_) => None,
|
||||
}
|
||||
}
|
||||
|
||||
fn big(self) -> Option<BigQ> {
|
||||
match self {
|
||||
Operand::Dec(d) => Some(BigQ::from_i128(d.mantissa(), pow10(d.scale()))),
|
||||
Operand::Small(n, d) => Some(BigQ::from_i128(n, d)),
|
||||
Operand::Big(q) => Some(q.clone()),
|
||||
Operand::Float(_) => None,
|
||||
exact => exact.parts().map(|(n, d)| BigQ::from_i128(n, d)),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -247,7 +249,9 @@ fn negate(a: Operand) -> Interim {
|
||||
match a {
|
||||
Operand::Dec(d) => Interim::from(Num::exact(-*d)),
|
||||
Operand::Float(x) => inexact(-x),
|
||||
Operand::Small(n, d) => small_ratio(-n, d)
|
||||
Operand::Small(n, d) => n
|
||||
.checked_neg()
|
||||
.and_then(|m| small_ratio(m, d))
|
||||
.map(Interim::from)
|
||||
.unwrap_or_else(|| settle_wide(BigQ::from_i128(n, d).neg())),
|
||||
Operand::Big(q) => settle_wide(q.neg()),
|
||||
@@ -380,14 +384,24 @@ fn exact_power(a: Operand, n: &BigInt) -> Result<Interim, DivisionByZero> {
|
||||
(false, negative, false) => {
|
||||
let lower = |x: &BigInt| BigUint::from(x.bits().saturating_sub(1)) * k;
|
||||
let too_big = lower(&q.num).max(lower(&q.den)) > BigUint::from(CEILING_BITS);
|
||||
let odd_negative = q.num.is_negative() && k.is_odd();
|
||||
match (too_big, k.to_u32()) {
|
||||
(false, Some(k)) => Ok(exact_integer_power(&q, k, negative, target)),
|
||||
_ => Ok(inexact(a.float().powf(n.to_f64().unwrap_or(f64::INFINITY)))),
|
||||
_ => Ok(inexact(signed_powf(a.float(), n, odd_negative))),
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// `powf` on the magnitude, the sign taken from the exact exponent's parity.
|
||||
fn signed_powf(base: f64, n: &BigInt, odd_negative: bool) -> f64 {
|
||||
let magnitude = base.abs().powf(n.to_f64().unwrap_or(f64::INFINITY));
|
||||
match odd_negative {
|
||||
true => -magnitude,
|
||||
false => magnitude,
|
||||
}
|
||||
}
|
||||
|
||||
fn exact_integer_power(q: &BigQ, k: u32, negative: bool, target: u32) -> Interim {
|
||||
let (num, den) = (q.num.pow(k), q.den.pow(k));
|
||||
let value = match negative {
|
||||
|
||||
@@ -106,8 +106,8 @@ impl Num {
|
||||
/// `q` in an exact arm, at its minimal scale, if it fits one.
|
||||
pub(crate) fn fit(q: &BigQ) -> Option<Num> {
|
||||
decimal_of(q).or_else(|| {
|
||||
let num = to_i128(&q.num)?;
|
||||
let den = to_i128(&q.den)?;
|
||||
let num = q.num.to_i128()?;
|
||||
let den = q.den.to_i128()?;
|
||||
Some(Num::fraction(num, den))
|
||||
})
|
||||
}
|
||||
@@ -138,10 +138,6 @@ pub(crate) fn small_ratio(num: i128, den: i128) -> Option<Num> {
|
||||
})
|
||||
}
|
||||
|
||||
fn to_i128(x: &BigInt) -> Option<i128> {
|
||||
x.to_i128().filter(|v| *v != i128::MIN)
|
||||
}
|
||||
|
||||
pub(crate) fn pow10(k: u32) -> i128 {
|
||||
10i128.pow(k)
|
||||
}
|
||||
|
||||
@@ -147,7 +147,8 @@ pub struct NumSum {
|
||||
groups: BTreeMap<i128, I256>,
|
||||
group_floats: ExactSum,
|
||||
inexact: ExactSum,
|
||||
inexact_weight: i128,
|
||||
/// Net weight per f64 bit pattern: a read is inexact while any term is present.
|
||||
inexact_terms: BTreeMap<u64, i128>,
|
||||
}
|
||||
|
||||
impl NumSum {
|
||||
@@ -168,7 +169,11 @@ impl NumSum {
|
||||
match x.kind() {
|
||||
NumRef::Inexact(f) => {
|
||||
self.inexact.add(f, weight);
|
||||
self.inexact_weight += weight as i128;
|
||||
let net = self.inexact_terms.entry(f.to_bits()).or_insert(0);
|
||||
*net += weight as i128;
|
||||
if *net == 0 {
|
||||
self.inexact_terms.remove(&f.to_bits());
|
||||
}
|
||||
}
|
||||
NumRef::Exact(d) => {
|
||||
let unit = pow10(MAX_SCALE - d.scale()) as u128;
|
||||
@@ -225,7 +230,7 @@ impl NumSum {
|
||||
|
||||
/// One read: exact over at most K groups, else the f64 accumulators (clause 8).
|
||||
pub fn read(&self) -> Num {
|
||||
match (self.inexact_weight != 0, self.groups.len() > K) {
|
||||
match (!self.inexact_terms.is_empty(), self.groups.len() > K) {
|
||||
(false, false) => Num::settle(&self.exact_total()),
|
||||
(_, true) => {
|
||||
let mut acc = self.inexact.clone();
|
||||
|
||||
@@ -30,8 +30,8 @@ pub fn fits(v: &BigRational) -> bool {
|
||||
let scaled = v * BigRational::from_integer(BigInt::from(10).pow(s));
|
||||
scaled.is_integer() && scaled.to_integer().abs() < limit
|
||||
});
|
||||
let max = BigInt::from(i128::MAX);
|
||||
decimal || (v.numer().abs() <= max && v.denom() <= &max)
|
||||
let (min, max) = (BigInt::from(i128::MIN), BigInt::from(i128::MAX));
|
||||
decimal || (&min <= v.numer() && v.numer() <= &max && v.denom() <= &max)
|
||||
}
|
||||
|
||||
/// Whether `f` is the round-to-nearest-even f64 of `v`: the review's interval form.
|
||||
@@ -127,9 +127,11 @@ fn of_width(bits: u32) -> impl Strategy<Value = i128> {
|
||||
}
|
||||
|
||||
pub fn fraction() -> impl Strategy<Value = Num> {
|
||||
(1u32..=126, 1u32..=126)
|
||||
let widths = (1u32..=126, 1u32..=126)
|
||||
.prop_flat_map(|(a, b)| (of_width(a), of_width(b)))
|
||||
.prop_map(|(n, d)| Num::ratio(n, d).unwrap())
|
||||
.prop_map(|(n, d)| Num::ratio(n, d).unwrap());
|
||||
let edge = (1i128..i128::MAX).prop_map(|d| Num::ratio(i128::MIN, d | 1).unwrap());
|
||||
prop_oneof![8 => widths, 1 => edge]
|
||||
}
|
||||
|
||||
pub fn finite_f64() -> impl Strategy<Value = f64> {
|
||||
|
||||
@@ -473,6 +473,72 @@ fn min_and_max_prefer_the_exact_value_and_do_not_depend_on_order() {
|
||||
assert!(same(Num::inexact(f64::NAN).min_of(&dec("2")), &dec("2")));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn the_accumulator_keeps_inexact_terms_whose_weights_cancel() {
|
||||
let mut s = NumSum::new();
|
||||
s.add(&Num::inexact(1.0), 1);
|
||||
s.add(&Num::inexact(2.0), -1);
|
||||
s.insert(&dec("0.5"));
|
||||
let read = s.read();
|
||||
assert!(!read.is_exact(), "an inexact term is still present: {read}");
|
||||
assert_eq!(read.to_f64(), -0.5);
|
||||
s.add(&Num::inexact(1.0), -1);
|
||||
s.add(&Num::inexact(2.0), 1);
|
||||
assert!(same(&s.read(), &dec("0.5")), "every inexact term retracted");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_huge_odd_exponent_keeps_the_sign_of_a_negative_base() {
|
||||
let odd = Num::exact(Decimal::from(9_007_199_254_740_993i64));
|
||||
let up = dec("-2").pow(&odd).unwrap();
|
||||
assert!(!up.is_exact() && up.to_f64() == f64::NEG_INFINITY, "{up}");
|
||||
let down = dec("-2").pow(&-&odd).unwrap();
|
||||
assert!(
|
||||
!down.is_exact() && down.to_f64() == 0.0 && down.to_f64().is_sign_negative(),
|
||||
"{down}"
|
||||
);
|
||||
let third = Num::ratio(-1, 3).unwrap().pow(&odd).unwrap();
|
||||
assert!(third.to_f64().is_sign_negative(), "{third}");
|
||||
let even = &odd + &dec("1");
|
||||
assert_eq!(dec("-2").pow(&even).unwrap().to_f64(), f64::INFINITY);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn i128_min_is_an_exact_fraction_numerator() {
|
||||
let min = Num::ratio(i128::MIN, 1).unwrap();
|
||||
assert!(
|
||||
matches!(min.kind(), NumRef::Fraction(r) if *r.numer() == i128::MIN),
|
||||
"{min}"
|
||||
);
|
||||
let third = Num::ratio(i128::MIN, 3).unwrap();
|
||||
assert!(matches!(third.kind(), NumRef::Fraction(_)), "{third}");
|
||||
let negated = -&third;
|
||||
let want = -q(&third).unwrap();
|
||||
assert!(
|
||||
!negated.is_exact() && is_nearest(negated.to_f64(), &want),
|
||||
"{negated}"
|
||||
);
|
||||
let back =
|
||||
&(&Interim::from(third.clone()) * &Interim::from(dec("-1"))) * &Interim::from(dec("-1"));
|
||||
assert!(
|
||||
same(&back.store(), &third),
|
||||
"through the wide path and back"
|
||||
);
|
||||
let halved = third.checked_div(&dec("2")).unwrap();
|
||||
assert_eq!(
|
||||
q(&halved),
|
||||
Some(q(&third).unwrap() / BigRational::from_integer(BigInt::from(2)))
|
||||
);
|
||||
assert_eq!(
|
||||
min.cmp_exact(&Num::ratio(i128::MIN + 1, 1).unwrap()),
|
||||
Some(Ordering::Less)
|
||||
);
|
||||
assert_eq!(
|
||||
Num::from_canonical(&min.to_string()).map(|n| same(&n, &min)),
|
||||
Some(true)
|
||||
);
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// The accumulator is independent of order.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user