-
Notifications
You must be signed in to change notification settings - Fork 15
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 support for cryosphere campaign #144
Conversation
This requires #143 so I will rebase once that goes in. |
TestingDid a test run of
|
@forsyth2 and @tomvothecoder, I realize it's not really kosher to add a new feature after a release candidate has already been created. Would you be willing to make an exception for this feature? It's not critical. The more important thing is that |
Sounds good with me, since the intent is to align this |
Thanks @tomvothecoder! |
Yes, I would like this to be in the next release candidate. Besides, I would argue this is an important extension of #138 anyway. |
I just rebased. |
@wlin7, I'd appreciate your feedback on this before I ask anyone else to review. Please let me know what e3sm_diags analysis you would like to run by default and any other changes we need to make from |
@xylar Once this is ready to merge, we can add a commit I worked on for testing (forsyth2@d474073) |
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.
From the standpoint of integrating into zppy
, this looks good to me. (I can't say what is necessary for the cryosphere campaign in particular though).
This includes a new cryosphere config file in the templates and a special `--polar_regions` flag for MPAS-Analysis. The new flag will only be supported for MPAS-Analysis >= 1.5.0 (not yet released), so it will require an update of E3SM-Unified to work correctly.
27e2cef
to
b096b48
Compare
@forsyth2, my testing from above (which I'm rerunning right now) doesn't include any model vs. model testing for |
I'm taking @darincomeau and @stephenprice off as reviewers. I don't think we need them to approve officially because their existing feedback is all I needed. @forsyth2, I think this is ready to merge as long as we're okay with the possibility that there are some model vs. model kinks to iron out at a later date. |
@forsyth2, one caveat is that you might want to update documentation and/or tests. Do you have a good sense of what would be useful there? I can help but you're likely to be the better person to at least get the ball rolling. |
@xylar Thanks! Once Chrysalis is back online, I'll update the expected files for forsyth2@d474073, push the tests to this branch, and then merge. |
@forsyth2, sorry, I forgot you already said you had a commit to add. Sounds perfect! |
This includes a new cryosphere config file in the templates and a special
--polar_regions
flag for MPAS-Analysis. The new flag will only be supported for MPAS-Analysis >= 1.5.0 (not yet released), so it will require an update of E3SM-Unified to work correctly.