SciML / Sundials.jl

Julia interface to Sundials, including a nonlinear solver (KINSOL), ODE's (CVODE and ARKODE), and DAE's (IDA) in a SciML scientific machine learning enabled manner
https://diffeq.sciml.ai
BSD 2-Clause "Simplified" License
208 stars 78 forks source link

Memory leak in cvode() #58

Closed lucasp0927 closed 8 years ago

lucasp0927 commented 8 years ago

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 commented 8 years ago

See #29?

tshort commented 8 years ago

And #49.

acroy commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

@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 commented 8 years ago

@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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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

ChrisRackauckas commented 8 years ago

Closing due to #67.