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

FFT compressor improvements #53

Merged
merged 1 commit into from
Oct 18, 2023
Merged

FFT compressor improvements #53

merged 1 commit into from
Oct 18, 2023

Conversation

cjrolo
Copy link
Collaborator

@cjrolo cjrolo commented Oct 17, 2023

Currently the FFT compressor has a different signature than all the other compressors. One idea was to get the optimizer to do some work and set the FFT parametrization via other means.

So, the rational for this PR is the following:

  1. FFT have the same base function signature that the others, so a call to FFT will have a sane default
  2. New FFT signatures for dealing with either set frequencies or maximum error allowed
    • This can be called explicitly when required without interfering with a normal flow of the code (They are exceptional cases anyway)

Bugfixes:

  • FFT ID variable name
  • Greatly increased the performance of the FFT calculations for best error approximations (Still heavy)

Tests were adjusted for the new signatures and new defaults.

Copy link
Contributor

@rukai rukai left a comment

Choose a reason for hiding this comment

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

All very minor issues except the calculate_error issue which will tank your performance unless the compiler pulls off some heroic optimizations.

Feel free to merge now and then investigate these in a followup PR.

brro-compressor/src/compressor/fft.rs Show resolved Hide resolved
brro-compressor/src/compressor/fft.rs Show resolved Hide resolved
brro-compressor/src/compressor/fft.rs Show resolved Hide resolved
brro-compressor/src/compressor/fft.rs Show resolved Hide resolved
brro-compressor/src/compressor/fft.rs Show resolved Hide resolved
@cjrolo cjrolo merged commit ac5c2d8 into main Oct 18, 2023
@cjrolo cjrolo deleted the compressor-improvements branch October 18, 2023 08:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants