Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for f32, as well as f64 #17

Merged
merged 9 commits into from
Apr 1, 2024
Merged

Conversation

smu160
Copy link
Member

@smu160 smu160 commented Mar 18, 2024

This PR should address issue #14

@smu160 smu160 added the enhancement New feature or request label Mar 18, 2024
@smu160 smu160 requested a review from Shnatsel March 18, 2024 15:58
@smu160 smu160 linked an issue Mar 18, 2024 that may be closed by this pull request
@Shnatsel
Copy link
Collaborator

It seems like it fails to build on CI with a syntax error?

@smu160
Copy link
Member Author

smu160 commented Mar 19, 2024

@Shnatsel

Yes. Sorry, I should have converted this to a draft. I have local changes almost ready to check-in. They should address the aforementioned issue, as well as others.

@smu160 smu160 marked this pull request as draft March 19, 2024 16:11
@smu160 smu160 self-assigned this Mar 19, 2024
@codecov-commenter
Copy link

codecov-commenter commented Mar 20, 2024

Codecov Report

Attention: Patch coverage is 98.00000% with 7 lines in your changes are missing coverage. Please review.

Project coverage is 97.25%. Comparing base (a63ad3f) to head (4b909ae).

Files Patch % Lines
src/lib.rs 93.85% 7 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #17      +/-   ##
==========================================
+ Coverage   94.28%   97.25%   +2.96%     
==========================================
  Files           7        7              
  Lines         805      765      -40     
==========================================
- Hits          759      744      -15     
+ Misses         46       21      -25     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@smu160 smu160 marked this pull request as ready for review March 28, 2024 20:27
@@ -102,7 +102,7 @@ macro_rules! impl_fft_with_opts_and_plan_for {
if t < n - 1 {
filter_twiddles(twiddles_re, twiddles_im);
}
if chunk_size >= 16 {
if chunk_size >= $lanes * 2 {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Shnatsel Now we just use # of lanes * 2 when executing this check

@@ -49,7 +49,7 @@ macro_rules! fft_butterfly_n_simd {
}

fft_butterfly_n_simd!(fft_64_chunk_n_simd, f64, 8, f64x8);
fft_butterfly_n_simd!(fft_32_chunk_n_simd, f32, 8, f32x8);
fft_butterfly_n_simd!(fft_32_chunk_n_simd, f32, 16, f32x16);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Shnatsel Bumped up # of lanes to 16 when using AVX-512 and f32

Copy link
Member Author

@smu160 smu160 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Shnatsel I think this should be good to go now

@smu160 smu160 merged commit a65b542 into main Apr 1, 2024
8 checks passed
@smu160 smu160 deleted the feature/single-precision branch May 3, 2024 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for f32 in FFT computations
3 participants