-
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
Bring in constituent tendency updater #292
Conversation
… constituent-update
I had been holding this in draft until the atmospheric_physics PR ESCOMP/atmospheric_physics#111 was merged, but I have since realized that I should get this one reviewed and ready so they can be merged in quick succession (as one requires the other). Apologies for asking for yet another review |
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.
This all looks great to me. Thanks @peverwhee!
.gitmodules
Outdated
url = https://github.com/peverwhee/atmospheric_physics | ||
fxtag = cfe51a37eb2f8b4906ea0acde7b26ae23159d240 |
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.
Just adding a reminder to point this to the official ESCOMP atmospheric_physics repo once it is ready.
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.
Not to add on here but it looks like this might need to be updated to ebbd06d73b076a1437f1212b3224f35b843dfcb6
for current testing?
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.
@mwaxmonsky I've updated to the official repo hash, but I take your point on keeping the PR up to date with the atmos_phys external
This PR is dependent on NCAR/ccpp-framework#584 (merged) and ESCOMP/atmospheric_physics#111 (will be merged immediately before this PR, and the tag will be brought in) |
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.
Looks great @peverwhee! Just one minor thing that we should probably update but otherwise nothing that should hold this up!
.gitmodules
Outdated
url = https://github.com/peverwhee/atmospheric_physics | ||
fxtag = cfe51a37eb2f8b4906ea0acde7b26ae23159d240 |
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.
Not to add on here but it looks like this might need to be updated to ebbd06d73b076a1437f1212b3224f35b843dfcb6
for current testing?
Tag name (required for main):
Originator(s): peverwhee
Summary (include the keyword ['closes', 'fixes', 'resolves'] and issue number): Brings in and enables new apply_constituent_tendencies scheme (from ESCOMP/atmospheric_physics#111)
Describe any changes made to build system: Exclude constituent tendencies from required variables (now handled by the CCPP Framework in NCAR/ccpp-framework#584)
Describe any changes made to the namelist: None
List any changes to the defaults for the input datasets (e.g. boundary datasets): None
List all files eliminated and why: None
List all files added and what they do: None
List all existing files that have been modified, and describe the changes:
(Helpful git command:
git diff --name-status development...<your_branch_name>
)M .gitmodules
M src/data/write_init_files.py
If there are new failures (compare to the existing-test-failures.txt file),
have them OK'd by the gatekeeper, note them here, and add them to the file.
If there are baseline differences, include the test and the reason for the
diff. What is the nature of the change? Roundoff?
derecho/intel/aux_sima:
derecho/gnu/aux_sima:
CAM-SIMA date used for the baseline comparison tests if different than latest: