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

Extend HDF5 compression options #332

Open
wants to merge 23 commits into
base: master
Choose a base branch
from
Open

Conversation

djps
Copy link
Collaborator

@djps djps commented Feb 27, 2024

Closes #236 by expanding the hdf5 compression options to include lzf and also szip in addition to the standard gzip default options (which are integers for the compression level). As such the variable is renamed from compression_level to compression_option.

  • The default compression option in get_h5_literals is changed to 4 to match that of the h5py library.
  • As the matlab function h5create can not write with lzf compression a test is created in h5io_test
  • szip is not tested as it must be installed separately.

Copy link

codecov bot commented Feb 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.50%. Comparing base (d8f56f3) to head (4b1a563).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #332      +/-   ##
==========================================
+ Coverage   67.42%   67.50%   +0.08%     
==========================================
  Files          48       48              
  Lines        7024     7036      +12     
  Branches     1596     1595       -1     
==========================================
+ Hits         4736     4750      +14     
+ Misses       1700     1699       -1     
+ Partials      588      587       -1     
Flag Coverage Δ
3.10 67.66% <100.00%> (+0.08%) ⬆️
3.11 67.66% <100.00%> (+0.08%) ⬆️
3.12 67.66% <100.00%> (+0.08%) ⬆️
3.9 67.48% <100.00%> (+0.08%) ⬆️
ubuntu-latest 67.45% <100.00%> (+0.08%) ⬆️
windows-latest 67.45% <100.00%> (+0.08%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

kwave/options/simulation_options.py Outdated Show resolved Hide resolved
Tests the compression option `lzf`, which is not an option for the matlab h5create function, by
comparing written data to a reference matrix
"""
compression_option = 'lzf'
Copy link
Owner

Choose a reason for hiding this comment

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

I would like to see the test to test all possible compression options.

kwave/utils/io.py Outdated Show resolved Hide resolved
@waltsims
Copy link
Owner

I think this is a great start, but still needs some work before being merged in.

@djps
Copy link
Collaborator Author

djps commented Mar 6, 2024

May investigate additional options such as

  • shuffle details for speed-up in available compression options.
  • checksums details
  • scaleoffset for saving large datasets of indices such as p_source_index: details

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.

Extend HDF5 compression options
3 participants