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

Do not store grids, destroy them after usage #402

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

Conversation

aulemahal
Copy link
Collaborator

@aulemahal aulemahal commented Nov 20, 2024

Cleanest solution to #401 I could think of. An alternative less breaking solution would be to simply revert #387.

Once the weights are generated, we do not need the ESMpy grids (or locstreams) anymore. In fact, before #24, the grid were "destroyed" (they memory explicitly released by ESMpy) once the regrid was done. That meant that Regridder.grid_in was still there and with most properties accessible, but most methods would fail, with a segmentation fault. When implementing the Spatial Averager, I needed to reuse grid objects for multiple ESMpy regrid calls, so I commented out the explicit destroy.

As shown in JiaweiZhuang/xESMF#53, this creates a memory leak. #387 added these destroy back, as suggested by a comment on that thread. However, the moment they are executed seems to trigger #401.

Here, I make it clear that we do not need the grids : they are never set as attributes of the Regridder and are destroyed once the weights are generated. The destroy happens in the children (instead of a backend function) because that is were the objects were created. If users use the BaseRegridder with ESMpy objects directly, they should be unaffected.

But this is breaking because Regridder.grid_in and Regridder.grid_out do not exist anymore. Internally, we weren't really making use of them, but I don't know what are users are doing.

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.

1 participant