Memory leak in cvode()

[lucasp0927]

I'm using sundials to do some trajectory simulations. I noticed there's memory leak in both cvode() simple function and CVode(). The following code is able to reproduce the problem. Both mycvode() and Sundials.cvode() experience this issue.

using Sundials

function main()
    const t_span = collect(0:0.2:600)
    for i = 1:100000
        yout = Sundials.cvode(g,[1.0,0.0,0.0,0.0],t_span)
        #mycvode(f,[1.0,0.0,0.0,0.0],t_span)
    end
end

function mycvode(f::Function, y0::Vector{Float64}, t::Vector{Float64}; reltol::Float64=1e-8, abstol::Float64=1e-7)
    neq = length(y0)
    mem = Sundials.CVodeCreate(Sundials.CV_ADAMS, Sundials.CV_FUNCTIONAL)
    flag = Sundials.CVodeInit(mem, f, t[1], y0)
    flag = Sundials.CVodeSStolerances(mem, reltol, abstol)
    flag = Sundials.CVDense(mem, length(y0))
    y = copy(y0)
    tout = [t[1]]
    for k in 2:length(t)
        flag = Sundials.CVode(mem, t[k], y, tout, Sundials.CV_NORMAL)
    end
    Sundials.CVodeFree([mem])
end

function f(t, y, ydot, user_data)
    y = Sundials.asarray(y)
    ydot = Sundials.asarray(ydot)
    ydot[1] = 0.0
    ydot[2] = 0.0
    ydot[3] = 0.0
    ydot[4] = 0.0
    return Int32(0)
end


function g(t::Float64, x::Vector{Float64},arr::Vector{Float64})
    fill!(arr,0.0)
end

main()

[tshort]

See #29?

[tshort]

And #49.

[acroy]

The problem is that under the hood in both cases NVectors are created from Julia Arrays, but the memory is not released. This is addressed in #29, which is unfortunately a bit outdated.

[christianhaargaard]

If I were to update the changes from #29 to the current version what would I need to do? Would it be faster to attempt to merge (by hand) the code into a new fork, or to start over?
I think the complexity level is a bit higher than what I have looked at so far, but I I'm considering giving it a shot. Would really like if I could use the solver for some uncertainty quantification without it eating all of my memory.

[tshort]

That mainly depends on how well you know git. You can try using git to merge #29 into master to see how many conflicts there are. The code hasn't changed that much since then, so it may not be too bad.

[acroy]

@christianhaargaard : I think most of the changes in #29 shouldn't be affected by the recent changes in Sundials.jl - Merging shouldn't be a problem. You can also just copy the essentials from #29 and start a new PR. There are some comments from Steven that should be taken into account though.

[christianhaargaard]

@tshort and @acroy : I tried merging the entire acroy/types fork, but that doesn't seem viable. All of the bindings are using the @c macro, whereas the main repository has the ccall function written out completely. I will try and see if I have better luck merging only the corresponding commits, and then report back.

[christianhaargaard]

Putting this together is a mess when you have as little experience and overview as I do. However I did manage to get the simple cvode-interface to free up memory after each run. The code is so dirty however that I would never invite it into my house. I'm hoping I can get some time to look it through and better understand the structure, and possible clean it up.
If someone should be in dire need of running the solver without eating up all the memory, my code is available here (though I'm not proud to share it): https://github.com/christianhaargaard/Sundials.jl/
It's in the master branch.

[axsk]

Ah so I suppose thats why my program crashed with

terminate called after throwing an instance of 'std::bad_alloc'
  what():  std::bad_alloc

signal (6): Aborted
gsignal at /lib/x86_64-linux-gnu/libc.so.6 (unknown line)
abort at /lib/x86_64-linux-gnu/libc.so.6 (unknown line)
terminate called recursively

I will try ur fix @christianhaargaard :)

[axsk]

So the memory allocation is a big topic in Sundials :O.
The problem was already reported in #24, suggesting manual cleanup with CVodeFree (implemented by now) and N_VDestroy_Serial (not yet implemented).
#26 implemented the suggested measures, but was dropped in favour of #29, which supports the clearer approach of wrapping the construction and deconstruction into separate types.
Unfortunately #29 seems to have fallen asleep :(

Thus I basically reimplemented the changes from #26 for cvode (not the other solvers) based on the current master: https://github.com/axsk/Sundials.jl/tree/axsk/hotfixmemleak

Should I open a PR to it? I think a part-fix is still better then nothing (considering there seemingly is nothing else happening about this right now)

[acroy]

The non-progress on #29 is my fault :-(, but right now I don't have the time to finish it. As a first step one would need to rebase/port it against the current master, which shouldn't be hard. Then one could fix the loose ends ...

When I suggested #26, the others argued one should do it properly and I agree with that. If you happen to have some time, maybe you could try to port #29 to the current master and then we continue from there? However, if you want to revive #26 I would suggest to fix the other solvers too.

[alyst]

@acroy What are the loose ends of the #29 (besides the need to rebase)?

[ChrisRackauckas]

Closing due to #67.