Skip to content

Revision of section 2.12 (Lake Model) in technical note for CLM6#3997

Merged
slevis-lmwg merged 16 commits into
ESCOMP:b4b-devfrom
nmizukami:sec2_12_lake_edits
Jun 3, 2026
Merged

Revision of section 2.12 (Lake Model) in technical note for CLM6#3997
slevis-lmwg merged 16 commits into
ESCOMP:b4b-devfrom
nmizukami:sec2_12_lake_edits

Conversation

@nmizukami
Copy link
Copy Markdown
Contributor

Description of changes

After review, I made a few minor edits and various corrections e.g., links, equations. etc. I did not change anything on roughness length update made by @olyson.

Specific changes I made:

  • lots of tailing spaces are removed (due to my vim setting for rst file)
  • Added "equation" before the equation number (to distinguish section numbers), but not sure if this was good change.
  • Remove or update the section on changes since CLM4.
  • Editorial change (2.12. Lake Model)- "atmosphere model of Hostetler et al. (1993), Hostetler et al. (1993)." Remove second "Hostetler et al. (1993)."
  • Editorial change (2.12. Lake Model)- "spatially variable depth" to "spatially varying depth"
  • Editorial change (2.12.1 vertical discretization) - "assumed depth" to "default depth"? Later the text says “default (50 m lake)”
  • Equation style changes - Eqs. 2.12.13, 2.12.35 and 2.12.36.
  • Eq's notation change (2.12.4.2 Surface Flux Solution). Eq. 2.12.21 L↑ should be L_g↑
  • "light extinction coefficient varies between lake columns" based on eq. (2.12.46), I believe. It was empty.
  • "If freezing occurs, Q_avail is again given by" what? I believe it is eq. 2,12.52.
  • Fixed section references ("Monin-Obukhov Similarity Theory" for friction velocity, and "Surface Flux Solution Lake" for surface temperature solution). I believe these are correct references.

Specific notes

Contributors other than yourself, if any:

CTSM Issues Fixed (include github issue #): Resolve #3858

Are answers expected to change (and if so in what way)? N/A

Any User Interface Changes (namelist or namelist defaults changes)? N/A

Does this create a need to change or add documentation? Did you do so? Yes, This PR is for documentation change.

Testing performed, if any:

Built documentation on Casper (NCAR HPC) following the working with documentation guide, no errors.

@nmizukami nmizukami requested review from ekluzek, olyson and samsrabin May 6, 2026 21:23
@nmizukami nmizukami added b4b bit-for-bit docs:minor-tech-fix Minor technical fix needed to documentation content (e.g. broken link, broken rendering) docs-loc:tech-note Relates to Technical Note (science) labels May 6, 2026
@nmizukami nmizukami moved this to In Progress in CLM documentation May 6, 2026
@nmizukami nmizukami linked an issue May 6, 2026 that may be closed by this pull request
25 tasks
@nmizukami nmizukami requested a review from slevis-lmwg May 7, 2026 15:14
Copy link
Copy Markdown
Contributor

@ekluzek ekluzek left a comment

Choose a reason for hiding this comment

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

The changes make sense to me. And cover the things talked about. Removal of the section about updates since the previous version is good.

@nmizukami changed so that equations have equation before it, which makes sense to me. He questioned if that's a good change. But, I think we should go with it. It might be something to look at later for consistency between chapters.

@olyson
Copy link
Copy Markdown
Contributor

olyson commented May 7, 2026

@nmizukami , during my review of and changes to the Surface Albedo chapter, I found that the description of lake albedos was outdated (it referred to the prescribed lake albedos from LSM). So I changed this in the Surface Albedo chapter to follow the current code for lake albedo. I'd suggest then to simply refer to the Surface Albedo chapter (specifically Section 2.3.2 Ground Albedos) in the lake chapter. That way we don't duplicate the text/equations. I think the Ground Albedo section is appropriate since it describes albedos for the different surface types.

@ekluzek
Copy link
Copy Markdown
Contributor

ekluzek commented May 7, 2026

@nmizukami this failed the checker in regard to curly quotes (since Sphynx will handle this). So you'll need to fix that before it can be merged. This should be straightforward, but I can show you if you have any questions.

@samsrabin
Copy link
Copy Markdown
Member

You can see the offending curlies here.

I'm removing myself as reviewer since this isn't my bailiwick.

@samsrabin samsrabin removed their request for review May 7, 2026 17:03
Copy link
Copy Markdown
Contributor

@olyson olyson left a comment

Choose a reason for hiding this comment

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

Thanks for this @nmizukami . My review consists mostly of identifying orphaned equations, which were there before your modifications. See also my comment about lake albedos.

Comment thread doc/source/tech_note/Lake/CLM50_Tech_Note_Lake.rst Outdated
Comment thread doc/source/tech_note/Lake/CLM50_Tech_Note_Lake.rst Outdated
Comment thread doc/source/tech_note/Lake/CLM50_Tech_Note_Lake.rst Outdated
Comment thread doc/source/tech_note/Lake/CLM50_Tech_Note_Lake.rst Outdated
Comment thread doc/source/tech_note/Lake/CLM50_Tech_Note_Lake.rst Outdated
Comment thread doc/source/tech_note/Lake/CLM50_Tech_Note_Lake.rst Outdated
Comment thread doc/source/tech_note/Lake/CLM50_Tech_Note_Lake.rst Outdated
@wwieder
Copy link
Copy Markdown
Contributor

wwieder commented May 27, 2026

@nmizukami have you been able to address all of @olyson's review comments on this?

@nmizukami
Copy link
Copy Markdown
Contributor Author

@nmizukami have you been able to address all of @olyson's review comments on this?

Not yet. I will look into Keith and Sam's comments tomorrow during the party!

Adding equation references

Co-authored-by: Keith Oleson <oleson@ucar.edu>
@github-actions

This comment was marked as resolved.

@nmizukami
Copy link
Copy Markdown
Contributor Author

nmizukami commented May 28, 2026

I believe all the Keith comments have been resolved. Thank you @olyson. I filled a few more missing equation references. Maybe take a quick look again?

@wwieder wwieder requested review from olyson and removed request for slevis-lmwg May 28, 2026 21:53
@wwieder
Copy link
Copy Markdown
Contributor

wwieder commented May 28, 2026

@olyson can you give this one more review to see if @nmizukami addressed your comments.

Copy link
Copy Markdown
Contributor

@olyson olyson 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 thanks!

@olyson olyson self-requested a review June 2, 2026 18:07
@olyson
Copy link
Copy Markdown
Contributor

olyson commented Jun 2, 2026

Actually, don't merge yet, one more thing needs to be done. We need to remove the Surface Albedo section and just refer to the Surface Albedo chapter.

@slevis-lmwg
Copy link
Copy Markdown
Contributor

Ok, let me know :-)

@slevis-lmwg
Copy link
Copy Markdown
Contributor

Ok, I'm looking. I see two spots that need the change. I can go ahead and make these changes.

Copy link
Copy Markdown
Contributor

@slevis-lmwg slevis-lmwg left a comment

Choose a reason for hiding this comment

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

I found two instances of referring to an albedo section instead of an albedo chapter.

Comment thread doc/source/tech_note/Lake/CLM50_Tech_Note_Lake.rst Outdated
Comment thread doc/source/tech_note/Lake/CLM50_Tech_Note_Lake.rst Outdated
Replace reference to the albedo lake section with rst_Surface Albedos
Replace second reference to albedo lake section with rst_Surface Albedos
@slevis-lmwg slevis-lmwg moved this from In Progress to In review in CLM documentation Jun 2, 2026
@slevis-lmwg
Copy link
Copy Markdown
Contributor

slevis-lmwg commented Jun 2, 2026

@olyson I'm guessing that it's ok to merge now, but I will wait until tomorrow in case you think I missed something in my changes.

@olyson
Copy link
Copy Markdown
Contributor

olyson commented Jun 2, 2026

@olyson I'm guessing that it's ok to merge now, but I will wait until tomorrow in case you think I missed something in my changes.

@slevis-lmwg , sorry, my comment was confusing. The lake albedos are described in Section 2.3.2 and we want to simply either refer to that section from 2.12.3, or remove 2.12.3 entirely and just have a sentence about albedo, referencing Section 2.3.2.

Comment thread doc/source/tech_note/Lake/CLM50_Tech_Note_Lake.rst Outdated
Removing first half of Surface Albedo section
@github-actions

This comment was marked as resolved.

@github-actions

This comment was marked as resolved.

@slevis-lmwg
Copy link
Copy Markdown
Contributor

@nmizukami Keith told me that you plan to make the changes that he suggested. I just wanted to mention that there's a b4b-dev merge to master happening tomorrow. If you make the final changes today, then I can merge your branch to b4b-dev before the merge to master.

I could also make the changes, but then I need you to grant me "collaborator" access. If you decide to do that, you click on Settings near the top of this page, then Collaborators and Teams, and then follow instructions for adding me (slevis-lmwg) as collaborator.

@slevis-lmwg
Copy link
Copy Markdown
Contributor

I have been trying to make the changes myself here in github directly in the PR, but github won't let me remove multiple lines of text for some reason. So then I tried by checking out your branch and pushing, but that's where I need collaborator permission to push...

Sorry for any confusion I may have caused.

@nmizukami
Copy link
Copy Markdown
Contributor Author

Hi @slevis-lmwg, I just added you as a collaborator. Can you try to push to my repo now?

Comment thread doc/source/tech_note/Lake/CLM50_Tech_Note_Lake.rst Outdated
Comment thread doc/source/tech_note/Lake/CLM50_Tech_Note_Lake.rst Outdated
Change section reference from rst_Surface Albedos to Ground Albedos
Change section reference from rst_Surface Albedos to Ground Albedos
@slevis-lmwg slevis-lmwg merged commit 1aa20f5 into ESCOMP:b4b-dev Jun 3, 2026
7 checks passed
@github-project-automation github-project-automation Bot moved this from In review to Done in CLM documentation Jun 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

b4b bit-for-bit docs:minor-tech-fix Minor technical fix needed to documentation content (e.g. broken link, broken rendering) docs-loc:tech-note Relates to Technical Note (science)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Review 2.12. Lake Model

6 participants