ROOT 6.22 compatibility (CMSSW 11_2)

[maxgalli]

After seeing that this PR in Combine was merged in this branch, I started doing the same kind of work for CombineHarvester (here). Adapting to ROOT 6.22 seems to be limited to just a couple of TPython methods that have been moved in CPyCppyy and changed names (TPython::ObjectProxy_FromVoidPtr became CPyCppyy::Instance_FromVoidPtr and TPython::ObjectProxy_AsVoidPtr became CPyCppyy::Instance_AsVoidPtr) in the transition from "old" PyROOT to new PyROOT.

However, probably due to my lack of experience with scram, I haven't been able to succeed in correctly linking against libcppyy2_7.so in CMSSW_11_2_0_pre10.

To reproduce:

export SCRAM_ARCH=slc7_amd64_gcc900
cmsrel CMSSW_11_2_0_pre10
cd CMSSW_11_2_0_pre10/src
cmsenv
git clone -b 112x https://github.com/cms-analysis/HiggsAnalysis-CombinedLimit.git HiggsAnalysis/CombinedLimit
git clone -b root622_comp https://github.com/maxgalli/CombineHarvester.git
scramv1 b

Running this returns a bunch of undefined references to the changed methods, like e.g.:

/cvmfs/cms.cern.ch/slc7_amd64_gcc900/external/gcc/9.3.0/bin/../lib/gcc/x86_64-unknown-linux-gnu/9.3.0/../../../../x86_64-unknown-linux-gnu/bin/ld: tmp/slc7_amd64_gcc900/src/CombineHarvester/CombineTools/src/Combin
eHarvesterCombineTools/CombineHarvester_Python.cc.o: in function `convert_py_root_to_cpp_root<TFile>::construct(_object*, boost::python::converter::rvalue_from_python_stage1_data*)':
CombineHarvester_Python.cc:(.text._ZN27convert_py_root_to_cpp_rootI5TFileE9constructEP7_objectPN5boost6python9converter30rvalue_from_python_stage1_dataE[_ZN27convert_py_root_to_cpp_rootI5TFileE9constructEP7_object
PN5boost6python9converter30rvalue_from_python_stage1_dataE]+0x5): undefined reference to `CPyCppyy::Instance_AsVoidPtr(_object*)'

What I tried: root-config --libs returns

-L/cvmfs/cms.cern.ch/slc7_amd64_gcc900/cms/cmssw/CMSSW_11_2_0_pre10/external/slc7_amd64_gcc900/bin/../../../../../../../slc7_amd64_gcc900/lcg/root/6.22.03-ghbfee2/lib -lCore -lImt -lRIO -lNet -lHist -lGraf -lGraf3d -lGpad -lROOTVecOps -lTree -lTreePlayer -lRint -lPostscript -lMatrix -lPhysics -lMathCore -lThread -lMultiProc -lROOTDataFrame -pthread -lm -ldl -rdynamic

The lib directory printed, correctly contains libcppyy2_7.so, but ld does not seem to look there:

$ /cvmfs/cms.cern.ch/slc7_amd64_gcc900/external/gcc/9.3.0/bin/../lib/gcc/x86_64-unknown-linux-gnu/9.3.0/../../../../x86_64-unknown-linux-gnu/bin/ld --verbose | grep SEARCH_DIR | tr -s ' ;' \\012
SEARCH_DIR("/data/cmsbld/jenkins/workspace/auto-builds/CMSSW_11_1_0_pre6-slc7_amd64_gcc900/build/CMSSW_11_1_0_pre6-build/tmp/BUILDROOT/610f04827ed0f827de9b8700ad3d9670/opt/cmssw/slc7_amd64_gcc900/external/gcc/9.3.0/x86_64-unknown-linux-gnu/lib64")
SEARCH_DIR("/data/cmsbld/jenkins/workspace/auto-builds/CMSSW_11_1_0_pre6-slc7_amd64_gcc900/build/CMSSW_11_1_0_pre6-build/tmp/BUILDROOT/610f04827ed0f827de9b8700ad3d9670/opt/cmssw/slc7_amd64_gcc900/external/gcc/9.3.0/lib64")
SEARCH_DIR("/usr/local/lib64")
SEARCH_DIR("/lib64")
SEARCH_DIR("/usr/lib64")
SEARCH_DIR("/data/cmsbld/jenkins/workspace/auto-builds/CMSSW_11_1_0_pre6-slc7_amd64_gcc900/build/CMSSW_11_1_0_pre6-build/tmp/BUILDROOT/610f04827ed0f827de9b8700ad3d9670/opt/cmssw/slc7_amd64_gcc900/external/gcc/9.3.0/x86_64-unknown-linux-gnu/lib")
SEARCH_DIR("/data/cmsbld/jenkins/workspace/auto-builds/CMSSW_11_1_0_pre6-slc7_amd64_gcc900/build/CMSSW_11_1_0_pre6-build/tmp/BUILDROOT/610f04827ed0f827de9b8700ad3d9670/opt/cmssw/slc7_amd64_gcc900/external/gcc/9.3.0/lib")
SEARCH_DIR("/usr/local/lib")
SEARCH_DIR("/lib")
SEARCH_DIR("/usr/lib")

Also appending the path to LD_LIBRARY_PATH does not seem to work.

Do anyone have any suggestion concerning what to try and how to correctly implement it in a build process?

(ping also for @nsmith- and @andrzejnovak who worked on adapting Combine to ROOT 6.22).

[ajgilbert]

By chance I was also looking at this today, you can try the just-pushed branch 112x (#256). It compiles ok for me in CMSSW_11_2_4, but I didn't validate if everything is working ok yet.

[maxgalli]

Perfect, thanks a lot! I will try it out later today.

[maxgalli]

hi @ajgilbert, I saw the commit in the branch and I have an important heads up.

In ROOT 6.22, which for the first time introduces the possibility to build with multiple Python versions at the same time, we had to separate TPython from the bindings. Now, the bindings (i.e., PyROOT) are built for the highest Python 3 and 2 versions that CMake finds, while TPython is (unfortunately) built for the highest version between the two of them (meaning that if we build for e.g. Python 2.7 and Python 3.7, the TPython library will be linked against the Python 3.7 libraries). You can see it for instance from here, where the variables like e.g. ${PYTHON_LIBRARIES_Development_Main} refer to the highest beetween Python 2 and 3. This means that, if my interpretation of how CVMFS 11_2 is built is correct, when building code that uses TPython methods both compilation and linking work as expected (i.e., no complains) but the libraries are linking against a different version of Python (in our case we're using Python2 but TPython links against Python3 libraries). This will most likely cause crashes when executing the code.

For more considerations, I'm pinging @smuzaffar (since I honestly don't remember right now how it was decided to deal with this TPython situation in CMSSW) and @etejedor.

Anyways, I updated the commit in my branch adding explicitly libcppyy2_7 to the LDFLAGS (like you were doing for TPython) in the two BuildFiles, and now it compiles and links correctly. Thanks :)

[smuzaffar]

@maxgalli , Adding LDFLAGS works too but we prefer to have toolfile which instruct scram what to do when someone requires it. I can add a root-cppyy2.xml and root-cppyy3.xml and then you can add

  <use name="root-cppyy2"/>

in your BuildFile to avoid explicitly setting LDFLAGS

[ajgilbert]

Hi @maxgalli thanks for the heads up, so if I understand correctly, as per your branch we don't actually need to link against TPython here, but can rely on libcppyy2_7 directly, and everything should be consistent at execution time? If so, then yes @smuzaffar, I think it will be useful to have the <use name="root-cppyy2"/> available for the BuildFile here, thanks. And perhaps in any case we should think about migrating this repo to use python3 anyway

[maxgalli]

@ajgilbert yes exactly, relying on libcppyy2_7 should make everything consistent at execution time when we use Python 2. For migrating to Python3 I couldn't agree more: ROOT itself is not planning to stop providing support for Python2 in the foreseeable future (the whole MultiPython thing was done exactly with this idea in mind), but considering that Python2 reached EOL in January 2020 I think it's a good idea to invest time in migrating all the projects that are meant to stay around for many years.

@smuzaffar great, thanks!

[andrzejnovak]

@maxgalli do you now have a working 112x/combine compatible branch?

[maxgalli]

@andrzejnovak yes, this branch compiles correctly and I didn't notice anything suspicious (so far) in the few commands that I tried. It still sets explicitly LDFLAGS though.

[ajgilbert]

Added a note to the README.md that 11_2_X can be used, but not on the master branch.

[andrzejnovak]

Sorry it took me a bit of time to get back to this. Are there actual dependencies for this from CMSSW? Could it be built just with make instead like combine?

[ajgilbert]

In theory yes, but we do depend on parts of combine (headers & linking), so any makefile would need to handle that. To be honest I'm not that interested in supporting standalone compilation with this - it just makes more work out of providing user support.

[andrzejnovak]

Sure makes sense. I thought the motivation would be the same as for standalone combine. In principle wouldn't being only dependent on combine (with a specific root version) rather than a specific cmssw stack be less maintenance or at least less complexity?

[ajgilbert]

Right, but with CMSSW I know the exact environment users have - so when I have to debug a problem for someone I need only confirm they are using the recommended CMSSW release. I also don't really want to implement (and maintain) a new build system that tracks the dependency between this repo and combine, when scram handles this just fine. If it becomes clear there is heavy demand for this in CMS then we can reconsider. Until then, of course anyone is free to fork this package, implement a standalone version, and even advertise to the collaboration - as long as they are happy to provide the long-term user support.