Skip to content

Linear and adjoint boundary layer physics#182

Open
tom-j-h wants to merge 118 commits intoMetOffice:mainfrom
tom-j-h:tlad_boundary_layer
Open

Linear and adjoint boundary layer physics#182
tom-j-h wants to merge 118 commits intoMetOffice:mainfrom
tom-j-h:tlad_boundary_layer

Conversation

@tom-j-h
Copy link
Contributor

@tom-j-h tom-j-h commented Jan 26, 2026

PR Summary

Sci/Tech Reviewer: @cjohnson-pi
Code Reviewer: @mo-marqh

This development is described on an old trac ticket: https://code.metoffice.gov.uk/trac/lfric_apps/ticket/682. This includes many links to the details of the scheme, as well as plots of results. In particular, https://wwwspice/~tim.payne/docs/4D-Var/PF/TL_BL_2025/TL_BL_2025.html has links to relevant VSDP documents.

Development was done by Tim Payne - I am simply taking the old fcm branch and creating this GitHub PR.

PLEASE NOTE - this is a follow-on to #163. The branch was created from #163's branch in my fork, but I can't make a PR into that branch because then I would be stuck in my fork. So, to look at the actual changes relevant to this PR alone, look at the diff of this branch with #161's branch: tom-j-h/lfric_apps@jelf_C224_adjoint_tests...tom-j-h:lfric_apps:tlad_boundary_layer

PLEASE ALSO NOTE - #163 is blocked by #161 which is blocked by #156 which relies on MetOffice/lfric_core#227, so when testing, I used this Core branch.

The change includes new files to be added to BIG_DATA_DIR - see paths to files in /data/users/tim.payne/ in changes. Essentially it is just:
/data/users/tim.payne/lfric_apps/files/final_ls_with_land.nc (on SPICE)
/data/users/tim.payne/lfric_apps/files/final_2021060200-2021060207.nc (on EX, for weekly C224 test)

Also need KGO update for this weekly test: https://cylchub/services/cylc-review/taskjobs/tom.hill/?suite=tlad_boundary_layer-linear_model_ex1a_weekly%2Frun3

Test branch: https://github.com/tom-j-h/lfric_apps/tree/tlad_boundary_layer-test

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 Core rose-stem suite
  • If required (e.g. API changes) I have also run the LFRic Apps test suite using this branch
  • 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 - tlad_boundary_layer-developer/run20

Suite Information

Item Value
Suite Name tlad_boundary_layer-developer/run20
Suite User tom.hill
Workflow Start 2026-01-29T19:22:52
Groups Run developer
Dependency Reference Main Like
casim MetOffice/casim@2025.12.1 True
jules MetOffice/jules@2025.12.1 True
lfric_apps tom-j-h/lfric_apps@tlad_boundary_layer-test False
lfric_core MetOffice/lfric_core@bbb3d8a True
moci MetOffice/moci@2025.12.1 True
SimSys_Scripts MetOffice/SimSys_Scripts@2025.12.1 True
socrates MetOffice/socrates@2025.12.1 True
socrates-spectral MetOffice/socrates-spectral@2025.12.1 True
ukca MetOffice/ukca@2025.12.1 True

Task Information

❌ failed tasks - 12
Task State
check_jedi_lfric_tests_nwp_gal9-C12_MG_azspice_gnu_fast-debug-64bit failed
check_jedi_lfric_tests_nwp_gal9-C12_MG_ex1a_cce_fast-debug-64bit failed
check_jedi_lfric_tests_runge-kutta-C12_azspice_gnu_fast-debug-64bit failed
check_jedi_lfric_tests_runge-kutta-C12_azspice_gnu_full-debug-64bit failed
check_jedi_lfric_tests_runge-kutta-C12_ex1a_cce_fast-debug-64bit failed
check_jedi_lfric_tests_tlm_forecast_tl_nwp_gal9-C12_MG_azspice_gnu_fast-debug-64bit failed
check_jedi_lfric_tests_tlm_forecast_tl_nwp_gal9-C12_MG_ex1a_cce_fast-debug-64bit failed
check_linear_model_nwp_gal9-C12_MG_azspice_gnu_fast-debug-64bit failed
check_linear_model_nwp_gal9-C12_MG_ex1a_gnu_fast-debug-64bit failed
check_linear_model_nwp_gal9_random-C12_MG_azspice_gnu_fast-debug-64bit failed
check_linear_model_nwp_gal9_random-C12_MG_ex1a_gnu_fast-debug-64bit failed
kgo_groups_checker failed
✅ succeeded tasks - 1099
⌛ 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

tom-j-h and others added 30 commits January 7, 2026 10:23
…j-h/lfric_apps into align_adjoint_tests_to_linear_model
@github-actions github-actions bot added cla-modified The CLA has been modified as part of this PR - added by GA and removed cla-signed The CLA has been signed as part of this PR - added by GA labels Jan 29, 2026
Copy link

@ss421 ss421 left a comment

Choose a reason for hiding this comment

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

The main thing I noticed is the update to interfaces/jedi_lfric_interface/source/field/atlas_field_interface_mod.F90 that should not be required. I think there might need to be a small modification to the way that the LS fields are created. Details in the comments...

Ive also added some minor comments about configuration updates.

These changes are small and should not prevent the code being moved to CR but I would like to see them added before commit.

@tom-j-h
Copy link
Contributor Author

tom-j-h commented Jan 29, 2026

@cjohnson-pi I believe I have addressed everything (we discussed some of it on Teams/email).

I have left state_star as an array of fields of bundle_size, even though only the u field is used, as I assume the others may be used by future physics schemes. But only the u field is passed to tl_bdy_lyr_alg, as requested

I checked that linear_model outputs did not change as a result of the significant refactoring, to make sure no mistakes were made.

Running developer suite again now and will attach new trac.log when complete.
EDIT - now done, successful apart from KGOs that need changing.

@cjohnson-pi
Copy link
Contributor

Science review 3

Essential:

  • I have checked the rose gui. All is good except for the position of l_boundary_layer which should be moved to the top of the panel. (I think you can do this by adding sort-key=Panel-A00 to the l_boundary_layer item - see namelist:boundaries and limited_area in gungho for an example).
  • Please ensure that the C224 test in the linear_model_ex1a weekly test works. (I suspect that it wont because there is no land_fraction in the C224 input files – so l_boundary_layer will need to be set to false).

Non-essential:

  • Not required for science review – but please try to document or find scripts for the creation of the input files and setup an issue for adding land_fraction to the C224 linear_model test.
  • Note: the versions.py should introduce the boundary_layer settings to all the required configurations when the upgrade_macro is applied. So you shouldn’t need to add them to each configuration separately.
  • Comment: I have checked the linear_model plots associated with the kgo changes. We discussed on teams why the runge-kutta jedi tests have failed (the reason being they are resulting from changes in Align jedi_lfric_tests linear model/adjoint testing to adjoint_tests and linear_model #156, on which this branch is dependent).

@tom-j-h
Copy link
Contributor Author

tom-j-h commented Jan 30, 2026

  • I have checked the rose gui. All is good except for the position of l_boundary_layer which should be moved to the top of the panel. (I think you can do this by adding sort-key=Panel-A00 to the l_boundary_layer item - see namelist:boundaries and limited_area in gungho for an example).

Thanks, done and checked via GUI.

  • Please ensure that the C224 test in the linear_model_ex1a weekly test works. (I suspect that it wont because there is no land_fraction in the C224 input files – so l_boundary_layer will need to be set to false).

Have modified C224 opt config for this. KGO will still change since l_stabilise_bl is now .false., though (and as planned we will remove this option) - running test now: https://cylchub/services/cylc-review/taskjobs/tom.hill/?suite=tlad_boundary_layer-linear_model_ex1a_weekly%2Frun1

(On second thoughts, the C224 run might not be okay without some kind of stabiliser; running also with old stabiliser at https://cylchub/services/cylc-review/taskjobs/tom.hill/?suite=tlad_boundary_layer-linear_model_ex1a_weekly%2Frun2).
EDIT turns out the updated C224 file is available, running again and will revert changes if it's okay: https://cylchub/services/cylc-review/taskjobs/tom.hill/?suite=tlad_boundary_layer-linear_model_ex1a_weekly%2Frun3

  • Not required for science review – but please try to document or find scripts for the creation of the input files and setup an issue for adding land_fraction to the C224 linear_model test.

Waiting on this from Tim.
EDIT here is what Tim has provided. I'm not sure where the best place to put this information is:

# Extend netcdf file containing hourly LS (ie at seven times in window) to also contain land_area_fraction at hourly intervals
# Example here is for C224 - other resolutions similar

cd $SCRATCH


# Copy LS file with hourly fields at C224
cp /data/users/lfricadmin/data/tangent-linear/Ticket735/final_2021060200-2021060207.nc . &

# copy file containing land_area_fraction also at C224
cp /data/users/lfricadmin/data/ancils/basic-gal/yak/C224/land_sea_mask/etop01/qrparm.landfrac.nc .

# extract land_area_fraction into new file out.nc containing just land_area_fraction
ncks -v land_area_fraction qrparm.landfrac.nc out.nc

# concatenate this field 7 times to create new file containing 7 copies of land_area_fraction
ncecat out.nc out.nc out.nc out.nc out.nc out.nc out.nc out7.nc

# replace  record,dim0 with time, nMesh2d_face
ncrename -d record,time -d dim0,nMesh2d_face out7.nc out7a.nc

# add line 'land_area_fraction:interval_operation = "1 h" ;'
ncatted -a interval_operation,land_area_fraction,a,c,"1 h" out7a.nc out7b.nc

# add line 'land_area_fraction:interval_write = "1 h" ;'
ncatted -a interval_write,land_area_fraction,a,c,"1 h" out7b.nc out7c.nc

# change 'land_area_fraction:online_operation = "once"' to 'land_area_fraction:online_operation = "instant"'
ncatted -a online_operation,land_area_fraction,m,c,"instant" out7c.nc

# write out7c.nc to asci file out7.cdl for hand editing of variable names and header
ncdump out7c.nc > out7.cdl

# hand edit out7.cdl : 
# in "variables:" section change dimension of land_area_fraction from (time, nmesh2d_face) to  (time, one, nmesh2d_face); 
# in " dimensions:" section add line "one = 1;" 

# regenerate netcdf file from out7.cdl
ncgen -o out.7d.nc out7.cdl

# extend final_2021060200-2021060207.nc by adding land_area_fraction out.7d.nc to the end - creates new final_2021060200-2021060207.nc
ncks -A out7d.nc final_2021060200-2021060207.nc

  • Note: the versions.py should introduce the boundary_layer settings to all the required configurations when the upgrade_macro is applied. So you shouldn’t need to add them to each configuration separately.

Okay, thanks. Still haven't got to grips with Rose to be quite honest. There is a test branch linked above where apply_macros.py was run.

@ss421
Copy link

ss421 commented Jan 30, 2026

This requires a small JEDI update. @tom-j-h will provide this. Captured in the next release PR here: https://github.com/JCSDA-internal/lfric-jedi/pull/1206

@cjohnson-pi
Copy link
Contributor

Thanks for responding to my many science review requests. I am very happy to approve and for this to go to code review.

@tom-j-h tom-j-h requested a review from mo-marqh January 30, 2026 13:10
@tom-j-h
Copy link
Contributor Author

tom-j-h commented Feb 2, 2026

@cjohnson-pi the bot keeps moving this back to Sci Tech Review in the tracker. I think you'll need to give an approving review via GitHub (Files changed -> Submit review -> Approve).

@mo-marqh, please be aware that this approval was given before the deadline on Friday!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-modified The CLA has been modified as part of this PR - added by GA KGO This PR contains changes to KGO macro This PR contains a metadata upgrade macro

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Linear and adjoint boundary layer physics scheme

4 participants