root icon indicating copy to clipboard operation
root copied to clipboard

Published 20 hours ago • verified publisher icon root-project

[RF][HF] Clearly mark interpolation code 3 as unknown

Open guitargeek opened this issue 2 years ago • 5 comments

As noted in GitHub issue #7103, the interpolation code 3 is the same as code 2 in the FlexibleInterpVar and the PiecewiseInterpolation classes.

According to some comments in the source code, interpolation code 3 was meant to be "a parabolic version of log-normal".

There were two options to fix this:

  1. Actually implement this parabolic interpolation with linear extrapolation, analogous to code 2 but in log space.

  2. Clearly mark interpolation code 3 as non-existing.

This commit implements solution 2, because the interpolation code 3 is not mentioned anywhere outside the ROOT source code. Especially not is the HistFactory paper: https://cds.cern.ch/record/1456844/files/CERN-OPEN-2012-016.pdf

Implementing a new interpolation scheme that apparently was not needed in the last 10 years anyway would increase the burden on the user to understand all these different settings unnecessarily.

Closes #7103.

guitargeek avatar Oct 05 '23 12:10 guitargeek

Starting build on ROOT-performance-centos8-multicore/soversion, ROOT-ubuntu2204/nortcxxmod, ROOT-ubuntu2004/python3, mac11/noimt, mac12arm/cxx20, windows10/default How to customize builds

phsft-bot avatar Oct 05 '23 12:10 phsft-bot

Starting build on ROOT-performance-centos8-multicore/soversion, ROOT-ubuntu2204/nortcxxmod, ROOT-ubuntu2004/python3, mac11/noimt, mac12arm/cxx20, windows10/default How to customize builds

phsft-bot avatar Oct 05 '23 12:10 phsft-bot

Test Results

    18 files      18 suites   3d 17h 50m 25s ⏱️  2 678 tests  2 676 ✅ 0 💤 2 ❌ 46 342 runs  46 340 ✅ 0 💤 2 ❌

For more details on these failures, see this check.

Results for commit f53fa813.

:recycle: This comment has been updated with latest results.

github-actions[bot] avatar Oct 05 '23 15:10 github-actions[bot]

can this be maybe revamped? @hageboeck

dpiparo avatar Aug 22 '24 12:08 dpiparo

can this be maybe revamped? @hageboeck

I agree that if this was never implemented, it was probably never needed.

hageboeck avatar Aug 23 '24 08:08 hageboeck

If we agree that code 3 doesn't need to be implemented, can we merge this PR?

@will-cern also didn't see a problem with it not being implemented: https://github.com/root-project/root/pull/16858#issue-2641636630

guitargeek avatar Nov 15 '24 14:11 guitargeek