kevinganster / eikonalfm

The Fast Marching method to solve the eikonal equation
MIT License
35 stars 9 forks source link

Memory leak #6

Closed laughingrice closed 1 year ago

laughingrice commented 1 year ago

eikonalfm.fast_marching looks to be leaking memory. It is not a lot per call, running on a 256x256 grid looks to be leaking around 200KB, but I'm trying to run it around 400,000 time for batch travel time computation results with enough of a leak to crash a machine with 192GB of memory.

kevinganster commented 1 year ago

I'll try to figure out where this could be coming from. Did you use the sensitivities, i.e. set output_sensitivities=True in the call to eikonalfm.fast_marching, or did you just use eikonalfm.fast_marching(c, x_s, dx, order) without outputting the sensitivities?

laughingrice commented 1 year ago

I'm using it without computing sensitivities.

I think I found the problem, it currently works, but I need to check all paths to verify I didn't miss anything. I'll make a pull request in a day or two when I have a bit more time to cleanup, in the mean time for the record

in eikonalfm/cfm.cpp:212 (line number might be slightly off, I changed a few other things trying to find the issue) the return value of fastmarching should be corrected to:

        }
-       else
-               return (PyObject *)tau;
+
+    Py_DECREF(c);
+    Py_DECREF(x_s_);
+    Py_DECREF(dx_);
+
+    return PyArray_Return(tau);
 }
kevinganster commented 1 year ago

The return value is not the problem, but i forgot to decrease some reference counters. I've now fixed those and a few other problems i found while doing that and uploaded a new version to pypi. Please check if it works now (it did in all my tests).