MESMER-group / mesmer

spatially-resolved ESM-specific multi-scenario initial-condition ensemble emulator
https://mesmer-emulator.readthedocs.io/en/latest/
GNU General Public License v3.0
23 stars 17 forks source link

Harmonic model bonus test #458

Closed veni-vidi-vici-dormivi closed 4 months ago

veni-vidi-vici-dormivi commented 4 months ago

In this PR I add a more sophisticated test for the xarray harmonic model. I also deleted Shrutis test file because I think I already implemented all the ideas that were in that file.

codecov[bot] commented 4 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 88.80%. Comparing base (9b0b76b) to head (671a46b). Report is 51 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #458 +/- ## ========================================== + Coverage 87.90% 88.80% +0.89% ========================================== Files 40 43 +3 Lines 1745 1884 +139 ========================================== + Hits 1534 1673 +139 Misses 211 211 ``` | [Flag](https://app.codecov.io/gh/MESMER-group/mesmer/pull/458/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MESMER-group) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/MESMER-group/mesmer/pull/458/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MESMER-group) | `88.80% <ø> (+0.89%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MESMER-group#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

veni-vidi-vici-dormivi commented 4 months ago

Doing this, I made two interesting observations:

  1. for the sixth harmonic, the coefficients of the sine term will always end up at the first guess: $sin( \frac{\pi \cdot 6 \cdot mon}{6}) = sin(\pi * mon)$ with $mon \in [1, 12]$ as integers, this will always give 0 so there is no fit for the first two coefficients of this order. I think it is still fine to include the sixth harmonic in the fit because the cosine term can to some extent make up for missing the sine term. But we cannot test if coefficients are close for this order (in a straight forward way at least - still thinking about that).

  2. the tests I do here work fine if we have n_ts = 10 but for more time steps, the fitted orders increase to an extent where at 100 time steps, all the orders come out to be six which makes the fitting take muuuch longer. I am not sure why this happens yet. When fitting real data, it's fine.

I'll work on this next week I guess.