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.

Sign up for GitHub

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

Jump to bottom

Changes to Logging and Initialization of the CLM Lake Model #92

Merged
merged 25 commits into from
Aug 22, 2023
Merged

Changes to Logging and Initialization of the CLM Lake Model #92

merged 25 commits into from
Aug 22, 2023

Conversation

SamuelTrahanNOAA
Copy link
Collaborator

@SamuelTrahanNOAA SamuelTrahanNOAA commented Jul 27, 2023 •
edited
Loading

This is the work of @tanyasmirnova to fix some initialization issues in the CLM Lake Model. Initialization appears to work correctly without fractional ice. There are some lingering issues with fractional ice initialization that we're working on.

Changes:

  1. Use ice thickness hice(i) to find the level in the lake where ice is zero.
  2. Do not allow lake temperature to be below freezing point if there is no ice.
  3. If there is no snow or ice, do not allow surface lake temperature to be below freezing point. These changes fixed the problem with large errors in the energy budget at the beginning of the cold-start run with lakes.
  4. Added flag to turn on debug print statements in the CLM lake model.

mzhangw and others added 7 commits July 19, 2023 00:15
@mzhangw @mkavulich
Initiate SRW v3.0.0 SciDoc
2b6a7a4
@mzhangw @mkavulich
Some format and file size adjustments
7ec1eb8
@mzhangw @mkavulich
address some review comments
c901390
@mzhangw @mkavulich
Scidoc updates
57f72fb
@mzhangw @mkavulich
narrative description for previous update
8cd2586
@mkavulich
Host large images remotely
2a56d37
@SamuelTrahanNOAA
Changes to logging and initialization of the CLM Lake Model.
9e35de1 
1. Use ice thickness hice(i) to find the level in the lake where ice is
   zero.
2. Do not allow lake temperature to be below freezing point if there is
   no ice.
3. If there is no snow or ice, do not allow surface lake temperature to
   be below freezing point.
   These changes fixed the problem with large errors in the energy budget
   at the beginning of the cold-start run with lakes.
4. Added flag to turn on debug print statements in the CLM lake model.
@SamuelTrahanNOAA SamuelTrahanNOAA marked this pull request as draft July 27, 2023 19:16
@SamuelTrahanNOAA
Copy link
Collaborator Author

This wasn't the final version of Tanya's changes. I'm retesting her final version now, on top of the latest ufs-weather-model.

@SamuelTrahanNOAA SamuelTrahanNOAA mentioned this pull request Jul 27, 2023
Merged
tanyasmirnova
tanyasmirnova approved these changes Jul 27, 2023
View reviewed changes
Copy link
Collaborator

@tanyasmirnova tanyasmirnova left a comment

Choose a reason for hiding this comment

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

@SamuelTrahanNOAA Thank you, Sam, for making a PR with these modification to the CLM lake initialization.

@SamuelTrahanNOAA SamuelTrahanNOAA marked this pull request as ready for review July 27, 2023 22:32
@SamuelTrahanNOAA SamuelTrahanNOAA mentioned this pull request Jul 27, 2023
Merged
33 tasks
grantfirl
grantfirl approved these changes Jul 28, 2023
View reviewed changes
Copy link
Collaborator

@grantfirl grantfirl left a comment

Choose a reason for hiding this comment

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

I don't see any issues from a CCPP point of view. There is a lot more debugging output, of course, but it looks to be controlled by debug flags, so it shouldn't be an issue.

@SamuelTrahanNOAA
Copy link
Collaborator Author

There is a lot more debugging output, of course, but it looks to be controlled by debug flags, so it shouldn't be an issue.

The amount of output from clm_lake_debug=.true. was so large that it was useless. Tanya split it up and added a separate flag to enable the super-verbose prints.

dustinswales
dustinswales approved these changes Jul 31, 2023
View reviewed changes
Copy link
Collaborator

@dustinswales dustinswales left a comment

Choose a reason for hiding this comment

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

Looks good.

physics/clm_lake.f90
write(0,*) 'Warning: wght2 is not 0..1: ',wght2
endif
wght2 = max(0.0_kind_lake,min(1.0_kind_lake,wght2))
endif
wght1 = 1.0_kind_lake - wght2

if(LAKEDEBUG .and. me==0) then
if(debug_print ) then
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we want to do this on all processors?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That lets us confirm the value is the same, and correct, on all ranks.

@jkbk2004 jkbk2004 mentioned this pull request Aug 18, 2023
Closed
41 tasks
@zach1221
Copy link

@SamuelTrahanNOAA regression testing is finished on UFS-WM PR #1844. We can begin the merging process and start with this sub-component ccpp-physics PR.

@SamuelTrahanNOAA
Copy link
Collaborator Author

We can begin the merging process and start with this sub-component ccpp-physics PR.

A fine idea. Unfortunately, I lack access to press the "merge" button. We shall have to wait for a code manager to resurface.

@zach1221
Copy link

zach1221 commented Aug 21, 2023 •
edited
Loading

Hello. @dustinswales @grantfirl would you please merge this PR?

@dustinswales dustinswales merged commit 5b94685 into ufs-community:ufs/dev Aug 22, 2023
3 checks passed
@dustinswales
Copy link
Collaborator

@zach1221 Done. Sorry for the delay.
FYI. I'm more of a backup code manager these days. @grantfirl @Qingfu-Liu @mkavulich are the mains, and I help out as needed. Grant is on holiday this week, so I'll make sure I'm watching PRs closer this week.

@zach1221
Copy link

@zach1221 Done. Sorry for the delay. FYI. I'm more of a backup code manager these days. @grantfirl @Qingfu-Liu @mkavulich are the mains, and I help out as needed. Grant is on holiday this week, so I'll make sure I'm watching PRs closer this week.

@dustinswales Alright, understood. Thank you for helping!

@mkavulich
Copy link
Collaborator

@dustinswales Thanks for stepping in, sorry I missed the merge request here.

@grantfirl grantfirl mentioned this pull request Nov 2, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dustinswales dustinswales dustinswales approved these changes

@grantfirl grantfirl grantfirl approved these changes

@Qingfu-Liu Qingfu-Liu Qingfu-Liu approved these changes

@tanyasmirnova tanyasmirnova tanyasmirnova approved these changes

@JongilHan66 JongilHan66 Awaiting requested review from JongilHan66

@haiqinli haiqinli Awaiting requested review from haiqinli

@mzhangw mzhangw Awaiting requested review from mzhangw mzhangw is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

CLM Lake initialization issues cause an energy budget problem
8 participants
@SamuelTrahanNOAA @zach1221 @dustinswales @mkavulich @grantfirl @Qingfu-Liu @tanyasmirnova @mzhangw