cms-analysis / HiggsAnalysis-CombinedLimit

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

Remove rVrFLikelihood class and related test script #785

Closed guitargeek closed 2 years ago

guitargeek commented 2 years ago

The rVrFLikelihood had an issue with creating invalid proxies that caused it to not compile anymore in ROOT 6.28. The provious commit suggested to fix the class, but according to Andrew Gilbert (@ajgilbert) it is safe to remove the class from combine.

The previous commit with fixing the class is kept in the commit history because the work was already done and who knows if it will be useful to look at that commit one day.

guitargeek commented 2 years ago

@amarini @nsmith- , can this be marked safe to test? Thanks!

hcombbot commented 2 years ago

Pull Request Test. Summary

Running options:

guitargeek commented 2 years ago

I'm curious, what is this rVrFLikelihood class used for, if at all?

For combine to compile with the upcoming ROOT 6.28, the invalid proxies either need to be fixed as suggested in this PR, of the class can be removed if it is unused I guess.

ajgilbert commented 2 years ago

Sorry just looking at this now - I'm 99.9% certain this is not used by anyone now and can safely be removed from combine. I think this would be the simplest way forward.

guitargeek commented 2 years ago

Thanks Andrew for your assessment! I have updated the PR with a new commit that completely removes the class and a related test script. Let me know if you think it's fine like this.

hcombbot commented 2 years ago

Pull Request Test. Summary

Running options:

hcombbot commented 2 years ago

Pull Request Test. Summary

Running options: