Octree converter

[cphyc]

This adds the ability to convert a list of positions (x, y, z) with levels into an octree that can be loaded by yt.load_octree.

import numpy as np
import yt
from yt_experiments.octree.converter import OctTree
xyz = np.array([
    [0.375, 0.125, 0.125],
    [0.125, 0.125, 0.125],
    [0.375, 0.375, 0.375],
    [0.75, 0.75, 0.75],
])
levels = np.array([2, 2, 2, 1], dtype=np.int32)
data = {
    ("gas", "density"): np.random.rand(4)
}

oct = OctTree.from_list(xyz, levels, check=True)
ref_mask, leaf_order = oct.get_refmask()

for k, v in data.items():
    # Make it 2D so that yt doesn't think those are particles
    data[k] = np.where(leaf_order >= 0, v[leaf_order], np.nan)[:, None]

ds = yt.load_octree(ref_mask, data)

[chrishavlin]

Generally looks good to me, but I think there's a memory leak? I haven't been able to pin it down, but here's a little test script:

# mem_leak_test.py
import sys
import numpy as np
from yt_experiments.octree.converter import OctTree

if __name__ == '__main__':
    xyz = np.array(
        [
            [0.375, 0.125, 0.125],
            [0.125, 0.125, 0.125],
            [0.375, 0.375, 0.375],
            [0.75, 0.75, 0.75],
        ],
    )
    levels = np.array([2, 2, 2, 1], dtype=np.int32)

    for _ in range(int(sys.argv[1])):
        octree = OctTree.from_list(xyz, levels, check=True)        
        del octree

running with python mem_leak_test 1000000 (need a large number of iterations since the number of nodes is small), memory usage grows continuously. Ends up using close to 400 Mb before finishing the iterations.

[chrishavlin]

e.g., here's a flamegraph from memray for the above example -- the blue and orange lines are resident and heap size over time, y scale on the plot is from 0 to ~500 Mb

[chrishavlin]

oh! one more suggestion -- would be good to add *converter.cpp to .gitignore

[cphyc]

There was indeed a memory leak (I didn't implement a deallocate step). I've just checked with your code snippet, and the memory use is now constant. Thanks for the thorough review!

[chrishavlin]

Great! Ya, I thought you needed a __deallocate__ but i wasn't sure about whether it needed to recursively walk through the children (which based on your implementation now I see that it does!).