Skip to content
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

fixing lagged radiation bug to sync with model time; removing TWC cha… #786

Open
wants to merge 1 commit into
base: atmosphere/v6.x-openacc
Choose a base branch
from

Conversation

bawilt0920
Copy link

fixing lagged radiation bug to sync with model time; removing TWC changes to nTiedtke convection

@mgduda mgduda self-requested a review January 13, 2021 05:04
@mgduda
Copy link
Contributor

mgduda commented Jan 19, 2021

@bawilt0920 Thanks very much for these changes. Whenever possible, we try to use separate commits for independent sets of changes, and for unrelated commits, we typically create separate branches and PRs. Development on the atmosphere/v6.x-openacc branch is a little less formal than development on the main develop branch, so we could probably just use a single PR, here. But, it would be nice to separate the changes to the nTiedtke scheme from the fix for the lagged radiation. If you could add a short explanation of the changes to the commit messages, that would also be good. (Here are a couple of examples of the level of detail that we typically go into in commit messages: 1, 2, 3, and 4).

If there's any assistance that I could provide in splitting the single commit in this PR into two separate commits, just let me know and I'll be glad to help. One possible way to proceed might be to checkout the atmosphere/v6.x-openacc_twc in a clone of your fork, then to reset the HEAD of the branch to 498393d, at which point you can commit the changes individually before force-pushing the atmosphere/v6.x-openacc_twc branch back to your fork. The PR should automatically be updated to show your two commits.

@raghuraj19
Copy link

@mgduda L Is this fix for 6.x? Would the changes apply for 7.x or does a new commit needs to be made for 7.x?

@mgduda
Copy link
Contributor

mgduda commented Aug 21, 2021

@mgduda L Is this fix for 6.x? Would the changes apply for 7.x or does a new commit needs to be made for 7.x?

Are you referring to the lagged radiation fix specifically, or to all of the changes in this PR? I think the lagged radiation fix would apply to v7.x as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants