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

Convert AHI HSD dask chunking to be based on band resolution #2584

Merged
merged 5 commits into from
Oct 6, 2023

Conversation

djhoese
Copy link
Member

@djhoese djhoese commented Oct 2, 2023

As discussed in #2562, performance (memory usage for sure) is better when chunking is done based on resolution. Especially when native resampling is done. This means the 500m band's chunks are twice the size of the 1km band's chunks and 4 times the size of the 2km band's chunks. This should mean dask doesn't have to do any rechunking.

This PR is a continuation of #2052 and should be rebased/remerged after that is merged first. This PR also includes a fix for sunz correction where the dtype for 32-bit floats was being bumped up to 64-bit floats inside dask computations causing more memory usage than we thought.

  • Closes #xxxx
  • Tests added
  • Fully documented
  • Add your name to AUTHORS.md if not there already

@djhoese djhoese added enhancement code enhancements, features, improvements component:readers labels Oct 2, 2023
@djhoese djhoese self-assigned this Oct 2, 2023
@djhoese
Copy link
Member Author

djhoese commented Oct 2, 2023

Oh I still need to add tests for the reader chunking and dtype fixes there.

@djhoese djhoese requested a review from sfinkens as a code owner October 2, 2023 20:57
@codecov
Copy link

codecov bot commented Oct 3, 2023

Codecov Report

Merging #2584 (ec20ba9) into main (8d68425) will increase coverage by 0.00%.
Report is 23 commits behind head on main.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #2584   +/-   ##
=======================================
  Coverage   94.92%   94.92%           
=======================================
  Files         350      350           
  Lines       51085    51098   +13     
=======================================
+ Hits        48491    48504   +13     
  Misses       2594     2594           
Flag Coverage Δ
behaviourtests 4.28% <0.00%> (-0.01%) ⬇️
unittests 95.54% <100.00%> (+<0.01%) ⬆️

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

Files Coverage Δ
satpy/modifiers/angles.py 99.17% <100.00%> (ø)
satpy/readers/ahi_hsd.py 99.29% <100.00%> (ø)
satpy/tests/reader_tests/test_ahi_hsd.py 99.68% <100.00%> (+<0.01%) ⬆️
satpy/tests/test_modifiers.py 100.00% <100.00%> (ø)

@coveralls
Copy link

coveralls commented Oct 3, 2023

Pull Request Test Coverage Report for Build 6425912482

  • 31 of 31 (100.0%) changed or added relevant lines in 4 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.001%) to 95.495%

Totals Coverage Status
Change from base Build 6424552767: 0.001%
Covered Lines: 48627
Relevant Lines: 50921

💛 - Coveralls

Copy link
Member

@mraspaud mraspaud left a comment

Choose a reason for hiding this comment

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

Lgtm! Feel free to merge when the rebating is done and the tests pass.

@djhoese djhoese force-pushed the feature-ahi-abi-res-chunking branch from 595262d to 3350539 Compare October 5, 2023 22:24
@djhoese djhoese force-pushed the feature-ahi-abi-res-chunking branch from 3350539 to 9169abe Compare October 5, 2023 22:25
@djhoese
Copy link
Member Author

djhoese commented Oct 6, 2023

Reran my profiling and got the same results as before all this rebasing and refactoring so I think we're good here.

@djhoese djhoese merged commit f018f12 into pytroll:main Oct 6, 2023
19 of 20 checks passed
@djhoese djhoese deleted the feature-ahi-abi-res-chunking branch October 6, 2023 01:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:readers enhancement code enhancements, features, improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants