Fix the FormSum memory leak
#3897
Conversation
|
|
|
Oh dear, if this fixes the issue then that interface could do with updating! I think that the traversal method is taking advantage of the behaviour described here: #3348 (comment) This behaviour is not widely known and is quite unintuitive - it's usually considered a bug. It might be best to change this method signature (and the preorder traversal method) to use |
No. This does not fix it. I am still debugging. |
Shame it wasn't so simple! It may still be good to change the signatures to avoid that behaviour though |
I see. I gonna try |
firedrake/assemble.py
Outdated
| @@ -584,7 +583,8 @@ def update_tensor(assembled_base_form, tensor): | |||
| raise NotImplementedError("Cannot update tensor of type %s" % type(tensor)) | |||
|
|
|||
| @staticmethod | |||
| def base_form_postorder_traversal(expr, visitor, visited={}): | |||
| def base_form_postorder_traversal(expr, visitor, visited=None): | |||
| visited = visited if visited is not None else {} | |||
There was a problem hiding this comment.
To have the same caching behaviour as before, without the {} default kwarg, this should stash visited as an attribute so it get's reused unless the user passes visited, e.g.
visited = visited if visited is not None else self._visitedThere was a problem hiding this comment.
I decided to keep the original code here.
| dat_result.data_ro_with_halos, | ||
| w * dat_op.data_ro_with_halos, | ||
| out=dat_result.data_wo_with_halos) |
There was a problem hiding this comment.
I am not sure if make this operation with_halos is the right think to do.
There was a problem hiding this comment.
Does result.assign(sum(w*arg for arg in args)) work? This code looks very very similar to what we do in assign.py.
There was a problem hiding this comment.
No. result is a Cofunction, and Cofunction assigning an exp that is isinstance(expr, BaseForm) will reach this code again, which leads to a maximum recursion. See this Cofunction assignment code.
Description
We are having memory leaks and more expensive computation of solvers involving
FormSum(See more details on this discussion). That is caused by this operation. My solution here is to make a sum operation through the numpy arrays. Below is a comparison (using the example added at the discussion) of time and memory computation differences between the master branch and the current PR.