Repository navigation
feat: add fft/base/fftpack/float32/rffti - #14591
Conversation
---
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: passed
- task: lint_shell
status: na
- task: lint_typescript_declarations
status: passed
- task: lint_typescript_tests
status: passed
- task: lint_license_headers
status: passed
---
Coverage Report
The above coverage report was generated for the changes in this PR. |
| # @param {(string|void)} fPIC - flag indicating whether to generate position independent code | ||
| #/ | ||
| $(c_targets): %.out: %.c | ||
| $(QUIET) $(CC) $(CFLAGS) $(fPIC) -o $@ $(FFTPACK_SRC) $< -lm |
There was a problem hiding this comment.
Not having the DFFTPACK_DOUBLE_PRECISION flag here, which is present in https://github.com/stdlib-js/stdlib/blob/develop/lib/node_modules/%40stdlib/fft/base/fftpack/float64/rffti/test/fixtures/c/fftpack/Makefile#L104, since this is for single-precision. Ref: https://github.com/marton78/pffft/blob/0b4ee12c4ba45a4a8e567550c16d96d1679f50ce/src/fftpack.c#L60
In FFTPACK, they reinterpret the workspace memory as an `int` array. As a consequence, we should emulate integer ops, not float ops. In this case, this just means floor division. Signed-off-by: Athan <kgryte@gmail.com>
kgryte
left a comment
There was a problem hiding this comment.
LGTM. Removed some of the float32 emulation after looking at the reference implementation. In particular, FFTPACK reinterprets real memory as int and then performs integer ops, not float ops. So, in order to avoid accuracy issues once exceeding the maximum safe float32 integer, I removed various f32 wrapper calls and already updated float32/decompose, as well.
We could, in principle, exactly mimic their logic. Nothing is stopping us from creating a new, say, Int32Array view over the underlying buffer, but I am fine keeping as is, for now.
It is possible that users will want to compute an FFT having more than 16.7M elements
In [42]: FLOAT32_MAX_SAFE_INTEGER
Out[42]: 16777215
but we will cross that bridge when we get there, as things get a bit messy if you need to support int64 arrays (which I don't think we do (!), as surely even the largest practical sequence length will have less than 2^31-1 factors). And given that factors would be written as int32 and not float, it would make fixture and example logic more complicated. But this is also doable, as would require creating an int32 array view to reinterpret raw memory.
We should discuss more during our next sync.
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes. report:
Resolves stdlib-js/metr-issue-tracker#1215.
Description
This pull request:
fft/base/fftpack/float32/rffti, which will be the single-precision variant offft/base/fftpack/generic/rffti.Related Issues
This pull request has the following related issues:
fft/base/fftpack/float32/rfftimetr-issue-tracker#1215Questions
No.
Other
No.
Checklist
AI Assistance
If you answered "yes" above, how did you use AI assistance?
Disclosure
Used Claude Code to review my implementation and to do some R&D.
@stdlib-js/reviewers