Skip to content

Conversation

davidhassell
Copy link
Collaborator

@davidhassell davidhassell commented Mar 23, 2023

Fixes #623

Notes

  • The previous approach loaded both the source and destination grid masks onto the weights, which had to be a dense numpy array since scipy sparse arrays do not support masks.
  • The new approach keeps the weights in a scipy sparse array, but creates a separate boolean mask to applied to the regridded data.
  • The change to cf/regrid/regrid.py pre-creates the sparse matrix representation in a returned regrid operator. This makes sense because the purpose of returning the operator is for efficiency in repeated future uses, so it makes sense not to have to do this every time.
  • The change from coo_array to csr_array is because the latter is much more efficient at row slicing.

@davidhassell davidhassell added performance Relating to speed and memory performance regridding Relating to regridding operations labels Mar 23, 2023
@davidhassell davidhassell added this to the 3.14.2 milestone Mar 23, 2023
@sadielbartholomew
Copy link
Member

FYI I just added one minimal commit to resolve the lone trivial merge conflict (new Changelog entries to combine) so I can review without it being in the way.

Copy link
Member

@sadielbartholomew sadielbartholomew left a comment

Choose a reason for hiding this comment

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

Clever approach which I agree with as covered in the notes of the opening comment. The implementation is sound and doesn't call for any new testing. The performance improves very significantly in test cases I played around with and did basic profiling on, so passes with flying colours in that respect.

Therefore overall this is all good, though I've raised some minor comments and a question. Please consider those and then merge when ready. Thanks.

davidhassell and others added 5 commits April 24, 2023 09:00
Co-authored-by: Sadie L. Bartholomew <[email protected]>
Co-authored-by: Sadie L. Bartholomew <[email protected]>
Co-authored-by: Sadie L. Bartholomew <[email protected]>
Co-authored-by: Sadie L. Bartholomew <[email protected]>
@davidhassell davidhassell merged commit c5e5201 into NCAS-CMS:main Apr 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Relating to speed and memory performance regridding Relating to regridding operations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Regridding sometimes fails due to running out of memory
2 participants