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 weighting function for several scenarios #567

Merged

Conversation

veni-vidi-vici-dormivi
Copy link
Collaborator

Add a function go generate weights such that each scenario contributes equally to some fitting routine. This means that a member of a scenario with a lot of members weighs less than a member of a scenario with few members.

Copy link

codecov bot commented Nov 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.10%. Comparing base (456776d) to head (693562e).
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #567      +/-   ##
==========================================
+ Coverage   77.93%   78.10%   +0.17%     
==========================================
  Files          49       49              
  Lines        2986     3010      +24     
==========================================
+ Hits         2327     2351      +24     
  Misses        659      659              
Flag Coverage Δ
unittests 78.10% <100.00%> (+0.17%) ⬆️

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.

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.

Oh - I just realized that the text and code of Lea's weight generation does not align:

derive scenario weights such that each has equal weight, i.e., 1 / number of samples
(= nr_runs * nr_ts)

weights.append(np.full(nr_samples_scen, 1 / nr_runs))

Let me check if I introduced this error or if this was always the case. -> See #569


Given your tests: should exclude be all dims that are not in ens_dim (i.e. remove it from the input params)? I am a bit afraid that the user will forget to add the correct exclude. Does it actually work if you don't exclude all the remaining dims?

mesmer/core/weighted.py Outdated Show resolved Hide resolved
mesmer/core/weighted.py Outdated Show resolved Hide resolved
mesmer/core/weighted.py Outdated Show resolved Hide resolved
mesmer/core/weighted.py Outdated Show resolved Hide resolved
mesmer/core/weighted.py Outdated Show resolved Hide resolved
tests/unit/test_weighted.py Outdated Show resolved Hide resolved
@veni-vidi-vici-dormivi
Copy link
Collaborator Author

Hm given #569 we could also extend the weighting function to have several options, something like "by_ens", "by_ens_ts" or even pass a function to do it... I have to think about it some more.

Co-authored-by: Mathias Hauser <[email protected]>
Co-authored-by: Mathias Hauser <[email protected]>
@mathause
Copy link
Member

Hm given #569 we could also extend the weighting function to have several options, something like "by_ens", "by_ens_ts" or even pass a function to do it... I have to think about it some more.

I would probably go via ens_dim or maybe dims by allowing to pass more than one dimension. I suggest to make our lives easier and keep the simple case for now.

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

should exclude be all dims that are not in ens_dim

No, in practice all dims except for ens_dim and time should be excluded. The test does not reflect the use case in practice... I agree that if the outcome should always be the same we might as well do it internally. I wrote it because I was not sure if there might be another use case for the function but actually in find_localized_empirical_covariance takes the same weights and I am not sure yet how we will handle weights in the autoregression. I guess we can cross that bridge when we come to it.

dependabot bot and others added 7 commits November 27, 2024 16:23
Bumps [codecov/codecov-action](https://github.com/codecov/codecov-action) from 4 to 5.
- [Release notes](https://github.com/codecov/codecov-action/releases)
- [Changelog](https://github.com/codecov/codecov-action/blob/main/CHANGELOG.md)
- [Commits](codecov/codecov-action@v4...v5)

---
updated-dependencies:
- dependency-name: codecov/codecov-action
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
)

* implement datatree and dataset in linear regression

* tests

* extend tests to root dt

---------

Co-authored-by: Mathias Hauser <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
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.

Looks good, thanks. map_over_subtree skips empty nodes, right?

mesmer/core/weighted.py Outdated Show resolved Hide resolved
mesmer/core/weighted.py Outdated Show resolved Hide resolved
mesmer/core/weighted.py Outdated Show resolved Hide resolved
mesmer/core/weighted.py Outdated Show resolved Hide resolved
@veni-vidi-vici-dormivi
Copy link
Collaborator Author

skips empty nodes, right?

Yes.

@veni-vidi-vici-dormivi veni-vidi-vici-dormivi merged commit 6552d66 into MESMER-group:main Dec 3, 2024
11 checks passed
@veni-vidi-vici-dormivi veni-vidi-vici-dormivi deleted the scenweights branch December 3, 2024 15:16
veni-vidi-vici-dormivi added a commit to veni-vidi-vici-dormivi/mesmer that referenced this pull request Dec 12, 2024
* implement weighting for several scnearios and members

* implement tests

* work around datatree.testing

* extend tests to root dt

* docs

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Mathias Hauser <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@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.

get_scenario_weights for new data structures
2 participants