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

feat: add initial implementaion of fft/base/fftpack #4121

Draft
wants to merge 41 commits into
base: develop
Choose a base branch
from

Conversation

gunjjoshi
Copy link
Member

Progresses #3131

Description

What is the purpose of this pull request?

This pull request:

  • adds fft/base/fftpack.

Related Issues

Does this pull request have any related issues?

This pull request:

Questions

Any questions for reviewers of this pull request?

No.

Other

Any other information relevant to this pull request? This may include screenshots, references, and/or implementation notes.

No.

Checklist

Please ensure the following tasks are completed before submitting this pull request.


@stdlib-js/reviewers

@gunjjoshi gunjjoshi marked this pull request as draft December 21, 2024 05:52
@stdlib-bot stdlib-bot added Needs Review A pull request which needs code review. and removed Needs Review A pull request which needs code review. labels Dec 21, 2024
@gunjjoshi gunjjoshi changed the title feat: add initial implementtaion of fft/base/fftpack feat: add initial implementaion of fft/base/fftpack Dec 21, 2024
Signed-off-by: Gunj Joshi <gunjjoshi8372@gmail.com>
Signed-off-by: Gunj Joshi <gunjjoshi8372@gmail.com>
Signed-off-by: Gunj Joshi <gunjjoshi8372@gmail.com>
@kgryte kgryte added the Feature Issue or pull request for adding a new feature. label Dec 21, 2024
Signed-off-by: Gunj Joshi <gunjjoshi8372@gmail.com>
---
type: pre_push_report
description: Results of running various checks prior to pushing changes.
report:
  - task: run_javascript_examples
    status: na
  - task: run_c_examples
    status: na
  - task: run_cpp_examples
    status: na
  - task: run_javascript_readme_examples
    status: na
  - task: run_c_benchmarks
    status: na
  - task: run_cpp_benchmarks
    status: na
  - task: run_fortran_benchmarks
    status: na
  - task: run_javascript_benchmarks
    status: na
  - task: run_julia_benchmarks
    status: na
  - task: run_python_benchmarks
    status: na
  - task: run_r_benchmarks
    status: na
  - task: run_javascript_tests
    status: na
---
---
type: pre_push_report
description: Results of running various checks prior to pushing changes.
report:
  - task: run_javascript_examples
    status: na
  - task: run_c_examples
    status: na
  - task: run_cpp_examples
    status: na
  - task: run_javascript_readme_examples
    status: na
  - task: run_c_benchmarks
    status: na
  - task: run_cpp_benchmarks
    status: na
  - task: run_fortran_benchmarks
    status: na
  - task: run_javascript_benchmarks
    status: na
  - task: run_julia_benchmarks
    status: na
  - task: run_python_benchmarks
    status: na
  - task: run_r_benchmarks
    status: na
  - task: run_javascript_tests
    status: na
---
---
type: pre_push_report
description: Results of running various checks prior to pushing changes.
report:
  - task: run_javascript_examples
    status: na
  - task: run_c_examples
    status: na
  - task: run_cpp_examples
    status: na
  - task: run_javascript_readme_examples
    status: na
  - task: run_c_benchmarks
    status: na
  - task: run_cpp_benchmarks
    status: na
  - task: run_fortran_benchmarks
    status: na
  - task: run_javascript_benchmarks
    status: na
  - task: run_julia_benchmarks
    status: na
  - task: run_python_benchmarks
    status: na
  - task: run_r_benchmarks
    status: na
  - task: run_javascript_tests
    status: na
---
---
type: pre_push_report
description: Results of running various checks prior to pushing changes.
report:
  - task: run_javascript_examples
    status: na
  - task: run_c_examples
    status: na
  - task: run_cpp_examples
    status: na
  - task: run_javascript_readme_examples
    status: na
  - task: run_c_benchmarks
    status: na
  - task: run_cpp_benchmarks
    status: na
  - task: run_fortran_benchmarks
    status: na
  - task: run_javascript_benchmarks
    status: na
  - task: run_julia_benchmarks
    status: na
  - task: run_python_benchmarks
    status: na
  - task: run_r_benchmarks
    status: na
  - task: run_javascript_tests
    status: na
---
---
type: pre_push_report
description: Results of running various checks prior to pushing changes.
report:
  - task: run_javascript_examples
    status: na
  - task: run_c_examples
    status: na
  - task: run_cpp_examples
    status: na
  - task: run_javascript_readme_examples
    status: na
  - task: run_c_benchmarks
    status: na
  - task: run_cpp_benchmarks
    status: na
  - task: run_fortran_benchmarks
    status: na
  - task: run_javascript_benchmarks
    status: na
  - task: run_julia_benchmarks
    status: na
  - task: run_python_benchmarks
    status: na
  - task: run_r_benchmarks
    status: na
  - task: run_javascript_tests
    status: na
---
var i;
var k;

TI11 *= fsign;
Copy link
Member

Choose a reason for hiding this comment

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

Just checking. Seems a little strange to be mutating a variable in the parent scope.

Copy link
Member Author

Choose a reason for hiding this comment

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

In the reference implementation, these constants are multiplied with fsign at the time of declaration itself. But, as fsign comes as an argument to the passfb5 function, we can't use it in the // VARIABLES // section.

Copy link
Member Author

Choose a reason for hiding this comment

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

Should I move these declarations inside the function, similar to the reference implementation?

@gunjjoshi
Copy link
Member Author

I had a confusion here. If we see the reference implementations of cosqf and cosqf1, cosqf calls cosqf1 here, and passes pointers to the wsave arrays, as cosqf1 accepts pointers to that array.
But in the current JS implementation in this PR, I am passing a single element of the array here, while cosqf1 takes an entire array as an argument here. This might not be the right approach. Is there any other way to achieve the same behavior as shown in the reference C implementation?

@gunjjoshi
Copy link
Member Author

Should I follow something like what we are doing in the C and JS implementations of modf?

@kgryte
Copy link
Member

kgryte commented Jan 19, 2025

@gunjjoshi It's a little hard to follow what you are asking given the various links. Can you provide relevant snippets in a single comment so we can see what you are referencing in one view?

@gunjjoshi
Copy link
Member Author

gunjjoshi commented Jan 19, 2025

@kgryte Ahh, sorry for the confusion.

This is the cosqf1 function:

static void cosqf1(integer n, real *x, real *w, real *xh)

This cosqf1 is used within the cosqf function as:

cosqf1(n, &x[1], &wsave[1], &wsave[n + 1]);

In order to replicate this same behavior in JS, currently, what I'm doing is:

cosqf1:

function cosqf1( n, x, w, xh )

where n is a number, while x, w and xh are Float64Arrays.

And, while calling this cosqf1 from cosqf, we are currently using it as:

cosqf1( n, x[ 1 - 1 ], wsave[ 1 - 1 ], wsave[ n + 1 - 1 ] );

But in this JS version, instead of passing arrays, I am currently passing single elements, such as wsave[ n + 1 - 1 ].
This seems to be wrong. Is there any other way of passing arrays here, but from a specific index (something like a subarray)?

I think this problem might be summed up as, how should we replicate the logic of passing subarrays (using pointers) in C, for the JS implementation?

@kgryte
Copy link
Member

kgryte commented Jan 19, 2025

@gunjjoshi What about providing an array and then offsets, similar in concept to the ndarray APIs we have. E.g.,

xptr = ...;
wptr = ...;
xhptr = ...;

cosqf1(n, x, xptr, wsave, wptr, xhptr ); // =>cosqf1( n: number, x: Float64Array, xptr: number, wsave: Float64Array, wptr: number, xhptr: number )

where the *ptr variables are indices into the respective arrays.

Another alternative is that you pass down slices which has more overhead due to object creation.

cosqf1( n, x.slice( 1 ), wsave.slice( 1 ), wsave.slice( n+1 ) )

@gunjjoshi
Copy link
Member Author

Thanks, @kgryte, I'll go with the first method!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Issue or pull request for adding a new feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants