added chirp-z transform to calculate non-power-of-2 fft#2900
Merged
Conversation
Owner
|
Thanks cyavictor88! This looks good, and well tested. I guess you mean it adresses #2577 :) There would indeed be a circular dependency between ifft and fft. Since ifft is only two lines of code, I think it's ok to duplicate it. Alternatively, we could pull out the logic in an internal util function and reuse it. What does have your preference? |
Contributor
Author
|
Hi, thanks for pointing out the mislabeling! I am ok with leaving that one line of ifft operation inside the _czf function. I actually think it is more straight forward for people who want to read the code :) Thanks! |
Owner
|
Ok sounds good. Indeed, too much indirection makes code harder to read, we should be pragmatic. Will merge your PR now, thanks again! |
Owner
|
Published now in |
jakubriegel
pushed a commit
to jakubriegel/mathjs
that referenced
this pull request
Mar 1, 2023
…ong#2900) * added chirp-z transform to calculate non-power-of-2 fft * simplify/remove _ifft function inside _czt function
josdejong
added a commit
that referenced
this pull request
Mar 9, 2023
* #2567: accept array as parameter for gcd() * #2567: accept 1d matrix as gcd() argument * #2567: support nested 1d array in gcd * #2567: simplify matrix signature * [fix] intersect method parameter type (#2897) * Update history and authors (see #2897) * feat: added chirp-z transform to calculate non-power-of-2 fft (#2900) * added chirp-z transform to calculate non-power-of-2 fft * simplify/remove _ifft function inside _czt function * chore: remove an unused dependency from `simplifyConstant` * fix: quantileSeq not accepting a matrix as second argument `prob` (see #2902) * fix a broken example of function `to` * fix a typo in the examples functions `distance`, `getMatrixDataType`, `subset`, and `max` (see #2902) * fix linting issue * Broadcasting (#2895) * broadcasting * Simplified broadcasting * Updated for broadcasting * Changed to camel case * Camel case and auto formating * Added comments * Skip if matrices have the same size * Fixed issue with undefined variable missing dot in `A._size` * Implemented broadcasting in all functions * Added helper functions * Added function to check for broadcasting rules * Tests for broadcasted arithmetic * Fixed issue with matrix the size of a vector * Documented and updated broadcasting * Included broadcast.test --------- Co-authored-by: David Contreras <david.contreras@guentner.com> Co-authored-by: Jos de Jong <wjosdejong@gmail.com> * Update history and authors * Update devDependencies * publish v11.6.0 * fix #2906: improve description of the behavior of `subset` for scalar values in the docs * fix #2907: determinant of empty matrix should be 1 * chore: add a few more unit tests to `det` --------- Co-authored-by: Jos de Jong <wjosdejong@gmail.com> Co-authored-by: Jaeu Jeong <wodndb@gmail.com> Co-authored-by: cyavictor88 <100557319+cyavictor88@users.noreply.github.com> Co-authored-by: David Contreras <dvd.cnt@gmail.com> Co-authored-by: David Contreras <david.contreras@guentner.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
for #2577
There is one concern, in the newly created
_cztfunction, I basically duplicate the ifft function, so its not really DRY. I tried to import 'ifft' withdependencieslist andfactoryfunction. But I when I try to run it, I gotError: Cannot resolve dependencies of factoryI wonder if it is because of circular dependency.