search

cms-analysis / CombineHarvester

CMSSW package for the creation, editing and analysis of combine datacards and workspaces
cms-analysis.github.io/CombineHarvester/
15 stars 180 forks source link

Segfault in CombineHarvester::WriteDatacard(string, string) in Python #239

Closed andrey-popov closed 3 years ago

andrey-popov commented 4 years ago

I encountered a bug in Python bindings. I'm trying to create a datacard for a shape-based analysis using the Python interface. With the current master (6af279a33e9b88bfcca4803dfc06cbe05e07a5ae), running

harvester = ch.CombineHarvester()
# ...
harvester.WriteDatacard('datacard.txt', 'shapes.root')

triggers a segmentation violation. The code works fine if I create the ROOT file beforehand:

shapes_file = ROOT.TFile('shapes.root', 'recreate')
harvester.WriteDatacard('datacard.txt', shapes_file)

The backtrace to the main WriteDatacard method suggest that the problem is in the Python bindings:

#0  0x00007ffff675cb50 in ch::CombineHarvester::WriteDatacard(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, TFile&)@plt ()
   from /user/aapopov/CMSSW/CMSSW_10_2_13__combine/lib/slc7_amd64_gcc700/libCombineHarvesterCombineTools.so
#1  0x00007ffff696dc7a in Overload3_WriteDatacard (cb=..., name="datacard.txt", file=...)
    at /user/aapopov/CMSSW/CMSSW_10_2_13__combine/src/CombineHarvester/CombineTools/src/CombineHarvester_Python.cc:136
#2  0x00007ffff69cda79 in boost::python::detail::invoke<int, void (*)(ch::CombineHarvester&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, boost::python::api::object&), boost::python::arg_from_python<ch::CombineHarvester&>, boost::python::arg_from_python<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&>, boost::python::arg_from_python<boost::python::api::object&> > (
    f=@0x68de18: 0x7ffff696dc1c <Overload3_WriteDatacard(ch::CombineHarvester&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, boost::python::api::object&)>, ac0=..., ac1=..., ac2=...) at /cvmfs/cms.cern.ch/slc7_amd64_gcc700/external/boost/1.63.0-gnimlf/include/boost/python/detail/invoke.hpp:81
#3  0x00007ffff69bf6a5 in boost::python::detail::caller_arity<3u>::impl<void (*)(ch::CombineHarvester&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, boost::python::api::object&), boost::python::default_call_policies, boost::mpl::vector4<void, ch::CombineHarvester&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, boost::python::api::object&> >::operator() (this=0x68de18, args_=0x7ffff6af2aa0)
    at /cvmfs/cms.cern.ch/slc7_amd64_gcc700/external/boost/1.63.0-gnimlf/include/boost/python/detail/caller.hpp:218
#4  0x00007ffff69b0343 in boost::python::objects::caller_py_function_impl<boost::python::detail::caller<void (*)(ch::CombineHarvester&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, boost::python::api::object&), boost::python::default_call_policies, boost::mpl::vector4<void, ch::CombineHarvester&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, boost::python::api::object&> > >::operator() (this=0x68de10, args=0x7ffff6af2aa0, kw=0x0)
    at /cvmfs/cms.cern.ch/slc7_amd64_gcc700/external/boost/1.63.0-gnimlf/include/boost/python/object/py_function.hpp:38
...

Note how the call is passed through Overload3_WriteDatacard instead of Overload1_WriteDatacard. Apparently Overload3_WriteDatacard tries to convert a string into a TFile. I also see that file_ inside it is null. Unfortunately, I don't know enough about the bindings to propose a fix.

ajgilbert commented 4 years ago

It's possible changing the order of the defs here will fix the problem: https://github.com/cms-analysis/CombineHarvester/blob/6af279a33e9b88bfcca4803dfc06cbe05e07a5ae/CombineTools/src/CombineHarvester_Python.cc#L261-L263 But I didn't test it yet. A better solution is probably just to rename one of the functions in the python interface, which we'll address in a PR soon.