Re: Discrepancy in RooFit when used in python and C++

Hello Wim,

On Thu, Apr 12, 2012 at 01:04, <wlavrijsen_at_lbl.gov> wrote:
> Hi,
>
>
>> The "Model" in my code is RooDecay with a RooGaussian resolution model.
>
>
> okay, so nothing strange there at the level of plotOn, as that Model does
> not
> use a 'using' in its inheritance tree.
>
> So on two case number 2: temporaries abuse.
>
> ProjWData does (see: roofitcore/src/RooGlobalFunc.cxx):
>
>  return RooCmdArg("ProjData",binData,0,0,0,0,0,&projSet,&projData) ;
>
> with the RooCmdArg doing this with arg o1 which is &projSet above (see
> roofitcore/src/RooCmdArg.cxx):
>
>  _o[0] = (TObject*) o1 ;
>
> so now a pointer to the temporary RooArgSet is stored and carried by return
> value RooCmdArg from ProjWData. Sweet. :P
>
> To proof, compile and run this simple C++ program to see that the address of
> the temporary and the address carried by the RooCmdArg return value match:
>
> //-----
> #include "RooRealVar.h"
> #include "RooDataHist.h"
> #include "RooArgSet.h"
> #include "RooCmdArg.h"
> #include "RooGlobalFunc.h"
>
> #include <iostream>
>
> RooCmdArg GimeArg(RooRealVar& x, RooAbsData& d) {
>   RooArgSet a(x);
>   std::cout << &a << std::endl;
>   return RooFit::ProjWData(a, d, true);
> }
>
> RooCmdArg GimeArg(RooCmdArg& arg) {
>   return arg;
> }
>
> int main() {
>   RooRealVar x("X", "myX", 3.14);
>   RooDataHist data;
>   RooCmdArg arg = GimeArg(x, data);
>   std::cout << arg.getObject(0) << std::endl;
>   return 0;
> }
> //----
>
> The above qualifies as a bug in my book, although the argument "you should
> not do that" can probably be made. :}
>
> Now, in C++, the scope of the temporary RooArgSet(dt) is equal to the scope
> of the statement (i.e. up to the ';'), and hence the full plotOn function
> call.
> You'd have to do something like this:
>
>  RooCmdArg arg = RooFit.ProjWData(RooArgSet(dt), dataset, true);
>  Model.plotOn(tframe1, arg);
>
> to be bitten by this in C++. But even then, b/c RooArgSet lives on the
> stack,
> there's a high likelihood that its contents haven't been overwritten yet.
> Also,
> the compiler need not destroy the temporary there and then, it can move to
> the
> end of the current scope (in the example C++ code above, the RooArgSet is
> put
> in its own scope, so a segfault on use of the RooCmdArg is very likely).
>
> Now, in python, the scoping rule is slightly different: the RooArgSet temp
> will see its reference count go to zero already at the end of the ProjWData
> call, not at the end of the plotOn as in C++. In addition, it lives on the
> heap. Hence, fun ensues much more easily.
>
> One way that typically works is to do:
>
>  argset = RooArgSet(dt)
>  Model.plotOn(tframe1, RooFit.ProjWData(argSet, dataset, True))
>
> to better control the lifetime. In the above, the argset is guaranteed to
> survive the plotOn call.
>
> Do note that there's an additional issue specific to python: since there are
> no implicit conversions, you'll end up much more often writing RooArgSet(dt)
> or similar to do some conversion, and hence introducing lifetime issues.
>

Thank you for this wonderfully clear analysis of the problem. I forwarded this to some of my colleagues which sparked quite a bit of a discussion. :) We will be mindful of these subtleties in the future.

>
>> Personally I prefer C++, but I see more and more people preferring Python
>> both within the collaboration (LHCb) and at my institute and then there
>> is the convenience of a powerful interactive environment, so I decided
>> to give it a try for my current project.
>
>
> Sure, but RooFit isn't a good place to start as most of the errors that will
> pop up are terribly, terribly subtle (as all invalid memory reads are).
>

Yes I see that now. The interface to our fitter is mostly written in Python, so I don't think I can abandon it completely in the context of RooFit, but I'll stick with C++ for my part. Thank you again for all the cautionary advice, I appreciate it a lot.

>
>> I'll give your recommendation some more thought and decide. Thanks a lot.
>
>
> Have a look around on the roottalk forum:
>
>   http://root.cern.ch/phpBB3/viewforum.php?f=14
>
> as these RooFit issues pop up many a time.
>

Thanks, I 'll have a look.

Cheers,

:)

-- 
Suvayu

Open source is the future. It sets us free.