RooGenericPdf fails after variable rename and deserialization

[elusian]

Good morning

I’m modeling a simple pdf with a RooGenericPdf.
In my code I need to import the pdf into a workspace multiple times, so I use RenameAllVariablesExcept to rename the parameters (and not the observables).
This code works on the server I usually run it on (CMSSW 10.3.0, slc6, ROOT 6.12/7), but fails on my laptop (recent ROOT master).

From a few tests, RooGenericPdf manages to resist a variable renaming, but fails after being deserialized (like with RooWorkspace:import or RooAbsPdf::clone).

The code to reproduce is

void testGenericRename() {
    RooRealVar var("var", "var", 0, 1);
    RooRealVar par("par", "par", 0.5, 0, 1);
    RooGenericPdf genPdf("genPdf", "var*(par + 1)", RooArgSet(var, par));
    
    // works
    clog << genPdf.getVal() << endl;
    
    par.SetName("par_test");
    // also works
    clog << genPdf.getVal() << endl;
    
    auto pdfClone = (RooAbsPdf*)genPdf.clone("clone");
    // crash on master, returns 0 on 6.12
    clog << pdfClone->getVal() << endl;
    
    par.SetName("par");
    
    RooWorkspace ws1("ws1");
    ws1.import(genPdf);
    auto impPdf1 = ws1.pdf("genPdf");
    
    // works again
    clog << impPdf1->getVal() << endl;
    
    RooWorkspace ws2("ws2");
    ws2.import(genPdf, RooFit::RenameAllVariablesExcept("new", "var"));
    auto impPdf2 = ws2.pdf("genPdf");
    
    // works on 6.12, crash on master
    clog << impPdf2->getVal() << endl;
}

Is this a known bug?

Thanks in advance
Enrico

[StephanH]

Hi Enrico,

nice observation. It happens because the interpretation of formulae has been overhauled completely. It’s now saving the actual string of the formula you used to define it instead of a condensed version.
I’ll have a look into the renaming to make sure that the elements of the formula are also renamed so that the formula always looks consistent.

Edit:
A fix is on the way:
https://sft.its.cern.ch/jira/browse/ROOT-10411

[elusian]

Thank you for looking into this, I will update my ROOT version when the fix is merged.

[elusian]

@StephanH
It appears I was too hasty with marking the solution: my actual use case is still failing. The difference is that in my code I never compile the formula before serialization and that seems to break the fix.

New macro to reproduce is

void testGenericRenameV2() {
    RooRealVar var("var", "var", 0, 1);
    RooRealVar par("par", "par", 0.5, 0, 1);
    RooGenericPdf genPdf("genPdf", "var*(par + 1)", RooArgSet(var, par));
    
    // don't compile the formula before serializing
    //clog << genPdf.getVal() << endl;
    
    par.SetName("par_test");
    
    auto pdfClone = (RooAbsPdf*)genPdf.clone("clone");
    // crash
    clog << pdfClone->getVal() << endl;
    
    par.SetName("par");
    
    RooWorkspace ws1("ws1");
    ws1.import(genPdf);
    auto impPdf1 = ws1.pdf("genPdf");
    
    // works as before
    clog << impPdf1->getVal() << endl;
    
    RooWorkspace ws2("ws2");
    ws2.import(genPdf, RooFit::RenameAllVariablesExcept("new", "var"));
    auto impPdf2 = ws2.pdf("genPdf");
    
    // crash
    clog << impPdf2->getVal() << endl;
}

Edit:
Interestingly, this works if I replace RooGenericPdf with RooFormulaVar in the test, since RooFormulaVar forces the instantiation of the RooFormula and updates the expression inside redirectServersHook while RooGenericPdf doesn’t do anything if the formula has not yet been created.

I hope this helps
Enrico

[StephanH]

Ok, I guess that the same as for the formulaVar can be done for the genericPdf. This will have to wait until December, though, as I’m travelling.

[elusian]

Yes, not a problem. In the meantime I will work around the issue by using positional names inside the formula.

Thanks for the help