Native wind transport - #699
Conversation
github-actions
Bot
added
the
cla-modified
| __str__ = __repr__ | ||
|
|
||
|
|
||
| class vn32_t699(MacroUpgrade): |
There was a problem hiding this comment.
Hi Tom,
This upgrade macro looks good, but it should go into science/gungho/rose-meta/lfric-gungho/versions.py rather than here
iboutle
left a comment
iboutle
left a comment
There was a problem hiding this comment.
Some minor comments - mainly suggesting adding the native wind transport as default to idealised Cartesian domains. Presume there are also some new KGOs that need adding at some point?
DrTVockerodtMO
left a comment
DrTVockerodtMO
left a comment
There was a problem hiding this comment.
Adjoint config changes look good to me, thanks!
I've made the changes and new KGO's have been added to the PR |
thomasmelvin
added
KGO
There was a problem hiding this comment.
Pull request overview
Implements native wind transport (keeping winds in native W2 space) for regional spherical/planar domains, including the additional SL-based transport kernels, metric application, configuration controls, and associated test/KGO updates.
Changes:
- Adds new SL W2H kernels and W2 split/combine/metrics kernels to support native W2 wind transport.
- Updates SL vertical coefficient generation and wind-transport control flow to enable/validate the new option.
- Extends Rose metadata/configs, rose-stem task configurations, and updates KGOs/unit tests accordingly.
Reviewed changes
Copilot reviewed 59 out of 59 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| science/gungho/unit-test/kernel/transport/sl/horizontal_linear_sl_w2h_kernel_mod_test.pf | Adds unit test coverage for the new linear SL W2H kernel. |
| science/gungho/unit-test/kernel/transport/sl/horizontal_cubic_sl_w2h_kernel_mod_test.pf | Adds unit test coverage for the new cubic SL W2H kernel. |
| science/gungho/unit-test/kernel/transport/sl/compute_vertical_quintic_coef_kernel_mod_test.pf | Updates vertical quintic coefficient unit test for new stencil args. |
| science/gungho/unit-test/kernel/transport/sl/compute_vertical_linear_coef_kernel_mod_test.pf | Updates vertical linear coefficient unit test for new stencil args. |
| science/gungho/unit-test/kernel/transport/sl/compute_vertical_cubic_coef_kernel_mod_test.pf | Updates vertical cubic coefficient unit test for new stencil args. |
| science/gungho/unit-test/kernel/transport/mol/polyv_wtheta_koren_kernel_mod_test.pf | Updates transport config setup to include new native_w2_wind_transport flag. |
| science/gungho/unit-test/kernel/transport/mol/poly1d_vert_adv_coeffs_kernel_mod_test.pf | Updates transport config setup to include new native_w2_wind_transport flag. |
| science/gungho/unit-test/kernel/transport/ffsl/ffsl_reconstruction_xy_kernel_mod_test.pf | Adds a new FFSL reconstruction XY kernel unit test. |
| science/gungho/unit-test/kernel/transport/common/transport_metadata_collection_test.pf | Updates metadata collection tests to include new native_w2_wind_transport flag. |
| science/gungho/unit-test/kernel/transport/common/split_w2_to_ws_kernel_mod_test.pf | Adds unit test for splitting W2 winds into W2h/Wtheta components. |
| science/gungho/unit-test/kernel/transport/common/combine_ws_to_w2_kernel_mod_test.pf | Adds unit test for recombining W2h/Wtheta winds into W2. |
| science/gungho/source/kernel/transport/sl/vertical_quintic_sl_kernel_mod.F90 | Extends vertical quintic SL kernel to support additional spaces/paths. |
| science/gungho/source/kernel/transport/sl/vertical_cubic_sl_kernel_mod.F90 | Extends vertical cubic SL kernel to support additional spaces/paths. |
| science/gungho/source/kernel/transport/sl/horizontal_linear_sl_w2h_kernel_mod.F90 | Adds new horizontal linear SL kernel specialized for W2H fields. |
| science/gungho/source/kernel/transport/sl/horizontal_cubic_sl_w2h_kernel_mod.F90 | Adds new horizontal cubic SL kernel specialized for W2H fields. |
| science/gungho/source/kernel/transport/sl/compute_vertical_quintic_coef_kernel_mod.F90 | Updates vertical quintic coefficient kernel to accept stencils and W2h handling. |
| science/gungho/source/kernel/transport/sl/compute_vertical_linear_coef_kernel_mod.F90 | Updates vertical linear coefficient kernel to accept stencils and W2h handling. |
| science/gungho/source/kernel/transport/sl/compute_vertical_cubic_coef_kernel_mod.F90 | Updates vertical cubic coefficient kernel to accept stencils and W2h handling. |
| science/gungho/source/kernel/transport/common/split_w2_to_ws_kernel_mod.F90 | Adds kernel to split W2 winds into native horizontal/vertical components. |
| science/gungho/source/kernel/transport/common/combine_ws_to_w2_kernel_mod.F90 | Adds kernel to combine native components back into W2. |
| science/gungho/source/kernel/transport/common/apply_wind_metrics_kernel_mod.F90 | Adds kernel to apply metric/rotation/slope corrections to wind increments. |
| science/gungho/source/configuration/check_configuration_mod.F90 | Adds configuration checks for native_w2_wind_transport and adjusts related validation. |
| science/gungho/source/algorithm/transport/sl/horizontal_sl_advective_alg_mod.x90 | Routes W2H fields through new SL W2H kernels in horizontal advective transport. |
| science/gungho/source/algorithm/transport/sl/compute_sl_coefficients_alg_mod.x90 | Updates coefficient computation invokes/signatures (incl. stencil depth). |
| science/gungho/source/algorithm/transport/control/wind_transport_alg_mod.x90 | Implements native W2 wind transport control path and spherical metric handling. |
| science/gungho/source/algorithm/transport/common/wind_precomputations_mod.x90 | Extends precomputations storage/access for W2H vertical SL coefficients/indices. |
| science/gungho/rose-meta/lfric-gungho/versions.py | Adds upgrade macro to introduce native_w2_wind_transport setting. |
| science/gungho/rose-meta/lfric-gungho/HEAD/rose-meta.conf | Adds Rose metadata entry for native_w2_wind_transport and validation. |
| rose-stem/site/meto/kgos/transport/ex1a/checksum_transport_hadley_dcmip_mol_alt-C32_ex1a_cce_fast-debug-64bit.txt | Updates KGO checksum for transport Hadley DCMIP MOL alt (ex1a). |
| rose-stem/site/meto/kgos/transport/azspice/checksum_transport_hadley_dcmip_mol_alt-C32_azspice_gnu_fast-debug-64bit.txt | Updates KGO checksum for transport Hadley DCMIP MOL alt (azspice). |
| rose-stem/site/meto/kgos/lfric_atm/ex1a/checksum_lfric_atm_rce-BiP64x64-1500x1500_MG_ex1a_cce_fast-debug-32bit.txt | Updates KGO checksum for lfric_atm RCE (ex1a). |
| rose-stem/site/meto/kgos/lfric_atm/ex1a/checksum_lfric_atm_ral3-seuk_MG_ex1a_cce_fast-debug-32bit.txt | Updates KGO checksum for lfric_atm RAL3 seuk (ex1a). |
| rose-stem/site/meto/kgos/lfric_atm/ex1a/checksum_lfric_atm_ral3_ens-seuk_MG_ex1a_cce_fast-debug-32bit.txt | Updates KGO checksum for lfric_atm RAL3 ensemble seuk (ex1a). |
| rose-stem/site/meto/kgos/lfric_atm/azspice/checksum_lfric_atm_rce-BiP64x64-1500x1500_MG_azspice_gnu_fast-debug-32bit.txt | Updates KGO checksum for lfric_atm RCE (azspice). |
| rose-stem/site/meto/kgos/lfric_atm/azspice/checksum_lfric_atm_ral3-seuk_MG_azspice_gnu_fast-debug-32bit.txt | Updates KGO checksum for lfric_atm RAL3 seuk (azspice). |
| rose-stem/site/meto/kgos/lfric_atm/azspice/checksum_lfric_atm_ral3_ens-seuk_MG_azspice_gnu_fast-debug-32bit.txt | Updates KGO checksum for lfric_atm RAL3 ensemble seuk (azspice). |
| rose-stem/site/meto/kgos/gungho_model/ex1a/checksum_gungho_model_straka_200m-alt1-BiP256x4-200x200_ex1a_gnu_fast-debug-64bit.txt | Updates KGO checksum for gungho_model Straka 200m (ex1a). |
| rose-stem/site/meto/kgos/gungho_model/ex1a/checksum_gungho_model_baroclinic-alt1-C24s_MG_ex1a_gnu_fast-debug-64bit.txt | Updates KGO checksum for gungho_model baroclinic wave (ex1a). |
| rose-stem/site/meto/kgos/gungho_model/azspice/checksum_gungho_model_straka_200m-alt1-BiP256x4-200x200_azspice_gnu_fast-debug-64bit.txt | Updates KGO checksum for gungho_model Straka 200m (azspice). |
| rose-stem/site/meto/kgos/gungho_model/azspice/checksum_gungho_model_baroclinic-alt1-C24s_MG_azspice_gnu_fast-debug-64bit.txt | Updates KGO checksum for gungho_model baroclinic wave (azspice). |
| rose-stem/site/meto/groups/groups_gungho_model.cylc | Adds native-wind variants of gungho_model cartesian tasks to groups. |
| rose-stem/site/common/lfric_atm/tasks_lfric_atm.cylc | Enables native_wind_transport opt config in selected lfric_atm tasks. |
| rose-stem/site/common/gungho_model/tasks_gungho_model_cartesian.cylc | Adds new cartesian task configurations enabling native wind transport. |
| rose-stem/app/lfric_atm/opt/rose-app-native_wind_transport.conf | Adds opt config turning on native_w2_wind_transport for lfric_atm rose-stem. |
| rose-stem/app/gungho_model/rose-app.conf | Adjusts default gungho_model vertical_method configuration. |
| rose-stem/app/gungho_model/opt/rose-app-native_wind_transport.conf | Adds opt config turning on native_w2_wind_transport for gungho_model rose-stem. |
| applications/transport/example/configuration.nml | Adds native_w2_wind_transport default setting to transport example. |
| applications/shallow_water/example/configuration.nml | Adds native_w2_wind_transport default setting to shallow water example. |
| applications/ngarch/example/configuration_casim.nml | Adds native_w2_wind_transport default setting to NGARCH (casim) example. |
| applications/ngarch/example/configuration_bl.nml | Adds native_w2_wind_transport default setting to NGARCH (bl) example. |
| applications/name_transport/example/configuration.nml | Adds native_w2_wind_transport default setting to NAME transport example. |
| applications/linear_model/example/configuration.nml | Adds native_w2_wind_transport default setting to linear model example. |
| applications/lfric_coupled/example/configuration.nml | Adds native_w2_wind_transport default setting to coupled example. |
| applications/lfric_atm/example/configuration.nml | Adds native_w2_wind_transport default setting to lfric_atm example. |
| applications/jules/example/configuration.nml | Adds native_w2_wind_transport default setting to jules example. |
| applications/jedi_lfric_tests/example/configuration.nml | Adds native_w2_wind_transport default setting to jedi tests example. |
| applications/gungho_model/example/configuration.nml | Adds native_w2_wind_transport default setting to gungho_model example. |
| applications/adjoint_tests/example/configuration.nml | Adds native_w2_wind_transport default setting to adjoint tests example. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| ! Extract departure distances and physical heights | ||
| displacement(:) = dep_dist_z(map_w2v(1) : map_w2v(1)+nlayers) | ||
| int_disp(:) = INT(displacement(:), i_def) | ||
| frac_dist(:) = ABS(displacement(:) - REAL(int_disp(:), r_tran)) | ||
| sign_offset(:) = 0.5_r_tran*(1.0_r_tran + SIGN(1.0_r_tran, displacement(:))) | ||
| z_arr(:) = theta_height(map_wt(1) : map_wt(1)+nlayers) |
There was a problem hiding this comment.
This has been done, the duplicated code has been removed. Since none of the native wind tests use the quintic interpolation (and it was already correct just duplicated before) then there are no changes to answers
| select case( ndf_wf ) | ||
| case ( 1 ) ! W3 | ||
| nl = nlayers - 1 | ||
| ndofs = 1 | ||
| case ( 2 ) ! Wtheta | ||
| nl = nlayers | ||
| ndofs = 1 | ||
| case ( 4 ) ! W2h | ||
| nl = nlayers - 1 | ||
| ndofs = 2 | ||
| case default | ||
| call log_event('Invalid ndf in vertical_cubic_sl_kernel', LOG_LEVEL_ERROR) | ||
| end select |
There was a problem hiding this comment.
The code is correct and the comment was misleading, I've updated the comment to match the code
| select case( ndf_wf ) | ||
| case ( 1 ) ! W3 | ||
| nl = nlayers - 1 | ||
| ndofs = 1 | ||
| case ( 2 ) ! Wtheta | ||
| nl = nlayers | ||
| ndofs = 1 | ||
| case ( 4 ) ! W2h | ||
| nl = nlayers - 1 | ||
| ndofs = 2 | ||
| case default | ||
| call log_event('Invalid ndf in vertical_cubic_sl_kernel', LOG_LEVEL_ERROR) | ||
| end select |
There was a problem hiding this comment.
Again, the code is correct and the comment was misleading, I've updated the comment to match the code
| dofs2check = (/ 1, 10, 19, 28 /) | ||
| answer = (/ 1.0_r_tran, 10.0_r_tran, 1.9_r_tran, 28.0_r_tran /) | ||
| do i = 1,4 | ||
| @assertEqual(real(dofs2check(i), r_tran), field_x(dofs2check(i)), use_tol) | ||
| end do | ||
| ! increment_y = field_x(north-1) - field_x(centre) = +7 for all dofs | ||
| do i = 1,4 | ||
| @assertEqual(real(dofs2check(i), r_tran), field_y(dofs2check(i)), use_tol) | ||
| end do |
There was a problem hiding this comment.
The test should correctly compare increment_x and increment_y, this has now been corrected and the answers updated
| [namelist:transport=native_w2_wind_transport] | ||
| compulsory=true | ||
| description=Transport winds in the native W2 space. | ||
| fail-if=this == true and namelist:transport=horizontal_method != 3 and namelist:transport=vertical_method != 3 |
| call log_event( & | ||
| '3D unity transport can only be used when all variables ' & | ||
| // 'are using FFSL for vertical and horizontal transport', LOG_LEVEL_ERROR) | ||
| // 'are using FFSL for vertical and horizontal transport', LOG_LEVEL_WARNING) |
There was a problem hiding this comment.
The existing error flag is an overkill. 3D unity only requires that if ffsl is used for a variable then both the horizontal and vertical scheme for that variable are ffsl not that every field must use ffsl. I've updated the test to throw an error if a scheme uses ffsl in 1 direction but not the other
|
This will need a linked PR in lfric-jedi. I will link it here once it's ready |
DanStoneMO
added
the
Linked Jedi
James Kent (jameskent-metoffice)
left a comment
James Kent (jameskent-metoffice)
left a comment
There was a problem hiding this comment.
This ticket gets the native wind transport working with the SL scheme. Tests have been added to the suite to test this functionality, and it has also been applied to the RAL tests. There is an increase in cost to using this method, but it has been shown to be needed as it greatly improves the accuracy.
I'm pleasantly surprised that many of the SL/transport algorithms work for W2H fields without needing any modifications! For those that have changed or been added, the science looks correct (see comments below). The test results all look good. I have a few comments, mostly they are typos or comments in the code etc., but the ones that need fixing are:
- I think you are missing a minus sign in the rotation matrix for the shallow approximation.
- I don't think ffsl_reconstruction_xy_kernel is ever used, and so can be deleted.
| ! Transport the winds on their native W2 points | ||
| wt_fs => function_space_collection%get_fs( mesh, k_h, k_v, Wtheta ) | ||
| w2h_fs => function_space_collection%get_fs( mesh, k_h, k_v, W2h ) | ||
| call u_w3_n(1)%initialise( w2h_fs ) |
There was a problem hiding this comment.
I find it confusing when the variable is named u_w3 but it isn't actually u on W3. Could these be renamed u_w2h and u_w2v (or u_wt)? I'll leave this up to the developer to decide
There was a problem hiding this comment.
I've changed these to uv_w2h_ and w_wt_
| call chi2_d_w2%initialise( w2_fs ) | ||
|
|
||
| if ( geometry == geometry_spherical ) then | ||
| ! For the metric terms we need to transport the (horizontal) |
There was a problem hiding this comment.
Align comment with line below
| if ( geometry == geometry_spherical ) then | ||
| ! For the metric terms we need to transport the (horizontal) | ||
| ! spherical coordinate field to get the values at the departure points | ||
| ! Transport lambda & phi for lat-lon and alpha & beta for cubed sphere |
There was a problem hiding this comment.
Maybe note here that it isn't set up for the cubed sphere yet
| call invoke( setval_c( chi1_d_w2, 0.0_r_def ) , & | ||
| setval_c( chi2_d_w2, 0.0_r_def ), & | ||
| sample_field_kernel_type( chi1_d_w2, rmultiplicity_w2, chi1_d), & | ||
| sample_field_kernel_type( chi2_d_w2, rmultiplicity_w2, chi2_d) ) |
There was a problem hiding this comment.
align & in this invoke
| call log_field_minmax( LOG_LEVEL_TRACE, 'ydot', u_w3_n(2) ) | ||
| call log_field_minmax( LOG_LEVEL_TRACE, 'zdot', u_w3_n(3) ) | ||
|
|
||
|
|
There was a problem hiding this comment.
remove empty line
| @@ -0,0 +1,119 @@ | |||
| !----------------------------------------------------------------------------- | |||
| ! (c) Crown copyright 2025 Met Office. All rights reserved. | |||
There was a problem hiding this comment.
2026 date
| @@ -0,0 +1,135 @@ | |||
| !----------------------------------------------------------------------------- | |||
| ! (c) Crown copyright 2025 Met Office. All rights reserved. | |||
There was a problem hiding this comment.
2026 date
| @@ -0,0 +1,277 @@ | |||
| !----------------------------------------------------------------------------- | |||
There was a problem hiding this comment.
is this unit test needed if the corresponding kernel is never used?
There was a problem hiding this comment.
I've removed this and the kernel
| @@ -0,0 +1,192 @@ | |||
| !----------------------------------------------------------------------------- | |||
| ! (c) Crown copyright 2025 Met Office. All rights reserved. | |||
There was a problem hiding this comment.
2026 date
| @@ -0,0 +1,161 @@ | |||
| !----------------------------------------------------------------------------- | |||
| ! (c) Crown copyright 2025 Met Office. All rights reserved. | |||
There was a problem hiding this comment.
2026 date
DanStoneMO
left a comment
DanStoneMO
left a comment
There was a problem hiding this comment.
JEDI PR now live at: https://github.com/MetOffice/lfric-jedi/pull/1328
You're right on both these points and they have been implemented. During this change I realised that we are running the regional tests with The latest change locally hardwires the use of the deep atmosphere rotation matrix until such time as we have a full shallow atmosphere approximation. |
|
Your CLA signature was found on the base branch, but you appear to have modified the CONTRIBUTORS.md file in this PR. Please do not edit the CONTRIBUTORS.md file. If you have already signed the CLA, revert changes to the file and your signature will be picked up. |
James Kent (jameskent-metoffice)
left a comment
James Kent (jameskent-metoffice)
left a comment
There was a problem hiding this comment.
Thanks for making those changes, and your explanation about the shallow/deep rotation matrix makes sense. I'm happy this passes science review.
PR Summary
Sci/Tech Reviewer: James Kent (@jameskent-metoffice)
Code Reviewer: Mike Hobson (@mike-hobson)
Implement native wind transport for regional (spherical & planar) domains
There is a brief overview of the scheme in Native Wind Transport in LFRic.pdf along with some result from some example gungho & lfric_atm tests using the native wind transport along with results from (offline) rce tests showing ht ereduction in noise and improvement in spectra from this change.
Additionally, due to the changes in the vertical SL scheme two tests have changed KGO's even though they do not use the native wind branch, however as can be seen there is no practical difference in the results from these tests
Baroclinic wave alt1 - Trunk
Baroclinic wave alt1 - Branch
Straka 200m alt1 - Trunk
Straka 200m alt1 - Branch
Scheme Details
Cell centre transport
The current advecitve form transport scheme for the wind fields averages and projects the W2 wind components from the their native space (fluxes normal to cell faces) to be the pointwise cell centre components in a global Cartesian X,Y,Z coordinate scheme. This has two principal impacts
The Cartesian components are then transported using the existing transport scheme for a W3 variable (in general advective FFSL) and the transport increments are then projected back from the W3 space into W2 in the native components (which again will involve averaging). One advantage of this scheme is that due to the use of the Cartesian coordinate space the basis vectors are invariant across the domain and so there is no need to apply any metric transformations.
Native (cell face) transport
The new transport scheme holds the wind components in their native W2 space and transports them separately in this space. This means that there is no averaging of the wind components and so the transport scheme can see (and damp) any grid scale variation. After the components are transported and the increments are obtained a rotation matrix is applied to pick up the metric transformations for spherical geometry. To avoid the complexity of having to compute fluxes and mass reconstructions on shifted meshes the native wind transport requires that the Semi-Lagrangian (SL) scheme is used for the wind transport instead of the default FFSL scheme.
The algorithm is:
Restrictions & future development
Linked PRs
Code Quality Checklist
Testing
This change (at an earlier revision) has been extensively tested in both idealised (RCE/CBL) experiments and as part of the RAL3 development
trac.log
Test Suite Results - lfric_apps - native_w2_transport/run3
Suite Information
Task Information
❌ failed tasks - 13
⌛ waiting tasks - 2
Security Considerations
Performance Impact
The code adds a significant cost to the transport for spherical domains when turned on, the times below are for the UKv rose stem test
The missing increase in time is likely to be due to the need to now compute the SL coefficients (which are not separately timed). Future work will look at further optimising this code focussing on the following areas
and it is likely that significant savings can be made
AI Assistance and Attribution
Github copilot was used to correct doxygen comments and prototype unit tests
Documentation
PSyclone Approval
Sci/Tech Review
(Please alert the code reviewer via a tag when you have approved the SR)
Code Review