From 539caf7ad9cb2ca1456a60def745a46eb0688695 Mon Sep 17 00:00:00 2001 From: Corwin Date: Sun, 8 Oct 2023 14:22:50 +0100 Subject: [PATCH 1/5] add test showing inaccuracy --- agb-fixnum/src/lib.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/agb-fixnum/src/lib.rs b/agb-fixnum/src/lib.rs index 3ba3a47f..ba20b183 100644 --- a/agb-fixnum/src/lib.rs +++ b/agb-fixnum/src/lib.rs @@ -1234,6 +1234,12 @@ mod tests { test_base::<11>(); } + #[test] + fn check_cos_accuracy() { + let n: Num = Num::new(1) / 32; + assert_eq!(n.cos(), num!(0.9808)); + } + #[test] fn test_numbers() { // test addition From 548dd9ff673b4f4a119a92f4127ac47e3827aca6 Mon Sep 17 00:00:00 2001 From: Corwin Date: Sun, 8 Oct 2023 14:23:04 +0100 Subject: [PATCH 2/5] fix cos function --- agb-fixnum/src/lib.rs | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/agb-fixnum/src/lib.rs b/agb-fixnum/src/lib.rs index ba20b183..19e5a704 100644 --- a/agb-fixnum/src/lib.rs +++ b/agb-fixnum/src/lib.rs @@ -533,17 +533,10 @@ impl Num { /// ``` #[must_use] pub fn cos(self) -> Self { - let one: Self = I::one().into(); let mut x = self; - let four: I = 4.into(); - let two: I = 2.into(); - let sixteen: I = 16.into(); - let nine: I = 9.into(); - let forty: I = 40.into(); - - x -= one / four + (x + one / four).floor(); - x *= (x.abs() - one / two) * sixteen; - x += x * (x.abs() - one) * (nine / forty); + x -= num!(0.25) + (x + num!(0.25)).floor(); + x *= (x.abs() - num!(0.5)) * num!(16.); + x += x * (x.abs() - num!(1.)) * num!(0.225); x } From e894367c52b8629332d84665149076825b766f0b Mon Sep 17 00:00:00 2001 From: Corwin Date: Sun, 8 Oct 2023 14:56:32 +0100 Subject: [PATCH 3/5] use proper implementation of cos to check against --- agb-fixnum/src/lib.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/agb-fixnum/src/lib.rs b/agb-fixnum/src/lib.rs index 19e5a704..ea8d4adf 100644 --- a/agb-fixnum/src/lib.rs +++ b/agb-fixnum/src/lib.rs @@ -1230,7 +1230,10 @@ mod tests { #[test] fn check_cos_accuracy() { let n: Num = Num::new(1) / 32; - assert_eq!(n.cos(), num!(0.9808)); + assert_eq!( + n.cos(), + Num::from_f64((2. * core::f64::consts::PI / 32.).cos()) + ); } #[test] From 71e680f3655d511494013aeac9798e520b5834d6 Mon Sep 17 00:00:00 2001 From: Corwin Date: Sun, 8 Oct 2023 15:04:57 +0100 Subject: [PATCH 4/5] update changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6b03400b..90bbd777 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- Fixed inaccuracy in cosine implementation caused by accidentally multiplying correction term by zero. + ## [0.17.1] - 2023/10/05 ### Fixed From 09331d1cb5870fe2a2f8d12389439d372d4bccb4 Mon Sep 17 00:00:00 2001 From: Corwin Date: Mon, 9 Oct 2023 19:16:24 +0100 Subject: [PATCH 5/5] make 16 bit precision work --- agb-fixnum/src/lib.rs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/agb-fixnum/src/lib.rs b/agb-fixnum/src/lib.rs index ea8d4adf..8b0d1b0c 100644 --- a/agb-fixnum/src/lib.rs +++ b/agb-fixnum/src/lib.rs @@ -133,7 +133,7 @@ macro_rules! upcast_multiply_impl { .wrapping_mul(b_frac) .wrapping_add(b_floor.wrapping_mul(a_frac)), ) - .wrapping_add(a_frac.wrapping_mul(b_frac) >> n) + .wrapping_add(((a_frac as u32).wrapping_mul(b_frac as u32) >> n) as $T) } }; ($T: ty, $Upcast: ty) => { @@ -1236,6 +1236,17 @@ mod tests { ); } + #[test] + fn check_16_bit_precision_i32() { + let a: Num = num!(1.923); + let b = num!(2.723); + + assert_eq!( + a * b, + Num::from_raw(((a.to_raw() as i64 * b.to_raw() as i64) >> 16) as i32) + ) + } + #[test] fn test_numbers() { // test addition