root icon indicating copy to clipboard operation
root copied to clipboard

[RF] RooFit workspace work differently with pyROOT in 6.32-rc1 version and 6.30/02

Open yuhao-wang-nju opened this issue 1 year ago • 2 comments

Check duplicate issues.

  • [ ] Checked for duplicates

Description

Hi root team,

We are the ATLAS VHbbcc analysis team and we are testing with the new root preview version v6.32-rc1 since it has the bug fix related to the issue: https://root-forum.cern.ch/t/cling-jit-session-error-cannot-allocate-memory/56744

However we met some different behavior in this version compared to the previous 6.32/02 that we use.

  1. The pyROOT, we got a different type when access the RooCategory allowedState name. In the new version, it returns cppyy.gbl.std.string object, previously it is a python str object. May I ask it is designed or a bug?
  2. We found this because it crushes when we tried to pass it to re.findall() function. But according to the cppyy document, the cppyy should convert cppyy.gbl.std.string to str automatically when the str is needed. So what is the reason for not being converted automatically?
  3. We also found that it takes much longer time to run the fit using the new root version, in our case, a 1-hour fit now takes 3-hours to finish. Is there any hint what may be the cause?

Thanks a lot for your time

Yuhao

Reproducer

import ROOT

g = ROOT.TFile.Open("/afs/cern.ch/work/y/yuhao/public/test.root")
ws=g.combined
simpdf=ws.pdf("simPdf")
channelCat = simpdf.indexCat()
print(type(channelCat.begin().first))

ROOT version

6.32-rc1

Installation method

build from source

Operating system

Linux el9

Additional context

No response

yuhao-wang-nju avatar May 10 '24 14:05 yuhao-wang-nju

Hi @yuhao-wang-nju, thanks for your post!

  1. This is by design (see #15153)
  2. Where does the doc say that? I think cppyy pythonizes the cppyy.gbl.std.string such that it behaves like a Python str, therefore it can be used in many contexts where one would use str. But this doesn't always help. The type is different after all, and there is no automatic conversion for the reasons outlined in #15153. You will have to do this yourself with str() around the std::string.
  3. The evaluation was completely replaced in 6.32. Probably you have one of the corner cases where the new backend is slower and I need to debug this. Is it also possible to reproduce this slow fit with the workspace you shared? Maybe you can share the full script?

Thanks a lot!

Related PR: #14237

guitargeek avatar May 10 '24 15:05 guitargeek

Hi @guitargeek , thanks a lot for the reply and sorry for the late message.

For the first and second point, thanks a lot for the explanation. I may misunderstood the way cppyy works previously, now it seems quite reasonable to me!

For the third one, I prepared a workspace and also attached the code that we use (The dataset named obsData contained in the file is an asimov one). We are using a framework named WSMaker and I just picked up the fitting code. It is a bit long and hope it won't bring too much trouble to you. Here is the command we use for fit: root -l -b -q '$WORKDIR/FitCrossCheckForLimits.C+(FitToAsimov,1.0,0.0,true,"ws.root","output/","combined","ModelConfig","obsData",false,false)'

The file is a bit large so here is the gdrive link. Thanks a lot in advance for your time!

yuhao-wang-nju avatar May 14 '24 12:05 yuhao-wang-nju

After the official release, we tested and found there is no slowdown of the fit. Thanks a lot for the updates and reply! I think we can close the issue now;)

yuhao-wang-nju avatar May 31 '24 14:05 yuhao-wang-nju

Thanks for reporting and retesting.

dpiparo avatar Jun 01 '24 05:06 dpiparo