search

OrderN / CONQUEST-release

Full public release of large scale and linear scaling DFT code CONQUEST
http://www.order-n.org/
MIT License
96 stars 25 forks source link

Fix multiply kernel memory leak #293

Closed tkoskela closed 5 months ago

tkoskela commented 5 months ago

Closes #280

Uses assumed shape arrays for remote index arrays. These can vary in size and using a fixed size leads to accessing uninitialized memory.

Edit: I wanted to clarify that the inputs you gave in #292 still fail for me on all multiply kernels. So I'm not claiming to fix that issue. But I discovered a different issue and I'm fixing it :grinning:

Edit2: The reason the inputs from #292 fail for me is that my machine runs out of memory. Based on our last discussion that is to be expected with the size of the problem.

tkoskela commented 5 months ago

I tested this change on all multiply kernels in https://github.com/OrderN/CONQUEST-release/actions/runs/7045441423 and the tests pass so it's ready

tkoskela commented 5 months ago

Actually, with this change we can just pass slices of part_array into the multiply kernel, there is no need for pointers (other than making the subroutine calls look a bit more tidy).

See #294

davidbowler commented 5 months ago

I have added two small but important changes: we now zero part_array and ndimi and ndimj in the halo structure, all of which are passed to maxval. This should now fix the problem in #280