Removing std dependencies from num-traits and num-complex

[michaelciraci]

Closes #1

[VorpalBlade]

Trying to look at this in Rust-Analyzer isn't working well: I get "macro invocation exceeds token limit: produced 4744974 tokens, limit is 2097152" on lib.rs line 64. Also my computer is too old for it to really agree with this project I think (I have a Skylake era mobile i7). Building takes minutes.

I was wondering to what extent you could use FloatCore instead of Float. That way you wouldn't even need the libm feature.

I do believe your current approach would work but there are two possible caveats I would see with it:

1. When building tests you pull in rustfft, which also depends on num-traits and num-complex. You are going to get cargo feature unification then. So the num-complex/num-traits configuration will differ between when you run cargo build and cargo test. This might be quite the gotcha if trying to benchmark for example.
2. People who are not targeting embedded might prefer to have std rather than libm. I couldn't find information about the performance characteristics of libm from a quick google. But if it is much slower that could be an issue.

I don't know that there is a fix for issue 1, but for issue 2 you could do this by forwarding the features:

[features]
default = ["std"]
std = ["num-complex/std", "num-traits/std"]
libm = ["num-complex/libm", "num-traits/libm"]

[dependencies]
monarch-derive = { path = "crates/monarch-derive" }
num-complex = { version = "0.4", default-features = false }
num-traits = { version = "0.2", default-features = false }

Then your user gets to decide if they use std or libm. I don't think there is an automatic way to say "enable libm if std is disabled". I believe there is an open RFC about exclusive features for Cargo, but this is not even implemented for nightly yet. You could add an assert near the top (above the use statements) of lib.rs like this to give users better errors though:

#[cfg(not(any(feature = "std", feature = "libm")))]
compile_error!("At least one of std and libm must be enabled");

The downside of this is now you have two different configurations to test. I don't know that is something you want to do. Two configs isn't too bad, but features risk being a combinatorial explosion if you get too many of them.

Also testing this properly gets complicated due to issue 1 (feature unification with rustfft) that I mentioned above. Some thought is needed for how to properly set up the CI. You would need to cargo build and cargo test separately.

[VorpalBlade]

Looks like your benchmarks also pull in rustfft & fftw, so you would get feature unification there. You could test libm vs std by patching out those temporarily to check what numbers you get.

[michaelciraci]

Yes, right now the macros build for FFT sizes 1-200 but I'd like a more elegant solution to just build the FFT sizes you need, and then rust-analyzer would have a way easier time.

I don't think I could use FloatCore, because I need sin/cos which are used in this call stack: https://docs.rs/num-complex/latest/num_complex/struct.Complex.html#method.exp. And at that point I would need libm. If this requires libm, I don't believe feature unification would be an issue because there are no allocations needed anyways, so the benchmarks should be consistent.