Skip to content

Commit

Permalink
Move twiddles fwd * rev = identity test
Browse files Browse the repository at this point in the history
- Testing that all the values in the forward twiddle factors pointwise
multiplied by all the values in the inverse twiddle factors is more of a
unit test for twiddles. It made no sense to keep it in the planner.

- The previous commit modifies the planner in a way that this test would
  no longer pass. We can get the same, good coverage by keeping this
  test under twiddles.
  • Loading branch information
smu160 committed May 3, 2024
1 parent e150db3 commit 44b80a6
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 39 deletions.
40 changes: 1 addition & 39 deletions src/planner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ macro_rules! impl_planner_for {
/// # Panics
///
/// Panics if `num_points < 1` or if `num_points` is __not__ a power of 2.
pub fn new(num_points: usize, direction: Direction) -> Self {
pub fn new(num_points: usize, _direction: Direction) -> Self {
assert!(num_points > 0 && num_points.is_power_of_two());
if num_points <= 4 {
return Self {
Expand Down Expand Up @@ -73,8 +73,6 @@ impl_planner_for!(Planner32, f32, generate_twiddles_simd_32);

#[cfg(test)]
mod tests {
use utilities::assert_float_closeness;

use super::*;

macro_rules! test_no_twiddles {
Expand All @@ -91,40 +89,4 @@ mod tests {

test_no_twiddles!(no_twiddles_64, Planner64);
test_no_twiddles!(no_twiddles_32, Planner32);

macro_rules! forward_mul_inverse_eq_identity {
($test_name:ident, $planner:ty) => {
#[test]
fn $test_name() {
for i in 3..25 {
let num_points = 1 << i;
let planner_forward = <$planner>::new(num_points, Direction::Forward);
let planner_reverse = <$planner>::new(num_points, Direction::Reverse);

assert_eq!(
planner_reverse.num_twiddles(),
planner_forward.num_twiddles()
);

// (a + ib) (c + id) = ac + iad + ibc - bd
// = ac - bd + i(ad + bc)
planner_forward
.twiddles_re
.iter()
.zip(planner_forward.twiddles_im.iter())
.zip(planner_reverse.twiddles_re.iter())
.zip(planner_reverse.twiddles_im)
.for_each(|(((a, b), c), d)| {
let temp_re = a * c - b * d;
let temp_im = a * d + b * c;
assert_float_closeness(temp_re, 1.0, 1e-2);
assert_float_closeness(temp_im, 0.0, 1e-2);
});
}
}
};
}

forward_mul_inverse_eq_identity!(forward_reverse_eq_identity_64, Planner64);
forward_mul_inverse_eq_identity!(forward_reverse_eq_identity_32, Planner32);
}
45 changes: 45 additions & 0 deletions src/twiddles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -300,4 +300,49 @@ mod tests {
}
}
}

macro_rules! forward_mul_inverse_eq_identity {
($test_name:ident, $generate_twiddles_simd_fn:ident) => {
#[test]
fn $test_name() {
for i in 3..25 {
let num_points = 1 << i;
let dist = num_points >> 1;

let (fwd_twiddles_re, fwd_twiddles_im) = if dist >= 8 * 2 {
$generate_twiddles_simd_fn(dist, Direction::Forward)
} else {
generate_twiddles(dist, Direction::Forward)
};

assert_eq!(fwd_twiddles_re.len(), fwd_twiddles_im.len());

let (rev_twiddles_re, rev_twiddles_im) = if dist >= 8 * 2 {
$generate_twiddles_simd_fn(dist, Direction::Reverse)
} else {
generate_twiddles(dist, Direction::Reverse)
};

assert_eq!(rev_twiddles_re.len(), rev_twiddles_im.len());

// (a + ib) (c + id) = ac + iad + ibc - bd
// = ac - bd + i(ad + bc)
fwd_twiddles_re
.iter()
.zip(fwd_twiddles_im.iter())
.zip(rev_twiddles_re.iter())
.zip(rev_twiddles_im.iter())
.for_each(|(((a, b), c), d)| {
let temp_re = a * c - b * d;
let temp_im = a * d + b * c;
assert_float_closeness(temp_re, 1.0, 1e-2);
assert_float_closeness(temp_im, 0.0, 1e-2);
});
}
}
};
}

forward_mul_inverse_eq_identity!(forward_reverse_eq_identity_64, generate_twiddles_simd_64);
forward_mul_inverse_eq_identity!(forward_reverse_eq_identity_32, generate_twiddles_simd_32);
}

0 comments on commit 44b80a6

Please sign in to comment.