Fix the FormSum memory leak

[Ig-dolci]

Description

We are having memory leaks and more expensive computation of solvers involving FormSum (See more details on this discussion). That is caused by the sum operation involving dat. 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.

[github-actions]

	Tests	Passed ✅	Skipped ⏭️	Failed ❌
Firedrake complex	8125 ran	6540 passed	1585 skipped	0 failed

[github-actions]

	Tests	Passed ✅	Skipped ⏭️	Failed ❌
Firedrake real	8131 ran	7345 passed	786 skipped	0 failed

[JHopeCollins]

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 visited=None as the kwarg, and use a self._visited dict with visited = visited if visited else self._visited

[Ig-dolci]

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 visited=None as the kwarg, and use a self._visited dict with visited = visited if visited else self._visited

No. This does not fix it. I am still debugging.

[JHopeCollins]

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

[Ig-dolci]

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 visited=None as the kwarg, and use a self._visited dict with visited = visited if visited else self._visited

I see. I gonna try visited=None.

[JHopeCollins]

Looks good, just a couple of small suggestions on the code.

It would be good to have a test for the axpy methods too!

[connorjward]

Thanks! I would also like to see some tests. Including for the failure conditions like mismatching dats. These tests could go in https://github.com/firedrakeproject/firedrake/blob/master/tests/pyop2/test_dats.py (which contains some examples you could work off)

[connorjward]

Just docstring nitpicks from me. Looks fine otherwise. Thanks!

[JHopeCollins]

Looks good to me, thanks @Ig-dolci!

Connor reviewed.