Memory leak in fits

Roofit and RooStats
1

Memory leaks when doing repeated fits, either with fitTo or by calling RooMinimizer::migrad interactively. This becomes a major problem for applications that must do many fits. Reproducer below.

Reproducer:

import ROOT as r

# create a variable
x = r.RooRealVar("x", "x", 0, 10)

# create a pdf
gm = r.RooRealVar("gm", "gm", 5, 0, 10)
pdf = r.RooGaussian("g", "g", x, gm, r.RooFit.RooConst(1))

# generate data
data = pdf.generate({x})

# fit it many times
print("Starting loop")
r.RooMsgService.instance().setGlobalKillBelow(r.RooFit.FATAL) 
while True:
    pdf.fitTo(data, PrintLevel=-1)

Here is the resulting memory leak (made with psrecord):
test.pdf (13.6 KB)

This is also the case using Minuit interactively:

import ROOT as r

# create a variable
x = r.RooRealVar("x", "x", 0, 10)

# create a pdf
gm = r.RooRealVar("gm", "gm", 5, 0, 10)
pdf = r.RooGaussian("g", "g", x, gm, r.RooFit.RooConst(1))

# generate data
data = pdf.generate({x})

# create minimizer
nll = pdf.createNLL(data)
m = r.RooMinimizer(nll)
m.setPrintLevel(-1)

# fit it many times
print("Starting loop")
r.RooMsgService.instance().setGlobalKillBelow(r.RooFit.FATAL)
while True:
    m.migrad()

Resulting in test_with_nll.pdf (13.3 KB).

The memory leak disappears if the minimizer is recreated:

import ROOT as r

# create a variable
x = r.RooRealVar("x", "x", 0, 10)

# create a pdf
gm = r.RooRealVar("gm", "gm", 5, 0, 10)
pdf = r.RooGaussian("g", "g", x, gm, r.RooFit.RooConst(1))

# generate data
data = pdf.generate({x})

# create nll
nll = pdf.createNLL(data)

# in a loop, create minimizer and fit it
print("Starting loop")
r.RooMsgService.instance().setGlobalKillBelow(r.RooFit.FATAL)
while True:
    m = r.RooMinimizer(nll)
    m.setPrintLevel(-1)
    m.migrad()

test_with_nll_recreate.pdf (14.6 KB)

1 Like
2

Dear @mwilkins ,

Thank you for the report! I will ask @jonas or @moneta to take a look at the issue and try to address the leaks.

Cheers,
Vincenzo

3

Thank you very much for sharing this interesting observation!

In the second case where you call RooMinimizer::migrad() in a loop, the “leak” is because the RooMinimizer stores the history of status flags from all the Minuit2 algorithms that are executed, for storing in the RooFitResult. So we can’t do something about this without breaking features. Is that acceptable to you?

In the case of fitTo(), this is a PyROOT problem that I attempt to fix with the following PR:

The problem is that PyROOT assumes that all member functions that take a non-const pointer transfer ownership to the instance that you call the function on. This has many false negatives, resulting in memory leaks all over the place until this policy is changed.

As a temporary workaround until this change is released, the best you can do is to manually create the list of command arguments and reset the ownership to the PyROOT side after the memory policy heuristic mistakenly dropped it:

cmdList = ROOT.RooLinkedList()
pl = ROOT.RooFit.PrintLevel(-1)
cmdList.Add(pl)
# This Add() call will make current PyROOT wrongly think
# that the ownership is transferred, but actually the RooLinkedList
# is non-owning resulting in leak.

ROOT.SetOwnership(pl, True) # Tell PyROOT that it owns the RooCmdArg
...
pdf.fitTo(data, cmdList)
4

For my application, I do not need the full history, just the latest status flags. Would it be possible to add an option to store only the latest flags or to manually clear the history?

This leads to seg faults when applied to some arguments:

import ROOT as r

# create a variable
x = r.RooRealVar("x", "x", 0, 10)

# create a pdf
gm = r.RooRealVar("gm", "gm", 5, 0, 10)
pdf = r.RooGaussian("g", "g", x, gm, r.RooFit.RooConst(1))

# generate data
data = pdf.generate({x})

# fit it many times
print("Starting loop")
r.RooMsgService.instance().setGlobalKillBelow(r.RooFit.FATAL)
commands = [
    r.RooFit.PrintLevel(-1),
    r.RooFit.Save(),
]
cmdList = r.RooLinkedList()
for x in commands:
    cmdList.Add(x)
    # This Add() call will make current PyROOT wrongly think
    # that the ownership is transferred, but actually the RooLinkedList
    # is non-owning resulting in leak.
    r.SetOwnership(x, True)  # Tell PyROOT that it owns the RooCmdArg

while True:
    pdf.fitTo(data, cmdList)

results in

Traceback (most recent call last):
  File "/home/mwilkins/aslsRun2/analysis/KKAsymmetry/test_fits_suggested.py", line 29, in <module>
    pdf.fitTo(data, cmdList)
  File "/home/mwilkins/miniconda/envs/aslsRun2_ana/lib/python3.11/site-packages/ROOT/_pythonization/_roofit/_rooabspdf.py", line 62, in fitTo
    return self._fitTo(args[0], _pack_cmd_args(*args[1:], **kwargs))
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: Template method resolution failed:
  RooFitResult* RooAbsPdf::fitTo(RooAbsData& data, const RooLinkedList& cmdList = {}) =>
    length_error: vector::reserve
  Failed to instantiate "fitTo(RooDataSet&,RooLinkedList&)"
  Failed to instantiate "fitTo(RooDataSet*,RooLinkedList*)"
  Failed to instantiate "fitTo(RooDataSet,RooLinkedList)"
5

This leads to seg faults when applied to some arguments:

I have identified this to be yet another PyROOT bug that I opened an issue for:

I hope this can be fixed by the PyROOT experts.

I have no clue what’s going on, but I figured out that the bug is connected to the iteration via for x in commands. If you iterate over the commands via index and item-getting, this work for me:

for i in range(len(commands)):
    cmdList.Add(commands[i])
    # This Add() call will make current PyROOT wrongly think
    # that the ownership is transferred, but actually the RooLinkedList
    # is non-owning resulting in leak.
    r.SetOwnership(commands[i], True)  # Tell PyROOT that it owns the RooCmdArg

For my application, I do not need the full history, just the latest status flags. Would it be possible to add an option to store only the latest flags or to manually clear the history?

That’s a good idea, I have opened a PR for that. So you will be able to do that as of ROOT 6.30.00, like m.clearStatusHistory().

1 Like
6

Oh, in fact the issue is fixed by renaming the iteration variable to something unique :slight_smile:

See:

2 Likes
7

This topic was automatically closed 14 days after the last reply. New replies are no longer allowed.