ROOT 6.22 compatibility (CMSSW 11_2) about combineharvester HOT 13 CLOSED

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?

from combineharvester.

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.

from combineharvester.

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.

from combineharvester.

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

from combineharvester.

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 :)

from combineharvester.

@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

from combineharvester.

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

from combineharvester.

@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!

from combineharvester.

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

from combineharvester.

@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.

from combineharvester.

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

from combineharvester.

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?

from combineharvester.

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.

from combineharvester.