Implementation of conservative momentum diffusion and tensorial stress option by mo-snishimoto · Pull Request #656 · MetOffice/lfric_apps

Shusuke Nishimoto (mo-snishimoto) · 2026-07-23T10:50:49Z

PR Summary

Sci/Tech Reviewer: Thomas Bendall (@tommbendall)
Code Reviewer: allynt

Implements conservative form Smagorinsky diffusion operator for momentum and tensorial version of stress option.

Please see Implementation of conservative momentum diffusion and tensorial stress option #479 for details, documentation and extra tests performed.

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 - test_fullstress/run2

Suite Information

Item	Value
Suite Name	test_fullstress/run2
Suite User	shusuke.nishimoto
Workflow Start	2026-08-04T19:32:13
Groups Run	all

Dependency	Reference	Main Like
casim	MetOffice/casim@2026.07.1	True
jules	MetOffice/jules@2026.07.1	True
lfric_apps	mo-snishimoto/lfric_apps@fullstress2	False
lfric_core	MetOffice/lfric_core@2026.07.1	True
moci	MetOffice/moci@2026.07.1	True
SimSys_Scripts	MetOffice/SimSys_Scripts@2026.07.1	True
socrates	MetOffice/socrates@2026.07.1	True
socrates-spectral	MetOffice/socrates-spectral@2026.07.1	True
ukca	MetOffice/ukca@2026.07.1	True

Task Information

❌ failed tasks - 13

Task	State
check_gungho_model_robert-moist-smag-BiP100x8-10x10_azspice_gnu_fast-debug-64bit	failed
check_gungho_model_robert-moist-smag-BiP100x8-10x10_ex1a_gnu_fast-debug-64bit	failed
check_lfric_atm_comp_tran_ref_3d_l120-BiP64x64-1500x1500_MG_ex1a_cce_fast-debug-32bit	failed
check_lfric_atm_nwp_comorph_dev-C12_azspice_gnu_fast-debug-32bit-crun1	failed
check_lfric_atm_nwp_comorph_dev-C12_ex1a_cce_fast-debug-32bit-crun1	failed
check_lfric_atm_ral3-seuk_MG_azspice_gnu_fast-debug-32bit	failed
check_lfric_atm_ral3-seuk_MG_ex1a_cce_fast-debug-32bit	failed
check_lfric_atm_ral3_ens-seuk_MG_azspice_gnu_fast-debug-32bit-crun1	failed
check_lfric_atm_ral3_ens-seuk_MG_ex1a_cce_fast-debug-32bit-crun1	failed
check_lfric_atm_ral3_mixmol-seuk_MG_azspice_gnu_fast-debug-32bit-crun1	failed
check_lfric_atm_ral3_mixmol-seuk_MG_ex1a_cce_fast-debug-32bit-crun1	failed
check_lfric_atm_rce-BiP64x64-1500x1500_MG_azspice_gnu_fast-debug-32bit	failed
check_lfric_atm_rce-BiP64x64-1500x1500_MG_ex1a_cce_fast-debug-32bit	failed

✅ succeeded tasks - 1588

⌛ 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 TCD 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

Just one issue with the metadata

iboutle · 2026-07-30T08:31:14Z

    =angles calculated from the orbital elements but retains the distance
    =of the planet from the host star.
-sort-key=Panel-A17
+sort-key=Panel-[namelist:mixing=fullstress]


Think something has gone wrong here - please revert this change!

Shusuke Nishimoto (mo-snishimoto) · 2026-07-30T09:45:16Z

Just one issue with the metadata

Thank you so much for the check! (and apologize for the mistake.)
I fixed the mistake in rose-meta.conf.
I also fixed the year of the copyright, which I found wrong.

Thomas Bendall (tommbendall)

Thanks for this. This is a correct implementation of a very fiddly and technical bit of science, and I really appreciate that you took on this important bit of scientific capability. It's impressive that you've managed to handle the stencils and W1/W2 DoFs correctly, and thank you for using the face selectors and putting the k-loops innermost to ensure that the kernels will run efficiently.

I have no major worries but do have a few requests or suggestions.

1. Documentation: Thank you for writing up your approach in a PDF (in the linked issue), which was very helpful for the review. Actually it would be really good to get this documentation in .rst format onto LFRic-Apps main itself in the documentation folder. I appreciate that there is currently very little documentation in there, and that I didn't do this for the scalar diffusion routines! If you like you could add the documentation in this Pull Request, or we could have a follow-up Pull Request which adds both documentation for both scalar and vector diffusion routines? Let me know which you'd rather...

2. Results: Thank you also for the results PDF in the linked issue, which were helpful in showing the magnitude of the changes and show that it doesn't break the existing behaviour. Do you have any plots from the lfric_atm tests which changed KGOs, just as a sanity check that there is also not much change to these tests?

3. Interpolation kernel: suggest removing the new Wtheta -> W3 interpolation kernel as there is a kernel that already does this (see my comments below)

4. Other comments: I have also added some suggestions to expand on the in-line or Doxygen comments, and some very fussy style suggestions (sorry)

Thomas Bendall (tommbendall) · 2026-07-31T09:04:29Z

So there is actually already an LFRic-Core kernel that does this: sci_sample_wtheta_to_w3_kernel_mod.F90, https://github.com/MetOffice/lfric_core/blob/main/components/science/source/kernel/inter_function_space/sci_sample_wtheta_to_w3_kernel_mod.F90

That kernel doesn't use the height fields, but W3 DoFs lie exactly halfway between Wtheta DoFs so I think these are mathematically equivalent.

Can you call that kernel instead of introducing this new one?

Thomas Bendall (tommbendall) · 2026-07-31T09:08:00Z

@@ -0,0 +1,547 @@
+!-----------------------------------------------------------------------------
+! (C) Crown copyright 2026 Met Office. All rights reserved.


Suggested change

! (C) Crown copyright 2026 Met Office. All rights reserved.

! (C) Crown copyright Met Office. All rights reserved.

I think we actually now shouldn't include the year in the copyright statement for new files:
https://metoffice.github.io/lfric_core/how_to_contribute/coding_standards/fortran_coding_standards.html#copyright

Thomas Bendall (tommbendall) · 2026-07-31T09:08:24Z

@@ -0,0 +1,309 @@
+!-----------------------------------------------------------------------------
+! (C) Crown copyright 2026 Met Office. All rights reserved.


Suggested change

! (C) Crown copyright 2026 Met Office. All rights reserved.

! (C) Crown copyright Met Office. All rights reserved.

I think we actually now shouldn't include the year in the copyright statement for new files:
https://metoffice.github.io/lfric_core/how_to_contribute/coding_standards/fortran_coding_standards.html#copyright

Thomas Bendall (tommbendall) · 2026-07-31T09:09:13Z

@@ -0,0 +1,302 @@
+!-----------------------------------------------------------------------------
+! (c) Crown copyright 2026 Met Office. All rights reserved.


Suggested change

! (c) Crown copyright 2026 Met Office. All rights reserved.

! (C) Crown copyright Met Office. All rights reserved.

I think we actually now shouldn't include the year in the copyright statement for new files:
https://metoffice.github.io/lfric_core/how_to_contribute/coding_standards/fortran_coding_standards.html#copyright

Thomas Bendall (tommbendall) · 2026-07-31T09:09:28Z

@@ -0,0 +1,239 @@
+!-----------------------------------------------------------------------------
+! (c) Crown copyright 2026 Met Office. All rights reserved.


Suggested change

! (c) Crown copyright 2026 Met Office. All rights reserved.

! (C) Crown copyright Met Office. All rights reserved.

I think we actually now shouldn't include the year in the copyright statement for new files:
https://metoffice.github.io/lfric_core/how_to_contribute/coding_standards/fortran_coding_standards.html#copyright

Thomas Bendall (tommbendall) · 2026-07-31T11:15:32Z

+      ! This invoke must be separated from the latter one.
+      ! because it makes wrong meta data of W2 space(ndf_w2, undf_w2, map_w2)
+      ! , passing W2H data as W2 data to a kernel.


Suggested change

! This invoke must be separated from the latter one.

! because it makes wrong meta data of W2 space(ndf_w2, undf_w2, map_w2)

! , passing W2H data as W2 data to a kernel.

! This invoke must be separated from the following one, to avoid

! confusing W2H and W2 metadata.

I'm just suggesting tweaking the wording of this comment, feel free to ignore

Thomas Bendall (tommbendall) · 2026-07-31T11:16:07Z

+      call invoke(                                                             &
+          setval_C(visc_m_w2h, 0.0_r_def),                                     &
+          setval_C(rho_in_w2h, 0.0_r_def),                                     &
+          interp_wtheta_to_w3_kernel_type(                                     &


I think this could use the sample_wtheta_to_w3_kernel from sci_sample_wtheta_to_w3_kernel_mod which does the same thing

Thomas Bendall (tommbendall) · 2026-07-31T13:57:38Z

+    type(arg_type) :: meta_args(12) = (/                                       &
+         arg_type(GH_FIELD,   GH_REAL,    GH_WRITE,  W2),                      &! u_inc
+         arg_type(GH_FIELD,   GH_REAL,    GH_READ,   W3, STENCIL(CROSS)),      &! uflux_w3
+         arg_type(GH_FIELD,   GH_REAL,    GH_READ,   W3, STENCIL(CROSS)),      &! vflux_w3
+         arg_type(GH_FIELD,   GH_REAL,    GH_READ,   W1),                      &! uflux_w1
+         arg_type(GH_FIELD,   GH_REAL,    GH_READ,   W1),                      &! vflux_w1
+         arg_type(GH_FIELD,   GH_REAL,    GH_READ,   ANY_SPACE_1),             &! wflux_sh_w2h
+         arg_type(GH_FIELD,   GH_REAL,    GH_READ,   W2),                      &! detj_at_w2
+         arg_type(GH_FIELD,   GH_REAL,    GH_READ,   Wtheta),                  &! rho_in_wth
+         arg_type(GH_FIELD,   GH_REAL,    GH_READ,   W2H),                     &! rho_in_w2h
+         arg_type(GH_FIELD,   GH_REAL,    GH_READ,   ANY_DISCONTINUOUS_SPACE_9,&
+                                                             STENCIL(CROSS)),  &! panel_id
+         arg_type(GH_FIELD,   GH_INTEGER, GH_READ,  ANY_DISCONTINUOUS_SPACE_3),&! face_selector_ew
+         arg_type(GH_FIELD,   GH_INTEGER, GH_READ,  ANY_DISCONTINUOUS_SPACE_3) &! face_selector_ns
+         /)


Suggested change

type(arg_type) :: meta_args(12) = (/ &

arg_type(GH_FIELD, GH_REAL, GH_WRITE, W2), &! u_inc

arg_type(GH_FIELD, GH_REAL, GH_READ, W3, STENCIL(CROSS)), &! uflux_w3

arg_type(GH_FIELD, GH_REAL, GH_READ, W3, STENCIL(CROSS)), &! vflux_w3

arg_type(GH_FIELD, GH_REAL, GH_READ, W1), &! uflux_w1

arg_type(GH_FIELD, GH_REAL, GH_READ, W1), &! vflux_w1

arg_type(GH_FIELD, GH_REAL, GH_READ, ANY_SPACE_1), &! wflux_sh_w2h

arg_type(GH_FIELD, GH_REAL, GH_READ, W2), &! detj_at_w2

arg_type(GH_FIELD, GH_REAL, GH_READ, Wtheta), &! rho_in_wth

arg_type(GH_FIELD, GH_REAL, GH_READ, W2H), &! rho_in_w2h

arg_type(GH_FIELD, GH_REAL, GH_READ, ANY_DISCONTINUOUS_SPACE_9,&

STENCIL(CROSS)), &! panel_id

arg_type(GH_FIELD, GH_INTEGER, GH_READ, ANY_DISCONTINUOUS_SPACE_3),&! face_selector_ew

arg_type(GH_FIELD, GH_INTEGER, GH_READ, ANY_DISCONTINUOUS_SPACE_3) &! face_selector_ns

/)

type(arg_type) :: meta_args(12) = (/ &

arg_type(GH_FIELD, GH_REAL, GH_WRITE, W2), & ! u_inc

arg_type(GH_FIELD, GH_REAL, GH_READ, W3, STENCIL(CROSS)), & ! uflux_w3

arg_type(GH_FIELD, GH_REAL, GH_READ, W3, STENCIL(CROSS)), & ! vflux_w3

arg_type(GH_FIELD, GH_REAL, GH_READ, W1), & ! uflux_w1

arg_type(GH_FIELD, GH_REAL, GH_READ, W1), & ! vflux_w1

arg_type(GH_FIELD, GH_REAL, GH_READ, ANY_SPACE_1), & ! wflux_sh_w2h

arg_type(GH_FIELD, GH_REAL, GH_READ, W2), & ! detj_at_w2

arg_type(GH_FIELD, GH_REAL, GH_READ, Wtheta), & ! rho_in_wth

arg_type(GH_FIELD, GH_REAL, GH_READ, W2H), & ! rho_in_w2h

arg_type(GH_FIELD, GH_REAL, GH_READ, ANY_DISCONTINUOUS_SPACE_9,&

STENCIL(CROSS)), & ! panel_id

arg_type(GH_FIELD, GH_INTEGER, GH_READ, ANY_DISCONTINUOUS_SPACE_3),& ! face_selector_ew

arg_type(GH_FIELD, GH_INTEGER, GH_READ, ANY_DISCONTINUOUS_SPACE_3) & ! face_selector_ns

/)

Sorry, same fussy suggestion about spaces and comments

Thomas Bendall (tommbendall) · 2026-07-31T13:58:18Z

+  use fs_continuity_mod,             only : Wtheta, W3, W2, W2H, W1
+  use kernel_mod,                    only : kernel_type
+  use sci_face_selector_support_mod, only : face_from_face_selector
+  use mixing_config_mod,             only : fullstress


Same comment as for the previous kernel, could this be passed through as an argument? There is a desire for the CCD team to avoid importing any config variables into kernels

Shusuke Nishimoto (mo-snishimoto) · 2026-08-05T12:57:32Z

Hi Thomas Bendall (@tommbendall), I really appreciate the careful review. I fixed my branch and answer your comments as follows:

Documentation: Thanks for the suggestion. I am happy to make .rst documentation in lfric_apps repository (And I have made a draft myself). However, if you don't mind, I would like to have a follow-up PR and make a whole documentation of conservative diffusion scheme with you in order to complete consistent and readable documentation.
Results: I added the result of one of the LFRic nesting run (run_lfric_atm_ral3_mixmol-seuk_MG) in the same pdf. I also replaced the result of warm bubble experiment because the results changed during the code fix.
Interpolation kernel: Thank you for the comment, and I apologize that I missed the kernel. I fixed the code. This change arose a difference comparable to a numerical error because the previous (duplicated) kernel used physical heights for interpolation. I updated checksum files and track.log in the PR summary with the latest ones.
Other comments: I fixed the source code following your suggestion. (I apologize if the Doxygen comment I added was not readable English).

Many thanks

Thomas Bendall (tommbendall)

Many thanks for making these changes and adding the extra plots to your results PDF.

We agreed a plan for documenting the diffusion code offline.

I just have a couple of extra suggestions, which are mainly to undo a mistake I made in a suggestion! Once that's done I'll be happy for this to proceed.

Thomas Bendall (tommbendall) · 2026-08-05T17:23:20Z

+!!          \f]
+!!
+!!          The terms proportional to the vertical gradient in the vertical
+!!          fluxes are ommited as they are calculated separetely


Suggested change

!! fluxes are ommited as they are calculated separetely

!! fluxes are omitted as they are calculated separately

Typo!

Thomas Bendall (tommbendall) · 2026-08-05T17:24:58Z

+!!          These fluxes for horizontal momentum are expressed as zonal and
+!!          meridional fluxes at both W3 and W1 points, creating a


Suggested change

!! These fluxes for horizontal momentum are expressed as zonal and

!! meridional fluxes at both W3 and W1 points, creating a

!! These fluxes for horizontal momentum are expressed as contra-

!! variant flux components at both W3 and W1 points, creating a

Sorry I've realised that they technically may not be zonal and meridional components, since these are the contravariant component to the mesh

Thomas Bendall (tommbendall) · 2026-08-05T17:25:32Z

+!> @brief Calculates divergence of horizontal diffusion fluxes for momentum
+!> @details The conservative form of the horizontal diffusion operator for
+!!          momentum involves taking the divergence of fluxes.
+!!          These fluxes are expressed as zonal and meridional fluxes at


Suggested change

!! These fluxes are expressed as zonal and meridional fluxes at

!! These fluxes are expressed as contravariant components at

Sorry I've realised that they technically may not be zonal and meridional components, since these are the contravariant component to the mesh

Shusuke Nishimoto (mo-snishimoto) · 2026-08-05T18:20:43Z

Hi Thomas Bendall (@tommbendall) , thank you for checking my change carefully and pointing out my typo! I reflected your comment to my branch.

Shusuke Nishimoto (mo-snishimoto) added 2 commits July 21, 2026 18:18

Implement conservative momentum diffusion & tensorial stress.

e48dad2

Update KGO.

91db659

Shusuke Nishimoto (mo-snishimoto) requested review from iboutle, James Bruten (james-bruten-mo) and thomasmelvin as code owners July 23, 2026 10:50

github-actions Bot assigned Shusuke Nishimoto (mo-snishimoto) Jul 23, 2026

github-actions Bot requested review from Thomas Bendall (tommbendall) July 23, 2026 10:51

github-actions Bot added the cla-signed The CLA has been signed as part of this PR - added by GA label Jul 23, 2026

Merge branch 'main' into fullstress2

18f1451

Shusuke Nishimoto (mo-snishimoto) added enhancement New feature or request KGO This PR contains changes to KGO macro This PR contains a metadata upgrade macro labels Jul 23, 2026

Shusuke Nishimoto (mo-snishimoto) added this to the Autumn 2026 milestone Jul 23, 2026

Shusuke Nishimoto (mo-snishimoto) mentioned this pull request Jul 23, 2026

Implementation of conservative momentum diffusion and tensorial stress option #479

Open

iboutle approved these changes Jul 30, 2026

View reviewed changes

Fix mistake in rose-meta.conf & Copyright.

1f187d5

Thomas Bendall (tommbendall) requested changes Jul 31, 2026

View reviewed changes

Sci/Tech review comment

d25b738

Shusuke Nishimoto (mo-snishimoto) force-pushed the fullstress2 branch 2 times, most recently from 9087f80 to d25b738 Compare August 5, 2026 10:33

Thomas Bendall (tommbendall) requested changes Aug 5, 2026

View reviewed changes

Sci/Tech review comment 2

2a86b4b

		@@ -0,0 +1,547 @@
		!-----------------------------------------------------------------------------
		! (C) Crown copyright 2026 Met Office. All rights reserved.

	! (C) Crown copyright 2026 Met Office. All rights reserved.
	! (C) Crown copyright Met Office. All rights reserved.

		@@ -0,0 +1,309 @@
		!-----------------------------------------------------------------------------
		! (C) Crown copyright 2026 Met Office. All rights reserved.

		@@ -0,0 +1,302 @@
		!-----------------------------------------------------------------------------
		! (c) Crown copyright 2026 Met Office. All rights reserved.

		@@ -0,0 +1,239 @@
		!-----------------------------------------------------------------------------
		! (c) Crown copyright 2026 Met Office. All rights reserved.

	!! fluxes are ommited as they are calculated separetely
	!! fluxes are omitted as they are calculated separately

		!! These fluxes for horizontal momentum are expressed as zonal and
		!! meridional fluxes at both W3 and W1 points, creating a

	!! These fluxes are expressed as zonal and meridional fluxes at
	!! These fluxes are expressed as contravariant components at

Uh oh!

Conversation

Shusuke Nishimoto (mo-snishimoto) commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

Code Quality Checklist

Testing

trac.log

Test Suite Results - lfric_apps - test_fullstress/run2

Suite Information

Task Information

Security Considerations

Performance Impact

AI Assistance and Attribution

Documentation

PSyclone Approval

Sci/Tech Review

Code Review

Uh oh!

iboutle left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Shusuke Nishimoto (mo-snishimoto) commented Jul 30, 2026

Uh oh!

Thomas Bendall (tommbendall) left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Shusuke Nishimoto (mo-snishimoto) commented Aug 5, 2026

Uh oh!

Thomas Bendall (tommbendall) left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Shusuke Nishimoto (mo-snishimoto) commented Aug 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Shusuke Nishimoto (mo-snishimoto) commented Jul 23, 2026 •

edited

Loading