-
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
Add MESMER-M integration tests #501
base: main
Are you sure you want to change the base?
Add MESMER-M integration tests #501
Conversation
for more information, see https://pre-commit.ci
Thanks - can you save the coeffs as netCDF and not pickle (I know it's a bit more annoying but we want to move away from pickle) |
17afff8
to
f97ea6e
Compare
for more information, see https://pre-commit.ci
I haven't looked into it so not sure what is going on but a bit strange that none of the environments get it right. I thought this does not have to be high priority but then it would be good to stabilize the numerics for memser-m. What helped for the covariance thingy was testing the code on cfc vs exo, which have different cpus with different sets of SIMD instructions. But I am not really working today - if I find time I'll take a look tomorrow or else after our retreat... |
Okay I think (for the moment) this is good enough. I can open an issue on this after merging so we can come back to it maybe in the future. Also, should I separate the changes in the harmonic model and the power transformer from the integration tests into (3) separate PRs or just rename this PR to "MESMER-M integration test stabilizing"? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks - that's very good to have. I add some first suggestions - the biggest being to not merge the coeffs - let me know if you don't think that's a good idea.
AR1_fit.residuals, weights, phi_gc_localizer, "time", 30 | ||
) | ||
|
||
# merge into one dataset |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not a huge fan of this there and back renaming and merging. Would it be bad to save each Dataset as individual netCDF? Or merge into a datatree? (But maybe later).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea of having one dataset with all the calibrated parameters per ESM. I did some renaming in the modules such that everything can be merged without problems. What do you think about that?
Do you happen to know if the estimates are stable for each OS? Could we do os-dependent tolerances? (Not asking you to sink more time into this PR atm...) |
CHANGELOG.rst