-
Notifications
You must be signed in to change notification settings - Fork 52
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 CRUJRA2024 as an option to DATM_MODE #322
Conversation
@jedwards4b
Also I didn't see a more recent baseline to compare to in cesm_baselines and did not know if I should have looked elsewhere. |
@slevis-lmwg Yes this is an expected failure. I will generate an update to baseline tests this week, we can merge this if you are ready. Thank you for adding tests specific to this new datm mode. |
Thanks @jedwards4b. I will discuss my corresponding CTSM mods with @ekluzek in the next day or two in order to get his comments. Meanwhile @ekluzek I'm ok with Jim merging this PR but let me know if you disagree. @jedwards4b a heads up though: My branch here describes as |
@slevis-lmwg I did the merge here @jedwards4b |
Looks good, thank you for pointing that out, I hadn't noticed that you had done that. My mods seem orthogonal to those of the merge. Since you were going to generate a new baseline, I will let you run aux_cdeps this time, and I would expect it to work fine. |
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.
@slevis-lmwg and I went over this together and we noted the things to do. I'm marking this as "requesting changes", but once the conversations are resolved this will be good to go.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
@slevis-lmwg you'll need to update you share version in your CTSM clone in order to use the updated form for shr_sys_abort. |
@slevis-lmwg you just need to update the cesm_share external to share1.1.8 |
@ekluzek thank you. If I update /share, will shr_sys_abort still work in the other components that I'm pointing to and in clm? |
@slevis-lmwg share1.1.8 and everything should work. |
This reverts commit 3000f0c. The alternative to get the same outcome is to update to share1.1.8
Thank you, @jedwards4b |
@ekluzek CRUJRA is second in line in CTSM's upcoming tags, which requires this cdeps PR merged pending your approval (just a reminder). |
@jedwards4b should I run aux_cdeps against cdeps1.0.55 again given the latest merge of main? |
...and should I generate a new baseline in the process? |
@slevis-lmwg that would be great - thank you |
Thanks @jedwards4b. |
aux_cdeps results These are new tests, so this result is expected:
Otherwise, NLCOMP diffs, which I believe are expected. |
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.
Awesome to see this coming in. I make a few suggestions, so marked it so. But, they shouldn't be hard to do. But, feel free to ping me to discuss...
@ekluzek @jedwards4b I think you would then make the new cdeps1.0.68 tag? Or would Erik or I do it? |
I can merge and make the new tag as soon as @ekluzek follows through on approval. |
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 ask for some changes to comments. But, marking as approve, so you don't have to ask me again. Thanks for the work here @slevis-lmwg, I'm really glad to see this come in.
@jedwards4b I will rerun aux_cdeps since we changed one of the tests. I will be in touch as soon as it is done. |
@slevis-lmwg in terms of process, I think you've seen what to do here. But, to make it concrete here are what the steps are for here (as I view it) and other repositories like it (CMEPS, cime, etc..). This is how I view it, and @jedwards4b can confirm or suggest changes to anything below...
|
@jedwards4b aux_cdeps worked as expected, and I generated a new cdeps1.0.68 baseline. This is ready. |
@jedwards4b thank you for the merge. A heads up that in reviewing ESCOMP/CTSM#2956, @ekluzek found a few new issues that I will revise in cdeps. This means that I will open a new small cdeps PR, likely this afternoon. |
Additional minor revisions to #322
Description of changes
Change the default datm input from GSWP3v1 to CRUJRA2024 for Clm6.
Add a CRUJRA2024 compset for clm5. <-- This gets addressed in ctsm exclusively as far as I can tell.
Connected to the changes in ESCOMP/CTSM#2956 and the corresponding issue ESCOMP/CTSM#1895
See definition of DONE in the issue ESCOMP/CTSM#1895
Specific notes
Contributors other than yourself, if any:
CDEPS Issues Fixed (include github issue #):
#323
#324
Are there dependencies on other component PRs (if so list):
ESCOMP/CTSM#2956
Are changes expected to change answers (bfb, different to roundoff, more substantial):
Yes, see note in ESCOMP/CTSM#2956
Any User Interface Changes (namelist or namelist defaults changes):
Yes.
Testing performed (e.g. aux_cdeps, CESM prealpha, etc):
None, yet.
Hashes used for testing: