Repository navigation
feat: add fft/rfftf - #16089
feat: add fft/rfftf#16089gunjjoshi wants to merge 2 commits into
fft/rfftf#16089Conversation
---
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes.
report:
- task: lint_filenames
status: passed
- task: lint_editorconfig
status: passed
- task: lint_markdown_pkg_readmes
status: passed
- task: lint_markdown_docs
status: na
- task: lint_markdown
status: na
- task: lint_package_json
status: passed
- task: lint_repl_help
status: passed
- task: lint_javascript_src
status: passed
- task: lint_javascript_cli
status: na
- task: lint_javascript_examples
status: passed
- task: lint_javascript_tests
status: passed
- task: lint_javascript_benchmarks
status: passed
- task: lint_python
status: na
- task: lint_r
status: na
- task: lint_c_src
status: na
- task: lint_c_examples
status: na
- task: lint_c_benchmarks
status: na
- task: lint_c_tests_fixtures
status: na
- task: lint_shell
status: na
- task: lint_typescript_declarations
status: passed
- task: lint_typescript_tests
status: passed
- task: lint_license_headers
status: passed
---
|
|
||
| // VARIABLES // | ||
|
|
||
| var idtypes = dtypes( 'real_and_generic' ); |
There was a problem hiding this comment.
Used real_and_generic here instead of real_floating_point_and_generic, so that it can handle integer arrays too.
| /** | ||
| * Computes the forward FFT of a real-valued input. | ||
| * | ||
| * @private | ||
| * @param {ArrayLikeObject<Object>} arrays - array-like object containing ndarrays | ||
| * @returns {void} | ||
| */ | ||
| function gkernel( arrays ) { | ||
| kernel( reinterpret, grffti, grfftf, Float64Array, arrays ); | ||
| } | ||
|
|
||
| /** | ||
| * Computes the forward FFT of a real-valued double-precision input. | ||
| * | ||
| * @private | ||
| * @param {ArrayLikeObject<Object>} arrays - array-like object containing ndarrays | ||
| * @returns {void} | ||
| */ | ||
| function dkernel( arrays ) { | ||
| kernel( reinterpret128, drffti, drfftf, Float64Array, arrays ); | ||
| } | ||
|
|
||
| /** | ||
| * Computes the forward FFT of a real-valued single-precision input. | ||
| * | ||
| * @private | ||
| * @param {ArrayLikeObject<Object>} arrays - array-like object containing ndarrays | ||
| * @returns {void} | ||
| */ | ||
| function skernel( arrays ) { | ||
| kernel( reinterpret64, srffti, srfftf, Float32Array, arrays ); | ||
| } |
There was a problem hiding this comment.
Needed these wrappers to wrap our strided kernels, since we have a custom logic here, to avoid an explicit hc2c conversion.
| // Copy the input into the output buffer at offset 1: | ||
| gcopy( N, getData( x ), getStride( x, 0 ), getOffset( x ), view, 1, 1 ); | ||
|
|
||
| // Initialize the workspace and compute the forward FFT in-place: | ||
| w = new Workspace( ( 2*N ) + 34 ); | ||
| rffti( N, w, 1, 0 ); | ||
| rfftf( N, view, 1, 1, w, 1, 0 ); | ||
|
|
||
| // Move the DC component from the imaginary slot to the real slot of the first complex element and zero out the imaginary part: | ||
| view[ 0 ] = view[ 1 ]; | ||
| view[ 1 ] = 0.0; | ||
|
|
||
| // For even-length sequences, zero out the imaginary part of the Nyquist component: | ||
| if ( isEven( N ) ) { | ||
| view[ ( 2*M ) - 1 ] = 0.0; | ||
| } | ||
| } |
There was a problem hiding this comment.
This is the approach that we discussed in #fft > hc2c @ 💬.
Coverage Report
The above coverage report was generated for the changes in this PR. |
| return; | ||
| } | ||
| // Copy the input into the output buffer at offset 1: | ||
| gcopy( N, getData( x ), getStride( x, 0 ), getOffset( x ), view, 1, 1 ); |
There was a problem hiding this comment.
I am thinking you should pass in one of dcopy, scopy, or gcopy here. The former can be significantly faster. Should be straightforward to update your kernel wrappers below.
| * var out = new ndarray( 'complex128', new Complex128Array( 3 ), [ 3 ], [ 1 ], 0, 'row-major' ); | ||
| * | ||
| * // Perform operation: | ||
| * var v = rfftf( out, x ); |
There was a problem hiding this comment.
This call signature seems backward. Why is out first and x second?
There was a problem hiding this comment.
Okay. I see the issue. The main thing is that you are thinking that you need to provide a single kernel which combines both initialization AND computation. That isn't the case.
And importantly, you are missing one of the very advantages of the workspace array. Notice above that you are creating a new workspace for each subarray. That is unnecessary and you forgot the guidance when porting FTTPACK. Namely, for constant length N, you can reuse the workspace array for each computed FFT.
Which means, in main.js, assign.js, or similar, what you do is you allocate a single workspace array and initialize once. That workspace array should then become an auxiliary array which is passed down with the subarray view to the ndarray wrapper, using nullary-*.
So, in short, you need to refactor this implementation.
| if ( n !== N ) { | ||
| if ( n < N ) { | ||
| // Create a view of the first n elements: | ||
| x = sliceTo( x, [ n ], false, false ); | ||
| } else { | ||
| // Create a new array of length n and copy the first N elements: | ||
| y = baseZeros( getDType( x ), [ n ], getOrder( x ) ); | ||
| gcopy( N, getData( x ), getStride( x, 0 ), getOffset( x ), getData( y ), 1, 0 ); // eslint-disable-line max-len | ||
| x = y; | ||
| } | ||
| } |
There was a problem hiding this comment.
We need to discuss this, as this implementation is not correct.
You are assuming a one-dimensional input ndarray. This is not correct. A user should be able to specify a dim (default: -1), meaning that a user can batch an FFT along one or more tensor fibers.
| } else { | ||
| s = 1.0 / n; | ||
| } | ||
| gscal( 2*M, s, view, 1, 0 ); |
There was a problem hiding this comment.
This isn't going to work, as you are assuming a 1D input ndarray. We need to add blas/scal.
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes. report:
Resolves stdlib-js/metr-issue-tracker#1385.
Description
This pull request:
fft/rfftf.Related Issues
This pull request has the following related issues:
fft/rfftfmetr-issue-tracker#1385.Questions
No.
Other
No.
Checklist
AI Assistance
If you answered "yes" above, how did you use AI assistance?
Disclosure
Used Claude Code to help with the implementation, tests, and docs. Also used it to understand multiple things and do the necessary research work.
@stdlib-js/reviewers