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

disable CatchmentCNCLM45 (LSM_CHOICE=3) #900

Merged
merged 3 commits into from
Feb 21, 2024

Conversation

gmao-jkolassa
Copy link
Contributor

@gmao-jkolassa gmao-jkolassa commented Feb 13, 2024

This PR disables the use of CatchmentCNCLM45 (LSM_CHOICE=3) and supersedes #898.

This particular model version has lots of science issues, and NCAR discourages the use of CLM45, from which the carbon and nitrogen cycle components of CatchmentCNCLM45 are borrowed.

The PR removes the LAND_PARAMS=CN_CLM45 option from the surface resource file and removes the LSM_CHOICE=3 option in select case/if blocks to make sure the code does not run with this model version. A more thorough source code cleanup will be needed at a later time.

Related PRs:

GEOS-ESM/GEOSldas#707

This PR is zero-diff.

See also planned cleanup of source code #907

cc: @gmao-rreichle , @biljanaorescanin

@gmao-jkolassa gmao-jkolassa added the 0 diff The changes in this pull request have verified to be zero-diff with the target branch. label Feb 13, 2024
@gmao-jkolassa gmao-jkolassa self-assigned this Feb 13, 2024
… (GEOS_SurfaceGridComp.F90, SurfParams.F90, GEOS_CatchCNGridComp.F90)
@gmao-rreichle gmao-rreichle changed the title disable Catchment-CN4.5 parameter option in surface resource file disable CatchmentCNCLM45 (LSM_CHOICE=3) Feb 14, 2024
@gmao-rreichle gmao-rreichle added 0 diff trivial The changes in this pull request are trivially zero-diff (documentation, build failure, &c.) and removed 0 diff The changes in this pull request have verified to be zero-diff with the target branch. labels Feb 21, 2024
@gmao-rreichle gmao-rreichle marked this pull request as ready for review February 21, 2024 16:17
@gmao-rreichle gmao-rreichle requested review from a team as code owners February 21, 2024 16:17
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.

PR only comments out a few if blocks for CatchmentCNCLM45. I'd call it trivially 0-diff. CI build and runs work. @sdrabenh, please let us know if you need more formal testing.

@sanAkel
Copy link
Contributor

sanAkel commented Feb 21, 2024

Instead of commenting it, why don't you just remove this choice?

@gmao-rreichle
Copy link
Contributor

Instead of commenting it, why don't you just remove this choice?

A more thorough code cleanup will be done when we replace CatchCNCLM4.5 with CatchCNCLM5.1. This is a quick safety to make sure nobody can bypass the corresponding changes in setup.

@sanAkel
Copy link
Contributor

sanAkel commented Feb 21, 2024

@gmao-rreichle in that case, we'll need an issue that goes with this PR. The issue I suggest would have: A more thorough code cleanup will be done. Does that make sense?

@gmao-rreichle
Copy link
Contributor

@gmao-rreichle in that case, we'll need an issue that goes with this PR. The issue I suggest would have: A more thorough code cleanup will be done. Does that make sense?

@sanAkel : Done.

Copy link

@lcandre2 lcandre2 left a comment

Choose a reason for hiding this comment

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

Fine by me.

@sdrabenh
Copy link
Collaborator

Fine with me too

@sdrabenh sdrabenh merged commit 8faac49 into develop Feb 21, 2024
11 checks passed
@sdrabenh sdrabenh deleted the feature/jkolassa_disable_cn45 branch February 21, 2024 18:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0 diff trivial The changes in this pull request are trivially zero-diff (documentation, build failure, &c.)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants