cta-observatory / pyirf

Python IRF builder
https://pyirf.readthedocs.io/en/stable/
MIT License
15 stars 25 forks source link

Moment morph interpolator #221

Closed RuneDominik closed 1 year ago

RuneDominik commented 1 year ago

This PR introduces another interpolation method (Moment Morphing, see [1]). While performing comparably to the already existing Quantile Interpolation the main points in favor of this method is that it can be can be separated into two steps. First, one has to compute interpolation coefficients and then these are used to morph and interpolate the input pdfs. When changing the coefficients (e.g. changing interpolation to extrapolation or changing from interpolation in a triangle to interpolation in a rectangle) the second part remains unchanged. Consequently, using this for extrapolation is quite simple.

Some performance studies can be found in these notebooks: PerformanceStudy.zip

This PR is quite extensive in its additions. Most of these are actually tests.

[1] M. Baak, S. Gadatsch, R. Harrington and W. Verkerke (2015). Interpolation between multi-dimensional histograms using a new non-linear moment morphing method. Nucl. Instrum. Methods Phys. Res. A 771, 39-48. https://doi.org/10.1016/j.nima.2014.10.033

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.96 :tada:

Comparison is base (52e1297) 92.46% compared to head (52a4226) 93.42%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #221 +/- ## ========================================== + Coverage 92.46% 93.42% +0.96% ========================================== Files 49 51 +2 Lines 2005 2298 +293 ========================================== + Hits 1854 2147 +293 Misses 151 151 ``` | [Impacted Files](https://codecov.io/gh/cta-observatory/pyirf/pull/221?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cta-observatory) | Coverage Δ | | |---|---|---| | [pyirf/interpolation/\_\_init\_\_.py](https://codecov.io/gh/cta-observatory/pyirf/pull/221?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cta-observatory#diff-cHlpcmYvaW50ZXJwb2xhdGlvbi9fX2luaXRfXy5weQ==) | `100.00% <100.00%> (ø)` | | | [pyirf/interpolation/moment\_morph\_interpolator.py](https://codecov.io/gh/cta-observatory/pyirf/pull/221?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cta-observatory#diff-cHlpcmYvaW50ZXJwb2xhdGlvbi9tb21lbnRfbW9ycGhfaW50ZXJwb2xhdG9yLnB5) | `100.00% <100.00%> (ø)` | | | [...erpolation/tests/test\_moment\_morph\_interpolator.py](https://codecov.io/gh/cta-observatory/pyirf/pull/221?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cta-observatory#diff-cHlpcmYvaW50ZXJwb2xhdGlvbi90ZXN0cy90ZXN0X21vbWVudF9tb3JwaF9pbnRlcnBvbGF0b3IucHk=) | `100.00% <100.00%> (ø)` | | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cta-observatory). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cta-observatory)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

maxnoe commented 1 year ago

I think what is also needed, is a short, comparative discussion of the different interpolation methods in the docs

RuneDominik commented 1 year ago

I guess the main point of confusion here is, that there are two separate structures. There are

These three classes only operate on one simplex in the respective tessellation, so either a line-segment or a triangle. These can later-on easily reused to provide extrapolation, which is the primary reason why they are separated from the interpolator class that should be used by the user. It should, however, be possible to combine these three classes into one if wanted.

To make this easily usable with a grid of arbitrary size, there is the actual MomentMorphInterpolator, the second structure, which takes the full grid, checks its dimensionality, tessellates its, finds the correct simplex for a target-point and then uses the aforementioned classes to actually create a interpolation result.

I hope this cleared things up a bit. I'm open for suggestions, especially for less ambiguous naming schemes.

RuneDominik commented 1 year ago

Superseded by #229