Add FFT in Rust #688

fanninpm · 2020-05-06T19:11:13Z

@Liikt (and maybe @strega-nil) I felt like I had to do something ~~dirty~~ a little bit inelegant in order to make Clippy shut up. (You'll see what I mean.)

This code is incomplete right now. (It hasn't resolved all compiler warnings/errors, and I haven't put it through Clippy yet.)

Liikt

If you change all the &[Complex<f64>] to Vec<Complex<f64>> then you can drop the references and just move the value into the functions.

berquist

Maybe a big ask, but I think that, like at least the Julia and C implementations, this should compare to FFTW. It looks like the de facto crate is https://crates.io/crates/fftw.

berquist · 2020-05-24T03:54:09Z

contents/cooley_tukey/code/rust/fft.rs

+            let mut v = Complex::new(1.0_f64, 0.0_f64);
+            for k in 0..((stride / 2) as usize) {
+                let k_plus_j = new_x[k + j];
+                let k_plus_j_plus_half_stride = new_x[k + j + ((stride / 2) as usize)];


I think this intermediate and k_plus_j_plus_half_stride_2, since they're only used in one place each, don't buy you anything and make the code harder to read.

In getting rid of those three intermediates, I had to sacrifice the -= operator. If I didn't do that, the compiler warns that I "cannot borrow new_x as immutable because it is also borrowed as mutable".

berquist · 2020-05-24T03:56:45Z

contents/cooley_tukey/code/rust/fft.rs

+                })
+                .sum()
+        })
+        .collect::<Vec<_>>()


The turbofish isn't necessary here due to the return type in the function signature.

berquist · 2020-05-24T04:09:00Z

contents/cooley_tukey/code/rust/fft.rs

+    }
+    let y = cooley_tukey(&x.clone());
+    let z = iterative_cooley_tukey(&x.clone());
+    let t = dft(&x.clone());


Just noticed, these don't need to be cloned either.

I also think that you should leave the functions as not taking ownership.

We don't need to borrow a clone of an object if we only need to borrow the object.

fanninpm · 2020-05-24T19:26:43Z

Maybe a big ask, but I think that, like at least the Julia and C implementations, this should compare to FFTW. It looks like the de facto crate is https://crates.io/crates/fftw.

@berquist I'll try to add references to fftw later. (Note that #686 actually uses a different crate that offers a pure Rust FFT implementation.)

berquist · 2020-05-24T20:56:20Z

If you think using rustfft is better than fftw, then go ahead, I have no preference. Which crate is used is pretty clear from the import. Since we don't have Cargo.toml, you might want to specify in a code comment which version(s) you're using.

fanninpm · 2020-06-29T17:58:05Z

@berquist I have implemented rustfft into a new function fft(). Is there anything else that needs to be addressed before this gets merged?

berquist · 2020-07-05T00:56:59Z

I think this will be fine, but is there a better way for me to test this other than creating a temporary Cargo package and running it inside?

jdonszelmann · 2020-07-05T08:45:44Z

I tested my last submission with just rustc. rustc myprogram.rs && ./myprogram but that's a lot harder when using libraries etc. I think this is an issue fore more languages where you need some sort of project structure for the program to work properly. Maybe include this structure in the AAA? I know it might clutter up some things a bit, but new users may not know that this structure is required, and for those it might actually be very useful.

fanninpm · 2020-07-05T15:20:10Z

is there a better way for me to test this other than creating a temporary Cargo package and running it inside?

@berquist @jonay2000 Currently, there isn't any way to test this without doing the above. (See #691 for further description.)

berquist · 2020-07-05T21:07:47Z

I'm getting

error[E0308]: mismatched types
  --> src/main.rs:19:25
   |
19 |     transformer.process(&mut new_x, &mut y);
   |                         ^^^^^^^^^^ expected slice, found struct `std::vec::Vec`
   |
   = note: expected mutable reference `&mut [rustfft::num_complex::Complex<_>]`
              found mutable reference `&mut std::vec::Vec<num::Complex<f64>>`

error[E0308]: mismatched types
  --> src/main.rs:19:37
   |
19 |     transformer.process(&mut new_x, &mut y);
   |                                     ^^^^^^ expected slice, found struct `std::vec::Vec`
   |
   = note: expected mutable reference `&mut [rustfft::num_complex::Complex<_>]`
              found mutable reference `&mut std::vec::Vec<num::Complex<f64>>`

with num = 0.3.0, rand = 0.7.3, and rustfft = 3.0.1. Which version of rustfft should I use?

berquist

Great, thanks!

Moving the arguments into the functions doesn't seem important, since you don't need to take ownership of the input, unless you can figure out how to do the operations in-place.

…y fixes from algorithm-archivists#688

* Add rust builder and build script * Move file to be discovered by SCons, and update Cargo.toml to reflect this * Add rustc to SCons * Add Cargo.toml files for code requiring libraries * Add cargo building when Cargo.toml is present * Fix copying fail on linux due to directory name and file name collision * Add build artifacts to SCons clean command and .gitignore * Fix Not a directory issue due to getting parent directory of a file * Remove redefinition of languages dictionary * Add rustc and cargo install to docker * Update Cooley-Tukey to use correct version of crates * Update split-operator method to use correct library versions and apply fixes from #688

fanninpm added 2 commits May 6, 2020 12:46

Implement FFT in Rust

6dd00c5

This code is incomplete right now. (It hasn't resolved all compiler warnings/errors, and I haven't put it through Clippy yet.)

Finished FFT implementation in Rust

5a96451

Liikt previously requested changes May 20, 2020

View reviewed changes

berquist added the Implementation This provides an implementation for an algorithm. (Code and maybe md files are edited.) label May 24, 2020

berquist requested changes May 24, 2020

View reviewed changes

berquist reviewed May 24, 2020

View reviewed changes

fanninpm added 3 commits May 24, 2020 15:15

Remove intermediates for readability

a729948

Remove turbofish in line 22

cd676a3

Remove extraneous clones

d31b011

We don't need to borrow a clone of an object if we only need to borrow the object.

Add rustfft reference

6841071

Fix mismatched types

5b82222

berquist approved these changes Jul 5, 2020

View reviewed changes

berquist merged commit 8e6cb6a into algorithm-archivists:master Jul 5, 2020

fanninpm deleted the fft_in_rust branch July 5, 2020 23:51

PeanutbutterWarrior added a commit to PeanutbutterWarrior/algorithm-archive that referenced this pull request Nov 28, 2021

Update split-operator method to use correct library versions and appl…

b468db1

…y fixes from algorithm-archivists#688

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add FFT in Rust #688

Add FFT in Rust #688

fanninpm commented May 6, 2020

Liikt left a comment

berquist left a comment

berquist May 24, 2020

fanninpm May 24, 2020

berquist May 24, 2020

berquist May 24, 2020

fanninpm commented May 24, 2020

berquist commented May 24, 2020

fanninpm commented Jun 29, 2020

berquist commented Jul 5, 2020

jdonszelmann commented Jul 5, 2020 •

edited

Loading

fanninpm commented Jul 5, 2020

berquist commented Jul 5, 2020

berquist left a comment •

edited

Loading

Add FFT in Rust #688

Add FFT in Rust #688

Conversation

fanninpm commented May 6, 2020

Liikt left a comment

Choose a reason for hiding this comment

berquist left a comment

Choose a reason for hiding this comment

berquist May 24, 2020

Choose a reason for hiding this comment

fanninpm May 24, 2020

Choose a reason for hiding this comment

berquist May 24, 2020

Choose a reason for hiding this comment

berquist May 24, 2020

Choose a reason for hiding this comment

fanninpm commented May 24, 2020

berquist commented May 24, 2020

fanninpm commented Jun 29, 2020

berquist commented Jul 5, 2020

jdonszelmann commented Jul 5, 2020 • edited Loading

fanninpm commented Jul 5, 2020

berquist commented Jul 5, 2020

berquist left a comment • edited Loading

Choose a reason for hiding this comment

jdonszelmann commented Jul 5, 2020 •

edited

Loading

berquist left a comment •

edited

Loading