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

Feature/jkolassa/gndtmp cn cleanup #536

Merged
merged 14 commits into from
Feb 13, 2022

Conversation

gmao-jkolassa
Copy link
Contributor

This PR addresses issue #530 and involves a cleanup of subroutines gndtmp and gdntmp_cn.

Subroutine gndtmp was modified to make several input arguments optional and make parts of the code dependent on the presence of these optional input arguments. As a result gndtmp can now be called instead of gndtmp_cn while still retaining all previous functionality. The now obsolete subroutine gndtmp_cn was removed.

Two points to note/discuss:

  1. I had to change the order of the input arguments in gndtmp in order to avoid having to make all inputs keyword arguments. I have updated all calls to gndtmp accordingly. If the preference is that the input order remains the same, I can make the necessary changes, but it will mean that all input arguments have to be passed as keyword arguments.
  2. This branch has been created from 'develop' and as a result it does not contain the changes to gndtmp(_cn) and the temperature variable names that have been implemented as part of the peat PR. Depending on when each PR is expected to be merged, I can include the changes that were made in the peat PR here.

I have tested this new branch in 3 one-month offline experiments using Catchment, Catchment-CN4.0 and Catchment-CN4.5. For all three experiments the model outputs after one month were zero-diff with respect to a simulation generated with 'develop'. I have not run any coupled simulations.

@gmao-jkolassa gmao-jkolassa added 0 diff The changes in this pull request have verified to be zero-diff with the target branch. cleanup labels Feb 10, 2022
Copy link
Contributor

@gmao-rreichle gmao-rreichle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

preliminary 0-diff tests successful:

  • AGCM (Biljana)
  • GEOSldas (Rolf) [as expected, tests with aggressive build had roundoff differences]

@gmao-rreichle gmao-rreichle marked this pull request as ready for review February 11, 2022 11:11
@gmao-rreichle gmao-rreichle requested a review from a team as a code owner February 11, 2022 11:11
@sdrabenh sdrabenh merged commit b37677a into develop Feb 13, 2022
@sdrabenh sdrabenh deleted the feature/jkolassa/gndtmp_cn_cleanup branch February 13, 2022 21:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0 diff The changes in this pull request have verified to be zero-diff with the target branch. cleanup
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants