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

Change default for SNOW_ALBEDO_INFO (GEOS_SurfaceGridComp.rc) #989

Draft
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

gmao-rreichle
Copy link
Contributor

@gmao-rreichle gmao-rreichle commented Aug 29, 2024

@biljanaorescanin, @sdrabenh, @mathomp4,

I'm wondering how we can go about changing the default for the SNOW_ALBEDO_INFO resource parameter. The newest "v12" bcs are meant to be used with SNOW_ALBEDO_INFO=1, both in the new (post-Krok) AGCM (w/ increased vertical resolution etc) and in the new SMAP L4_SM Version 8 system. Going forward, we don't want users to run the model with the old SNOW_ALBEDO_INFO=0 by default. We can't keep instructing users to make this change manually just to keep current with the latest development, and we need to find a way to change the default for both the AGCM and GEOSldas.

Changing the default in the GEOS_SurfaceGridComp.rc (as in 1c12f21) should work for GEOSldas, owing to the preprocessing of GEOS_SurfaceGridComp.rc during ldas_setup. But this will upset the GEOSldas (nightly) regression tests (which also still use NLv3 bcs). To fix this, we could either change the tests to use v12 bcs (plus the new SNOW_ALBEDO_INFO default) and re-baseline, or we could add an explicit specification of SNOW_ALBEDO_INFO=0 in the GEOSldas test config (and keep using the old NLv3 bcs).

For the AGCM, the edit that engages the new default should be 2c236b0. (I think gcm_setup mostly ignores GEOS_SurfaceGridComp.rc.) But this will likewise upset the AGCM regression tests. Same choice here -- switch to new bcs and re-baseline, or augment with explicit SNOW_ALBEDO_INFO=0 config.

Please advise, thanks!

PS: I'm not sure if this change should be labeled 0-diff or non-0-diff, so I added both labels.

PPS: The above assumes that the default bcs version is also updated to v12. (SNOW_ALBEDO_INFO=1 will fail with NL3 bcs.)

@gmao-rreichle gmao-rreichle added 0 diff The changes in this pull request have verified to be zero-diff with the target branch. Non 0-diff The changes in this pull request are non-zero-diff labels Aug 29, 2024
@gmao-rreichle gmao-rreichle requested a review from a team as a code owner August 29, 2024 17:09
@mathomp4
Copy link
Member

@gmao-rreichle Well, gcm_setup does have a bit of "fiddle with Surface rc":

# Update LAND_PARAMS Choices
# ------------------------------------------------------------------
/bin/mv $EXPDIR/RC/GEOS_SurfaceGridComp.rc $EXPDIR/RC/GEOS_SurfaceGridComp.tmp
cat $EXPDIR/RC/GEOS_SurfaceGridComp.tmp | sed -e 's?# GEOSagcm=>?            ?g' > $EXPDIR/RC/GEOS_SurfaceGridComp.rc


if( $LSM_CHOICE == 2 ) then
    /bin/mv $EXPDIR/RC/GEOS_SurfaceGridComp.rc $EXPDIR/RC/GEOS_SurfaceGridComp.tmp
    cat $EXPDIR/RC/GEOS_SurfaceGridComp.tmp | sed -e '/^ *LAND_PARAMS:/ s/NRv7.2/CN_CLM40/' > $EXPDIR/RC/GEOS_SurfaceGridComp.rc
endif


if( $LSM_BCS == "ICA" ) then
    /bin/mv $EXPDIR/RC/GEOS_SurfaceGridComp.rc $EXPDIR/RC/GEOS_SurfaceGridComp.tmp
    cat $EXPDIR/RC/GEOS_SurfaceGridComp.tmp | \
    awk '{ if ($1~"LAND_PARAMS:") { sub(/NRv7.2/,"Icarus") }; print }' > $EXPDIR/RC/GEOS_SurfaceGridComp.rc


    /bin/mv $EXPDIR/RC/GEOS_SurfaceGridComp.rc $EXPDIR/RC/GEOS_SurfaceGridComp.tmp
    cat $EXPDIR/RC/GEOS_SurfaceGridComp.tmp | \
    awk '{ if ($1~"Z0_FORMULATION:") { sub(/4/,"2") }; print }' > $EXPDIR/RC/GEOS_SurfaceGridComp.rc
endif

We could do something similar if the land choice is v12. (Or, I suppose change the default to 1 and then if not v12, set to 0?)

@gmao-rreichle
Copy link
Contributor Author

Thanks, @mathomp4. I'm not sure we want to add more fiddle to gcm_setup, but it should work to add something along the lines of (pseudo-code):

if ( $LSM_BCS == "ICA" .or. $LSM_BCS == "NL3" .or. [any other bcs versions that do not allow using SNOW_ALBEDO_INFO=1])
  [insert SNOW_ALBEDO_INFO: 0 into rc file]
end

Adding this should let us continue with the tests as they are. Ultimately, we'll want to update the tests to reflect more recent config choices, so eventually we'll want to update the tests. It's more of a question of when, not if. If we want to postpone updating the tests, @biljanaorescanin can implement the above code snippet.

@biljanaorescanin biljanaorescanin marked this pull request as draft September 5, 2024 11:17
@gmao-rreichle gmao-rreichle changed the base branch from develop to feature/sdrabenh/gcm_v12 February 21, 2025 19:56
@gmao-rreichle gmao-rreichle changed the base branch from feature/sdrabenh/gcm_v12 to develop February 21, 2025 19:58
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. Non 0-diff The changes in this pull request are non-zero-diff
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants