Improved blending of 1D and 3D turbulence schemes

[Adrian Lock (Adrian-Lock)]

PR Summary

Sci/Tech Reviewer: Shusuke Nishimoto (@mo-snishimoto)
Code Reviewer: Ricky Wong (@mo-rickywong)

RMED problem #111 and development ticket #113 document problems found with the scale-aware wind gust diagnostic (stash 3515) that are much improved by these revisions to the method= 'blend_cth_shcu_only' option for how the 1D column and 3D Smagorinsky turbulence schemes are blended as a function of resolution. There is also potential for small benefit from including an additional new option (cap_blended_ml) to cap the blended length scale to be at most the physical length scale from the 1D scheme.

Code Quality Checklist

• I have performed a self-review of my own code
• My code follows the project's style guidelines
• Comments have been included that aid understanding and enhance the readability of the code
• My changes generate no new warnings
• All automated checks in the CI pipeline have completed successfully

Testing

• I have tested this change locally, using the LFRic Apps rose-stem suite
• If any tests fail (rose-stem or CI) the reason is understood and acceptable (e.g. kgo changes)
• I have added tests to cover new functionality as appropriate (e.g. system tests, unit tests, etc.)
• Any new tests have been assigned an appropriate amount of compute resource and have been allocated to an appropriate testing group (i.e. the developer tests are for jobs which use a small amount of compute resource and complete in a matter of minutes)

trac.log

Test Suite Results - lfric_apps - testmain_blending_opt4/run1

Suite Information

Item	Value
Suite Name	testmain_blending_opt4/run1
Suite User	adrian.lock
Workflow Start	2026-09-09T21:53:29
Groups Run	developer

Dependency	Reference	Main Like
casim	MetOffice/casim@2026.07.1	True
jules	MetOffice/jules@2026.07.1	True
lfric_apps	Adrian-Lock/lfric_apps@testmain_blending_opt4	False
lfric_core	MetOffice/lfric_core@3e965e8	True
moci	MetOffice/moci@2026.07.1	True
SimSys_Scripts	MetOffice/SimSys_Scripts@77a5166	True
socrates	MetOffice/socrates@2026.07.1	True
socrates-spectral	MetOffice/socrates-spectral@2026.07.1	True
ukca	MetOffice/ukca@9fc2b6d	True

Task Information

❌ failed tasks - 3

Task	State
check_lfric_atm_ral_dev-seuk_MG_azspice_gnu_fast-debug-32bit-crun1	failed
check_lfric_atm_ral_dev-seuk_MG_ex1a_cce_fast-debug-32bit-crun1	failed
kgo_groups_checker	failed

✅ succeeded tasks - 1220

⌛ waiting tasks - 2

Task	State
housekeep_azspice	waiting
housekeep_ex1a	waiting

Security Considerations

• I have reviewed my changes for potential security issues
• Sensitive data is properly handled (if applicable)
• Authentication and authorisation are properly implemented (if applicable)

Performance Impact

• Performance of the code has been considered and, if applicable, suitable performance measurements have been conducted

AI Assistance and Attribution

• Some of the content of this change has been produced with the assistance of Generative AI tool name (e.g., Met Office Github Copilot Enterprise, Github Copilot Personal, ChatGPT GPT-4, etc) and I have followed the Simulation Systems AI policy (including attribution labels)

Documentation

• Where appropriate I have updated documentation related to this change and confirmed that it builds correctly

PSyclone Approval

• If you have edited any PSyclone-related code (e.g. PSyKAl-lite, Kernel interface, optimisation scripts, LFRic data structure code) then please contact the HPC Optimisation Team

Sci/Tech Review

• I understand this area of code and the changes being added
• The proposed changes correspond to the pull request description
• Documentation is sufficient (do documentation papers need updating)
• Sufficient testing has been completed

(Please alert the code reviewer via a tag when you have approved the SR)

Code Review

• All dependencies have been resolved
• Related Issues have been properly linked and addressed
• CLA compliance has been confirmed
• Code quality standards have been met
• Tests are adequate and have passed
• Documentation is complete and accurate
• Security considerations have been addressed
• Performance impact is acceptable

[iboutle]

Thanks Adrian,

My main comment is around the new test being introduced. blopt4 is a bit opaque, especially given this magic number doesn't really mean anything in the lfric namelist. But also, I'm reluctant to end up with a plethora of different tests for every individual namelist option. So...

• how about calling the test raldev, and it can be built on with other things aimed at future RAL releases
• In the optional config, instead of duplicating everything from the ral3 config, just include the bits which you need that are different from the ral3 config, and then include both when setting up the test.
• Call the optional config something descriptive like blend_shcu or suchlike.
  • To get the metadata checker not to fail, you may have to include slightly more than you need here, as it needs to be a valid config that can be applied to the base app. That probably just means including the options needed to switch Smagorinsky on, since that is off in the base app.

Thanks!

[iboutle]

Dear Adrian Lock (Adrian Lock (Adrian Lock (@Adrian-Lock))) , I confirmed there were not problems but I left 2 minor comments. I hope this helps.
I also have to comment that modifying mixing namelists will arise a problem in unit-tests ( diffusion_momentum_flux_kernel_mod_test.pf and divergence_momentum_flux_kernel_mod_test.pf ), which are not exist in stable but only in main after merging this branch into main as I wrote in #725. So, could you please merge main into this branch and modify these unit_tests as done in #725 ? As a alternative way, I think it would be also possible to add 2 new namelists (cap_blended_ml and shallow_cu_maxtop) in not mixing section but blayer section since they are meant to be moved to blayer section in my PR(#734) in any case. (I think versions.py under um-boundary_layer directory shoud be modified instead of lfric-gungho directory in that case. I apologize the inconvenience due to my past PR in any case.
Many thanks Shusuke

Yes, updating my branch to main and sorting these aspects out now sounds very sensible. Just one question about using github that I still don't understand. My branch is called "vn3.2_...." because it is a branch from lfric_apps at that stable version. Hence I don't really want to upgrade it to be a branch from main (which is what merging in main will do, I believe) because the name will then be confusing. But if I create a new branch from main, "main_blending_opt4" say, and merge my vn3.2 branch into that, I would have to open a new PR to be associated with that new "main_blending_opt4" branch, wouldn't I? I suspect I am stuck with upgrading my vn3.2 branch but did want to check if you knew of a way round that!

No, there's no way around this - PRs are always attached to a single branch, and branches headed for main will always have main merged onto them at some point prior to commit - so it's generally not a good idea to put "vn3.2" in the branch name of anything headed to main. What you can do is tag the revision of the branch prior to merging main on:
git tag -a <label name> -m <message> <commit hash>
git push origin tag <tag_name>
that way vn3.2 suites can point to the tag instead of the head of the branch.

[Adrian Lock (Adrian-Lock)]

Sorry, that was co-pilot's suggestion to rename the branch, which it claimed would mean github would then automatically update the PR to the new name. Rather than close the PR, which is what it actually did! Presumably it interpreted the rename as a deletion of the vn3.2 branch.

At least it would let me reinstate the old vn3.2 branch and then reopen this PR.

Interestingly I now have a copy of my vn3.2 branch ("main_blending_opt4" that I attempted to rename it to) but that's going to be even more confusing if I upgrade my vn3.2 branch to main, so I'll just delete that one! Sorry about this...

[Adrian Lock (Adrian-Lock)]

Am I right in thinking that the commit of #656 since vn3.2 means my ral_dev kgo is expected to change now I've merged in main? I believe #656 adds conservative_diffusion to momentum for these ral3 apps, which have conservative_diffusion=.true..

[iboutle]

Am I right in thinking that the commit of #656 since vn3.2 means my ral_dev kgo is expected to change now I've merged in main? I believe #656 adds conservative_diffusion to momentum for these ral3 apps, which have conservative_diffusion=.true..

Yes, I think so

[Adrian Lock (Adrian-Lock)]

Dear Adrian Lock (Adrian Lock (@Adrian-Lock)) , I confirmed there were not problems but I left 2 minor comments. I hope this helps.

I also have to comment that modifying mixing namelists will arise a problem in unit-tests ( diffusion_momentum_flux_kernel_mod_test.pf and divergence_momentum_flux_kernel_mod_test.pf ), which are not exist in stable but only in main after merging this branch into main as I wrote in #725. So, could you please merge main into this branch and modify these unit_tests as done in #725 ? As a alternative way, I think it would be also possible to add 2 new namelists (cap_blended_ml and shallow_cu_maxtop) in not mixing section but blayer section since they are meant to be moved to blayer section in my PR(#734) in any case. (I think versions.py under um-boundary_layer directory shoud be modified instead of lfric-gungho directory in that case. I apologize the inconvenience due to my past PR in any case.

Many thanks Shusuke

I hope it's ok but I've gone for now with adding my two new namelist variables (cap_blended_ml and shallow_cu_maxtop) to your two new unit tests, so I have left them in the mixing namelist for now. I agree it would make sense to move them both to blayer to be consistent with the changes in #734 but I think it would be cleaner to make those moves alongside #734 rather than separately. Up to the code reviewers which way round to do it. Is that ok?
Thanks, Adrian

[Adrian Lock (Adrian-Lock)]

My rose stem developer test with main merged into vn3.2_blending_opt4 has passed everything except the ral_dev kgo change, as expected. For some reason it has failed to produce a trac.log and I can't immediately find any instructions on how to recreate it (I tried the old $UMDIR/SimSys_Scripts/suite_report but that appears to be fcm only?)

[Shusuke Nishimoto (mo-snishimoto)]

Thank you for reflecting my comments. I apologize that I missed the bug in OMP directive. I confirmed everything looks fine. So, I am happy to pass this PR to Code reviewer.

I think it would be cleaner to make those moves alongside #734 rather than separately. Up to the code reviewers which way round to do it. Is that ok?

Yes, I agree with your suggestion. I will resolve conflicts in my branch (in #734) flexibly if the timing of merge is after yours.

[Adrian Lock (Adrian-Lock)]

Just managed to recreate the trac.log for my merged-main revision so now pasted into this PR. I've also updated the kgo in the branch, consistent with the evolution of main from stable

[James Bruten (james-bruten-mo)]

A couple of code owner requests from me

[James Bruten (james-bruten-mo)]

I think you need a second self.add_setting for the shallow_cu_maxtop variable

[Adrian Lock (Adrian-Lock)]

Oh yes, good spot, thanks. Bizarrely, I'd also left out the return config, self.reports, maybe in the merge of main (not sure why it still worked), so added that back in too.

[James Bruten (james-bruten-mo)]

Could you revert these changes please - the issue with the builds timing out is more complex that just the time limit being too short

[Adrian Lock (Adrian-Lock)]

Done in latest commit

[James Bruten (james-bruten-mo)]

Same as the default directives file

[Adrian Lock (Adrian-Lock)]

Done in latest commit

[Adrian Lock (Adrian-Lock)]

Thanks James Bruten (@james-bruten-mo). I think I've corrected those and I reran the macro on a test branch to make sure it still worked, which it did.

[thomasmelvin]

LGTM

[James Bruten (james-bruten-mo)]

Thanks Adrian, all good for my parts now

[Adrian Lock (Adrian-Lock)]

Just removed the end-of-line white space I'd added in the documentation, that was causing lint sphinx errors. Many thanks, Shusuke, for pointing those out

[Shusuke Nishimoto (mo-snishimoto)]

I apologize for conducting the review while you were still working on. I missed the comment from two posts ago. Thank you for replacing trac.log with latest one. I think it looks fine.

And I also noticed that there seem trailing-whitespaces in the documentation (from L2127 to L2141 in bl_scheme_doc.rst) as alerted by the the automated checks. I am so sorry for letting you know after approving once.

[Shusuke Nishimoto (mo-snishimoto)]

Thank you for reflecting my comment. Although I have no idea why sphinx check wasn't triggered in the latest revision, I confirmed trailing-whitespace has been removed. So, I am happy to approve again.