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

Introduce land ice surface mass balance fields in default runs without MECs/GLC #6682

Merged
merged 16 commits into from
Dec 6, 2024

Conversation

chloewhicker
Copy link
Contributor

@chloewhicker chloewhicker commented Oct 13, 2024

Introduced new diagnostic Surface Mass Balance fields, they are calculated as if there is a coupled land ice component but are only diagnostic - it does not modify the energy/water budgets in default or coupled GLC simulations.

[BFB]


More info on modifications and testing can be found here

@bishtgautam bishtgautam self-requested a review October 15, 2024 15:34
@bishtgautam bishtgautam self-assigned this Oct 15, 2024
@bishtgautam bishtgautam added Land BFB PR leaves answers BFB labels Oct 15, 2024
@rljacob rljacob assigned jonbob and unassigned bishtgautam Oct 31, 2024
@rljacob rljacob requested review from matthewhoffman and removed request for bishtgautam October 31, 2024 17:20
@rljacob
Copy link
Member

rljacob commented Nov 12, 2024

@matthewhoffman please review.

Copy link
Contributor

@matthewhoffman matthewhoffman left a comment

Choose a reason for hiding this comment

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

@chloewhicker , thanks for this PR, and sorry for the delay in reviewing it. I'm not familiar with this code, but I followed that you are mirroring the qflx_glcice calculations with new _diag versions of the relevant variables, and that all makes sense. I requested a number of code formatting/whitespace changes to make it clearer what the PR is actually doing.

@chloewhicker
Copy link
Contributor Author

Thanks for the feedback Matt - all comments have been addressed in the new commit!

Copy link
Contributor

@matthewhoffman matthewhoffman left a comment

Choose a reason for hiding this comment

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

@chloewhicker , thanks for making the minor adjustments from my review. This looks good to me! @bishtgautam , would you like to look this over?

Comment on lines 5854 to 5866
call hist_addfld1d (fname='QICE', units='mm/s', &
avgflag='A', long_name='ice growth/melt', &
ptr_col=this%qflx_glcice, l2g_scale_type='ice')

this%qflx_glcice_frz(begc:endc) = spval
call hist_addfld1d (fname='QICE_FRZ', units='mm/s', &
avgflag='A', long_name='ice growth', &
ptr_col=this%qflx_glcice_frz, l2g_scale_type='ice')

this%qflx_glcice_melt(begc:endc) = spval
call hist_addfld1d (fname='QICE_MELT', units='mm/s', &
avgflag='A', long_name='ice melt', &
ptr_col=this%qflx_glcice_melt, l2g_scale_type='ice')
Copy link
Contributor

Choose a reason for hiding this comment

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

How about adding diagnostic and (with active GLC/MECs) in the long_name to mirror what is done below?

Copy link
Contributor

Choose a reason for hiding this comment

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

@bishtgautam -- does that address your questions?

Copy link
Contributor

Choose a reason for hiding this comment

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

It does.

@bishtgautam bishtgautam requested a review from jonbob November 26, 2024 16:29
@bishtgautam bishtgautam self-requested a review November 26, 2024 16:29
jonbob added a commit that referenced this pull request Dec 5, 2024
Introduce land ice surface mass balance fields in default runs without MECs/GLC

Introduced new diagnostic Surface Mass Balance fields, they are
calculated as if there is a coupled land ice component but are only
diagnostic - it does not modify the energy/water budgets in default
or coupled GLC simulations.

[BFB]
@jonbob
Copy link
Contributor

jonbob commented Dec 6, 2024

passes e3sm_developer on chrysalis except for a few I-cases that show DIFFs due to a change in fieldlists but otherwise BFB

merged to next

@wlin7
Copy link
Contributor

wlin7 commented Dec 6, 2024

The additional fields QICE, QICE_FRZ, QICE_MELT in elm.h0 do also cause prod_next tests to report DIFFs. The simulation results are otherwise unaffected.

@jonbob jonbob merged commit 79d4b09 into E3SM-Project:master Dec 6, 2024
9 checks passed
@jonbob
Copy link
Contributor

jonbob commented Dec 6, 2024

merged to master and expected DIFFs due only to different field lists blessed -- except for pm-cpu, which had issues overnight

@jonbob
Copy link
Contributor

jonbob commented Dec 9, 2024

Also blessed on pm-cpu and gcp12

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BFB PR leaves answers BFB Land
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants