-
Notifications
You must be signed in to change notification settings - Fork 18
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
Fix loading4mesmerx #502
Merged
veni-vidi-vici-dormivi
merged 11 commits into
MESMER-group:main
from
yquilcaille:fix_loading4mesmerx
Aug 22, 2024
Merged
Fix loading4mesmerx #502
veni-vidi-vici-dormivi
merged 11 commits into
MESMER-group:main
from
yquilcaille:fix_loading4mesmerx
Aug 22, 2024
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #502 +/- ##
==========================================
- Coverage 50.32% 49.78% -0.54%
==========================================
Files 50 50
Lines 3527 3565 +38
==========================================
Hits 1775 1775
- Misses 1752 1790 +38
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
veni-vidi-vici-dormivi
approved these changes
Aug 22, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Lorenzo warned me that the training with MESMER-X returns many coefficients with NaNs. I looked into that and found two types of issues.
Problem 1:
Loading data with MESMER-X requires to ensure that the same members are loaded. MESMER uses only temperature, thus it is ok. But with different variables, each variable has different ensemble members available. Thus it is important that the predictor and the target are loaded with the same set of members, in the same order. For some reason, it was not the case here.
Solution 1:
I implemented some code that I was using for the former versions of MESMER-X.
Results 1:
It works normally now,
Problem 2:
I validated the training of MESMER-X on different distributions and expressions, but I realized that I forgot to check for distributions with shape parameters. There were NaN because the coefficients on the shape parameter were not sufficiently well estimated during the first guess. Basically, the shape coefficients were not good enough, so points of the sample out of the support, identified during training as an error, thus no convergence.
Solution 2:
In MESMER-X, I was using the analytical expression of the support of a distribution, the estimates for the location and the shape, and the range of the sample to determine a range for the shape parameter. Here, I prefer not to use an analytical expression because it depends on each distribution, and i am trying to be as general as possible. The solution that I chose was to use the function
support
that comes with distributions ofscipy.stats
and thus the classExpression
, and tune the coefficients that are not on the location or the scale (not always a shape, sometimes more than 1) so that the whole sample is within the support.Results 2:
No NaN in coefficients anymore. Validated on GEV distribution, with linear evolution of location & scale, and linear evolution of location, scale & shape. I noticed a slight acceleration on the training, but the first guess is still too long.
CHANGELOG.rst