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

Langmuir turbulence parameterization improvements #1034

Merged

Conversation

alperaltuntas
Copy link
Collaborator

@alperaltuntas alperaltuntas commented Jul 6, 2023

Pull Request Summary

This PR cleans up the calculation of Stokes drift and other diagnostics variables in WW3 for Langmuir turbulence parameterization in CESM, and removes the CESMCOUPLED ifdefs.

Description

  • Provide a detailed description of what this PR does.
    • Add Langmuir turbulence parameterization
  • What bug does it fix, or what feature does it add?
    • Feature - allows enabling calculation of Stokes drift diagnostic quantities.
  • Is a change of answers expected from this PR?
    • No. Though the mod_defs will need to be updated.

Please also include the following information:

  • Add any suggestions for a reviewer
  • Mention any labels that should be added:
    • enhancement, new feature
  • Are answer changes expected from this PR? Please describe the changes and the reason why in addition to which of the following labels would apply:
    • Clean up the calculations of the surface Stokes drift (USSX and USSY) and surface layer averaged Stokes drift (USSHX and USSHY). The calculation of surface layer averaged Stokes drift and the tail contributions to both versions of Stokes drift are activated when LMPENABLED flag in mod_def file is set to True. Tail contribution is added in langmuir mixing computations if SDTAIL flag is set to True. HSLMODE switch determines how HSL (surface layer depth) is specified. HSLMODE=0 means testing (where HSL is set to 10m everywhere), HSLMODE=1 means HSL is received from the nuopc cap.
    • Change the input field of mixed layer depth HML to a more general surface layer depth HSL. This depth is used to calculate the surface layer averaged Stokes drift. When used in CESM, the surface layer depth is defined as 1/5 of the mixed layer depth. Now this 1/5 factor is applied to the mixed layer depth when importing the fields in the cap.
    • Clean up the diagnostic variables for Langmuir turbulence parameterization. Some variables are intermediate quantities and are not used in the parameterization. These variables are now deleted. The key variable diagnosed in WW3 is the surface layer averaged Stokes drift (USSHX and USSHY). The Langmuir enhancement factor (LAMULT) and the surface layer averaged Langmuir number (LASL) are now calculated when they are exported in the cap. LASL is needed for the Langmuir induced entrainment parameterization in CVMix but was not there in the cap code. So it is now added following the way LAMULT is defined.
    • This PR only changes the diagnostic variables in WW3 and shouldn't change the answers of WW3.

Issue(s) addressed

Commit Message

Clean up and refine Langmuir turbulence parameterization computations.
Authors: @qingli411, @alperaltuntas

Check list

Testing

  • How were these changes tested?
  • Are the changes covered by regression tests? (If not, why? Do new tests need to be added?)
  • Have the matrix regression tests been run (if yes, please note HPC and compiler)?
  • Please indicate the expected changes in the regression test output, (Note the list of known non-identical tests.)
  • Please provide the summary output of matrix.comp (matrix.Diff.txt, matrixCompFull.txt and matrixCompSummary.txt):

alperaltuntas and others added 5 commits June 26, 2023 11:14
…rization

In addition to the changes by @qingli411 as summarized below, I have added mod_def namelist parameters to control the Langmuir parameterization and eliminated the CESMCOUPLED and SDTAIL ifdefs by replacing them with if statements (with flags from the newly added namelist) wherever applicable.

 # Pull Request Summary
This PR cleans up the calculation of Stokes drift and other diagnostics variables in WW3 for Langmuir turbulence parameterization in CESM.

* Clean up the calculations of the surface Stokes drift (`USSX` and `USSY`) and surface layer averaged Stokes drift (`USSHX` and `USSHY`). The calculation of surface layer averaged Stokes drift and the tail contributions to both versions of Stokes drift are activated when `W3_CESMCOUPLED` is defined.
* Change the input field of mixed layer depth `HML` to a more general surface layer depth `HSL`. This depth is used to calculate the surface layer averaged Stokes drift. When used in CESM, the surface layer depth is defined as 1/5 of the mixed layer depth. Now this 1/5 factor is applied to the mixed layer depth when importing the fields in the cap.
* Clean up the diagnostic variables for Langmuir turbulence parameterization. Some variables are intermediate quantities and are not used in the parameterization. These variables are now deleted. The key variable diagnosed in WW3 is the surface layer averaged Stokes drift (`USSHX` and `USSHY`). The Langmuir enhancement factor (`LAMULT`) and the surface layer averaged Langmuir number (`LASL`) are now calculated when they are exported in the cap. `LASL` is needed for the Langmuir induced entrainment parameterization in CVMix but was not there in the cap code. So it is now added following the way `LAMULT` is defined.
* This PR only changes the diagnostic variables in WW3 and shouldn't change the answers of WW3. Ideally, this PR shouldn't change the diagnostic variables to be used in CESM (`LAMULT`, `LASL`) as well. But given that the calculation of these variables are moved to the cap, and the way division by zero and physically invalid values (both should be rare) are handled, small changes to these variables are expected.

Co-authored-by: qingli411 <[email protected]>
(Latest PR) Clean up diagnostic variables in WW3 for Langmuir turbulence parameterization
@alperaltuntas
Copy link
Collaborator Author

NOTE: this PR is currently a draft for an initial review from @MatthewMasarik-NOAA and/or @JessicaMeixner-NOAA. I am also mentioning @mvertens and @DeniseWorthen, who may be interested in reviewing this early version.

@MatthewMasarik-NOAA
Copy link
Collaborator

Awesome, @alperaltuntas, it's great to see this work! I have the UFS coupled tests running with it now. I'll post an update tomorrow.

@MatthewMasarik-NOAA
Copy link
Collaborator

@alperaltuntas the runs I submitted yesterday had failures due to the mod_def's. This I should have known since the PR modifies w3gridmd.F90, I needed to generate new mod_def's before running. I will be out next week, but will return and resolve this the following week. I discussed this PR with @JessicaMeixner-NOAA yesterday, and we will leave this in the Draft state while I'm away.

Copy link
Collaborator

@JessicaMeixner-NOAA JessicaMeixner-NOAA left a comment

Choose a reason for hiding this comment

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

@alperaltuntas --- First and foremost - THANK YOU! These updates so far look good and I'm planning on taking an even closer look when I return from a conference at the end of July.

A couple of things that would be helpful in the meantime:

  • Updates to model/inp/* and model/nml/* as appropriate to document new additions
  • Add regression test for new features/output parameters
  • Any updates that should go into the manual (Alternatively, if you'd create an issue with an assigned person + what needs to be document that will be sufficient, we'll just want to make sure we get it before the next public release).

@MatthewMasarik-NOAA
Copy link
Collaborator

@alperaltuntas --- First and foremost - THANK YOU! These updates so far look good and I'm planning on taking an even closer look when I return from a conference at the end of July.

A couple of things that would be helpful in the meantime:

* Updates to model/inp/* and model/nml/* as appropriate to document new additions

* Add regression test for new features/output parameters

* Any updates that should go into the manual (Alternatively, if you'd create an issue with an assigned person + what needs to be document that will be sufficient, we'll just want to make sure we get it before the next public release).

Thank you @JessicaMeixner-NOAA for this initial look and comments. @alperaltuntas I'll start to add in the items Jessica requested, so there's nothing additional needed at this point. I'll work first to get the namelist and regression test implemented. After that maybe we can touch base and together try to tackle some supporting documentation.

@alperaltuntas alperaltuntas marked this pull request as ready for review August 24, 2023 19:59
Copy link
Collaborator

@mvertens mvertens left a comment

Choose a reason for hiding this comment

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

I've suggest a load balancing exercise and some minor changes along with questions.
Frankly - I must not have submitted the suggestion - so please ignore this.

@alperaltuntas
Copy link
Collaborator Author

I've suggest a load balancing exercise and some minor changes along with questions.

@mvertens Could you clarify? I am not sure what you mean by load balancing exercise, and which minor changes and questions you are referring to.

@DeniseWorthen
Copy link
Contributor

DeniseWorthen commented Sep 5, 2023

@alperaltuntas I apologize for not responding earlier. I didn't see any issues w/ your changes in terms of the mesh cap.

At some point, if this feature is used for the unstructured mesh, I think there will need to be a few changes to allow the new fields (usshx,usshy) to be written as gridded output. But I wouldn't view that as a requirement for this PR.

@MatthewMasarik-NOAA I assume you've run (or will run) the UFS RTs with these changes prior to merge?

@MatthewMasarik-NOAA
Copy link
Collaborator

@MatthewMasarik-NOAA I assume you've run (or will run) the UFS RTs with these changes prior to merge?

@DeniseWorthen, yes, definitely. I'm going to make a few edits for the added regtest, which I plan to complete this week. Then @JessicaMeixner-NOAA and will review including running the UFS RTs.

@DeniseWorthen
Copy link
Contributor

@MatthewMasarik-NOAA Thanks. I will be able to approve once all the UFS are shown to pass.

@MatthewMasarik-NOAA
Copy link
Collaborator

@alperaltuntas Glad to see the polishing updates were merged in. Jessica will start her review on Monday.

…/unified

Fix for ww3_tp2.22 plus updates from dev/ufs-weather-model
Copy link
Collaborator

@JessicaMeixner-NOAA JessicaMeixner-NOAA left a comment

Choose a reason for hiding this comment

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

matrixCompFull.txt
matrixCompSummary.txt
matrixDiff.txt

Changes are in the mod_defs and out_grd as expected. Plan to do one last test of ensuring the new regtests reproduce itself, but this PR should be good to have a ufs-weather-model PR submitted.

Copy link
Collaborator

@JessicaMeixner-NOAA JessicaMeixner-NOAA left a comment

Choose a reason for hiding this comment

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

Thank you @alperaltuntas and @MatthewMasarik-NOAA for getting this in shape for eventual merge back to the develop branch.

@ukmo-ccbunney @mickaelaccensi @thesser1 - These updates will eventually be coming back to develop. If there's anything you see that needs to be immediately addressed, please feel free to chime in.

@mickaelaccensi
Copy link
Collaborator

ww3_ounf.nml and ww3_shel.nml should also be updated to add this new output field

@MatthewMasarik-NOAA
Copy link
Collaborator

ww3_ounf.nml and ww3_shel.nml should also be updated to add this new output field

Hi @mickaelaccensi

@JessicaMeixner-NOAA and I discussed this previously (1034#discussion) with the outcome of deciding to limit the current PR to *.inp files, and had added issue #1111 for the expansion to *.nml. Does that seem reasonable?

@mickaelaccensi
Copy link
Collaborator

ok, it seems acceptable to not expand this current PR with nml if it's done in another one already scheduled

@ukmo-ccbunney
Copy link
Collaborator

@ukmo-ccbunney @mickaelaccensi @thesser1 - These updates will eventually be coming back to develop. If there's anything you see that needs to be immediately addressed, please feel free to chime in.
Nothing is jumping out at me, so all good as far as I am concerned. 👍

@MatthewMasarik-NOAA
Copy link
Collaborator

Thanks for your confirmations @mickaelaccensi @ukmo-ccbunney

@MatthewMasarik-NOAA
Copy link
Collaborator

@alperaltuntas @JessicaMeixner-NOAA
I made small updates to the header, the main update checking off the list checkboxes.

@zach1221
Copy link

Ok, testing is complete on WM #2195. @JessicaMeixner-NOAA can you please merge this PR for us?

@JessicaMeixner-NOAA JessicaMeixner-NOAA merged commit d9b3172 into NOAA-EMC:dev/ufs-weather-model Mar 29, 2024
20 checks passed
@MatthewMasarik-NOAA
Copy link
Collaborator

A belated thank you and congratulations to @qingli411 and @alperaltuntas for this very nice contribution to WW3, which was merged into the dev/ufs-weather-model branch last Friday! I'd also like to acknowledge @CarstenHansen, whose early posts to #669 provided helpful insight for implementing the namelist functionality.

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

Successfully merging this pull request may close these issues.

10 participants