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

issue-109, benchmarks were improved #120

Merged
merged 1 commit into from
Sep 17, 2024
Merged

issue-109, benchmarks were improved #120

merged 1 commit into from
Sep 17, 2024

Conversation

testisnullus
Copy link
Contributor

@testisnullus testisnullus commented Sep 11, 2024

Benchmarks were improved by reading sample data from the real .wbro file

Closes #109

@testisnullus testisnullus self-assigned this Sep 11, 2024
@testisnullus testisnullus added enhancement New feature or request opensource-release Issues related to the opensource release of the repo labels Sep 11, 2024
@testisnullus testisnullus force-pushed the issue-109 branch 2 times, most recently from 79b278c to 3895065 Compare September 11, 2024 10:28
Comment on lines 11 to 12
fn prepare_test_dir() -> PathBuf {
let test_dir = tempfile::tempdir().unwrap().into_path();
fs::copy(TEST_WBRO_PATH, test_dir.join(TEST_FILE_NAME)).unwrap();
test_dir
}

/// Loads the file, decompresses it using fft_to_data, and returns the decompressed Vec<f64>.
fn load_data_from_wbro_file() -> Vec<f64> {
let test_dir = prepare_test_dir();

let test_file_path = test_dir.join(TEST_FILE_NAME);

WavBrro::from_file(&test_file_path).unwrap()
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe prepare_test_dir here is redundant because the benchmarks work only with f64 samples, not the wavbrro itself. So, you may just read the samples from it. Also, the comment doesn't fit the content. load_data_from_wbro_file doesn't decompress the data, it only reads f64 samples from wavbrros.

Suggested change
fn prepare_test_dir() -> PathBuf {
let test_dir = tempfile::tempdir().unwrap().into_path();
fs::copy(TEST_WBRO_PATH, test_dir.join(TEST_FILE_NAME)).unwrap();
test_dir
}
/// Loads the file, decompresses it using fft_to_data, and returns the decompressed Vec<f64>.
fn load_data_from_wbro_file() -> Vec<f64> {
let test_dir = prepare_test_dir();
let test_file_path = test_dir.join(TEST_FILE_NAME);
WavBrro::from_file(&test_file_path).unwrap()
}
/// Loads the file, decompresses it using fft_to_data, and returns the decompressed Vec<f64>.
fn load_data_from_wbro_file() -> Vec<f64> {
WavBrro::from_file(&test_file_path).unwrap()
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, my bad, will be fixed


// Basic FFT compression benchmark
/// Basic FFT compression benchmark with decompressed data
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you should revert it because the data provided by load_data_from_wbro_file is not decompressed

@cjrolo cjrolo merged commit 6634d37 into main Sep 17, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request opensource-release Issues related to the opensource release of the repo
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve benchmarks
3 participants