Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add tests for harmonic model #431

Merged
merged 28 commits into from May 16, 2024

Conversation

veni-vidi-vici-dormivi
Copy link
Collaborator

No description provided.

Copy link

codecov bot commented Apr 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.65%. Comparing base (9b0b76b) to head (c1feab9).
Report is 37 commits behind head on main.

❗ Current head c1feab9 differs from pull request most recent head 1387d5d. Consider uploading reports for the commit 1387d5d to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #431      +/-   ##
==========================================
- Coverage   87.90%   83.65%   -4.26%     
==========================================
  Files          40       44       +4     
  Lines        1745     1939     +194     
==========================================
+ Hits         1534     1622      +88     
- Misses        211      317     +106     
Flag Coverage Δ
unittests 83.65% <ø> (-4.26%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@veni-vidi-vici-dormivi
Copy link
Collaborator Author

veni-vidi-vici-dormivi commented May 2, 2024

Need to merge #433, #434 and #435 before this can work.

tests/unit/test_harmonic_model.py Show resolved Hide resolved
tests/unit/test_harmonic_model.py Outdated Show resolved Hide resolved
tests/unit/test_harmonic_model.py Outdated Show resolved Hide resolved
tests/unit/test_harmonic_model.py Outdated Show resolved Hide resolved
tests/unit/test_harmonic_model.py Outdated Show resolved Hide resolved
@veni-vidi-vici-dormivi
Copy link
Collaborator Author

Okay this looks good, now for the last hurdle: how do we want to test numerical stability? I can just hard code some expected output into it or we can save output data somewhere. The thing is that for hard coding output it is rather nasty because it is notoriously hard (impossible?) to get pretty numbers, but for saving it it is rather little data to check... To see what I mean I push an example hard coded test below.

Copy link
Member

@mathause mathause left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry some more small nits and a question:

  • Dosen't the $c = a + b \cdot T$ problem only exists for constant $T \neq 0$?
  • Yes writing numbers in test files is not nice, but if there is no other sensible option I prefer it over a data file.

tests/unit/test_harmonic_model.py Outdated Show resolved Hide resolved
@veni-vidi-vici-dormivi
Copy link
Collaborator Author

Dosen't the problem only exists for constant ?

Ah of course! We only need at least two different temperature values so that the system of equations is well defined. So I guess the easiest option would be to switch to that.

tests/unit/test_harmonic_model.py Show resolved Hide resolved
tests/unit/test_harmonic_model.py Show resolved Hide resolved
tests/unit/test_harmonic_model.py Show resolved Hide resolved
tests/unit/test_harmonic_model.py Outdated Show resolved Hide resolved
veni-vidi-vici-dormivi and others added 2 commits May 16, 2024 16:47
Co-authored-by: Mathias Hauser <mathause@users.noreply.github.com>
Co-authored-by: Mathias Hauser <mathause@users.noreply.github.com>
@mathause mathause merged commit eb1e20e into MESMER-group:main May 16, 2024
8 checks passed
@mathause
Copy link
Member

Thanks @veni-vidi-vici-dormivi!

@veni-vidi-vici-dormivi veni-vidi-vici-dormivi deleted the hm_testing branch May 16, 2024 15:56
veni-vidi-vici-dormivi added a commit to veni-vidi-vici-dormivi/mesmer that referenced this pull request May 23, 2024
* add tests for harmonic model

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* flaking

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* generalise tests

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* add test for fit to bic xr, rearrange and remove order in generate fs

* pandas datetime version compliance

* Revert "pandas datetime version compliance"

This reverts commit 40a1f45.

* actual pandas datetime compliance

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* make coefficients a numpy array

* extend tests on generate fourier series

* write out tolerance in test_fit_to_bic

* write out max_order

* use time.dt.month for months

* fix for the nan and zero wrangling

* verbose 0.1 in xr test

* add tests for dataarray in xr func

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* add test for numerical stability

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* test with non constant predictors

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* remove comment about close residuals

Co-authored-by: Mathias Hauser <mathause@users.noreply.github.com>

* add Note

Co-authored-by: Mathias Hauser <mathause@users.noreply.github.com>

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Mathias Hauser <mathause@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants