Skip to content

Add complevel argument to netcdf exporter + tests#550

Open
ladc wants to merge 5 commits into
masterfrom
add_complevel
Open

Add complevel argument to netcdf exporter + tests#550
ladc wants to merge 5 commits into
masterfrom
add_complevel

Conversation

@ladc
Copy link
Copy Markdown
Contributor

@ladc ladc commented May 26, 2026

As pointed out by @larsbonnefoy, setting this to a lower value significantly speeds up netcdf writing.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 26, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.17%. Comparing base (a1dd4a7) to head (af0873d).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #550   +/-   ##
=======================================
  Coverage   84.17%   84.17%           
=======================================
  Files         170      170           
  Lines       14985    14985           
=======================================
  Hits        12614    12614           
  Misses       2371     2371           
Flag Coverage Δ
unit_tests 84.17% <ø> (ø)

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.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@larsbonnefoy
Copy link
Copy Markdown

larsbonnefoy commented May 27, 2026

To complement @ladc pr, when using compression Level 9 the bottleneck when exporting is the compression leading to slow netcdf writes. Lowering the compression level only slightly increases output size but improves overall execution speed. This would help overall speed up without having to rely on other parallelization methods.

Another change we could implement is to be able to rely on other compression libraries than zlib.

Test whole range of compression levels (0-9).
@larsbonnefoy larsbonnefoy self-requested a review May 27, 2026 11:44
Copy link
Copy Markdown

@larsbonnefoy larsbonnefoy left a comment

Choose a reason for hiding this comment

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

Adding simple new argument to exporter + test, looks got for me

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a complevel argument to initialize_forecast_exporter_netcdf so users can trade off netCDF compression ratio for write speed (the previous behavior hard-coded complevel=9). The default is preserved at 9 for backward compatibility, and the test matrix is extended with several compression-level values.

Changes:

  • Add complevel parameter (default 9) to initialize_forecast_exporter_netcdf and forward it to both createVariable calls.
  • Document the new parameter in the docstring.
  • Extend test_io_export_netcdf_one_member_one_time_step parametrization with a complevel dimension.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
pysteps/io/exporters.py Replaces hard-coded complevel=9 with a user-configurable argument.
pysteps/tests/test_exporters.py Adds complevel to the parametrized test cases (but does not pass it through to the exporter).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pysteps/io/exporters.py
Comment thread pysteps/tests/test_exporters.py
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comment thread pysteps/tests/test_exporters.py Outdated
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