search

cms-analysis / HiggsAnalysis-CombinedLimit

CMS Higgs Combination toolkit.
https://cms-analysis.github.io/HiggsAnalysis-CombinedLimit/latest
Apache License 2.0
75 stars 378 forks source link

Update CMakeLists.txt - make path to header file match how it will be… #961

Closed will-cern closed 1 month ago

will-cern commented 2 months ago

… installed

I'm seeing this error that I'm trying to fix:

libCMSCombine dictionary payload:5:10: fatal error: 'src/classes.h' file not found
#include "src/classes.h"
         ^~~~~~~~~~~~~~~
anigamova commented 2 months ago

@will-cern with this PR https://github.com/root-project/root/pull/15299 these RooPower and RooExpPoly were removed from ROOT, so we can use the classes defined in Combine

will-cern commented 2 months ago

@will-cern with this PR root-project/root#15299 these RooPower and RooExpPoly were removed from ROOT, so we can use the classes defined in Combine

They were only removed in the 6.30 patches branch so far. They are still there in ROOT master (which will become 6.32), and of course they are still there in the 6.30.06 latest release

anigamova commented 2 months ago

as far as I understand they should be either removed or renamed in root 6.32, so these classes should stay in Combine I think, otherwise will be having a lot of compatibility issues

will-cern commented 2 months ago

as far as I understand they should be either removed or renamed in root 6.32, so these classes should stay in Combine I think, otherwise will be having a lot of compatibility issues

My preference would be for your implementations to be moved INTO ROOT and then they can be taken out of Combine for that version onwards.

The reason I am having to remove them now is because we don't have either a ROOT stable release that is compatible with Combine as it standard, nor is it compatible with ROOT master because the classes are still there. And I really want to have a version of CMSCombine we can put into the StatAnalysis releases that doesnt have clashes, even if it means that some of the CMSCombine models are incompatible (any model built with RooPower or RooExpPoly version from your sw).

anigamova commented 1 month ago

Hi @will-cern, you might be already aware that RooPower and RooExpPoly classes are renamed in ROOT 6.32 release to avoid the conflicts with Combine https://root.cern/doc/v632/release-notes.html#renaming-of-some-roofit-classes, would you consider switching to 6.32 in StatAnalysis and revert all of the commits hiding these classes from Combine here?

will-cern commented 1 month ago

Yes - if we are trying to target 6.32 now many of these changes are not needed. Instead we should just take the cmake file from the cmake3 branch

will-cern commented 1 month ago

https://github.com/cms-analysis/HiggsAnalysis-CombinedLimit/pull/967

anigamova commented 1 month ago

closing, merged #967 instead