Skip to content

Enable jedi_lfric to run lfric_atm forecast configurations - #512

Merged
James Bruten (james-bruten-mo) merged 44 commits into
MetOffice:mainfrom
ss421:update_forecast_config_ifdefphys
Jul 21, 2026
Merged

Enable jedi_lfric to run lfric_atm forecast configurations#512
James Bruten (james-bruten-mo) merged 44 commits into
MetOffice:mainfrom
ss421:update_forecast_config_ifdefphys

Conversation

@ss421

@ss421 Steven Sandbach (ss421) commented May 20, 2026

Copy link
Copy Markdown
Contributor

PR Summary

Sci/Tech Reviewer: Matt Shin (@matthewrmshin)
Code Reviewer: James Bruten (@james-bruten-mo)

This update requires the changes applied in: #502. This branch Update forecast config ifdefphys was created from the branch in that PR: cjohnson-pi/ifdefphys. The change being applied in this PR can be seen via: cjohnson-pi/lfric_apps@ifdefphys...ss421:lfric_apps:update_forecast_config_ifdefphys

The dependent PR #502 is now merged and the branch has been brought up-to the head of main (as of Wed 24/06) and retested via lfric-jedi with PR's included for the update as detailed below.

This PR updates jedi_lfric_tests to enable running of model forecast based on lfric_atm configurations. This requires setting UM_PHYSICS to true. The update largely follows the lfric_atm build configurations. I did not:

  1. Add the optimization scripts as it is not yet clear how this should be done. Some follow up work is required to better understand this.
  2. Update the build flags FFLAG_GROUPS because I encountered build issues that may be linked to Adjoint code which is included in the jedi_lfric library.

A small bug was fixed (removal of clock double tick).

Three new lfric_atm based configurations have been added: nwp_gal9-C12, nwp_gal9-C12_da and nwp_gal9-C48_MG. The existing gh-si-for-linear-C12 was removed as it was only a placeholder configuration and no longer required.

Note: The C48 test requires a new input file reference directly at: /home/users/darth/srv/mobb-data/lfric-jedi/lfric-diag/lfric_bg_mi-be607_C48_20210602_la354.nc currently and used in JEDI testing. Documentation on the file is included in: https://github.com/MetOffice/jjdocs/blob/38d5cfad6fd30cbea67e825775c5a20798490430/docs/Interfaces/ModelInterfaces/Input-Data/lfric-jedi-dir.rst?plain=1#L169

The applications/jedi_lfric_tests/integration-test was removed as it was problematic when integrating the UM physics and currently not testing anything useful. Furthermore, none of the other applications include integration tests.

  • linked MetOffice/mo-bundle#1127
  • linked MetOffice/lfric-jedi#1298

Note: given the nature of this change I would like to test this branch again before the branch is merged into main.

- is blocked-by #502

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)

Tested via lfric-jedi: MetOffice/mo-bundle#1127.

trac.log

Retest at: 08cc8b5

Test Suite Results - lfric_apps - LA_512/run11

Suite Information

Item Value
Suite Name LA_512/run11
Suite User steven.sandbach
Workflow Start 2026-07-02T16:14:18
Groups Run developer
Dependency Reference Main Like
casim MetOffice/casim@2026.03.2 True
jules MetOffice/jules@1ea67b4 True
lfric_apps ss421/lfric_apps@update_forecast_config_ifdefphys False
lfric_core MetOffice/lfric_core@5d6b2db True
moci MetOffice/moci@2026.03.2 True
SimSys_Scripts MetOffice/SimSys_Scripts@3e2998b True
socrates MetOffice/socrates@2026.03.2 True
socrates-spectral MetOffice/socrates-spectral@2026.03.2 True
ukca MetOffice/ukca@1cdb9c2 True

Task Information

✅ succeeded tasks - 1218

Retest at: [f1ee397](https://github.com/MetOffice/lfric_apps/pull/512/commits/f1ee3976e67c10103e1545b1bc1a2621bcb5963c)

# Test Suite Results - lfric_apps - LA_512/run6

## Suite Information

| Item | Value |
| :--- | :--- |
| Suite Name | [LA_512/run6](https://cylchub/services/cylc-review/cycles/steven.sandbach/?suite=LA_512%2Frun6) |
| Suite User | steven.sandbach |
| Workflow Start | 2026-06-25T00:04:31 |
| Groups Run | developer |

| Dependency | Reference | Main Like |
| :--- | :--- | :--- |
| casim | [MetOffice/casim@2026.03.2](https://github.com/MetOffice/casim/tree/2026.03.2) | True |
| jules | [MetOffice/jules@1ea67b4](https://github.com/MetOffice/jules/tree/1ea67b4) | True |
| lfric_apps | [ss421/lfric_apps@update_forecast_config_ifdefphys](https://github.com/ss421/lfric_apps/tree/update_forecast_config_ifdefphys) | False |
| lfric_core | [MetOffice/lfric_core@b2374d3](https://github.com/MetOffice/lfric_core/tree/b2374d3) | True |
| moci | [MetOffice/moci@2026.03.2](https://github.com/MetOffice/moci/tree/2026.03.2) | True |
| SimSys_Scripts | [MetOffice/SimSys_Scripts@3e2998b](https://github.com/MetOffice/SimSys_Scripts/tree/3e2998b) | True |
| socrates | [MetOffice/socrates@2026.03.2](https://github.com/MetOffice/socrates/tree/2026.03.2) | True |
| socrates-spectral | [MetOffice/socrates-spectral@2026.03.2](https://github.com/MetOffice/socrates-spectral/tree/2026.03.2) | True |
| ukca | [MetOffice/ukca@1cdb9c2](https://github.com/MetOffice/ukca/tree/1cdb9c2) | True |

## Task Information
:white_check_mark: succeeded tasks - 1206

Previous trac.log

Test Suite Results - lfric_apps - update_forecast_config_ifdefphys/run5

Suite Information

Item Value
Suite Name update_forecast_config_ifdefphys/run5
Suite User steven.sandbach
Workflow Start 2026-05-22T13:29:40
Groups Run developer
Dependency Reference Main Like
casim MetOffice/casim@2026.03.2 True
jules MetOffice/jules@2026.03.2 True
lfric_apps ss421/lfric_apps@update_forecast_config_ifdefphys False
lfric_core MetOffice/lfric_core@2026.03.2 True
moci MetOffice/moci@2026.03.2 True
SimSys_Scripts MetOffice/SimSys_Scripts@2026.03.2 True
socrates MetOffice/socrates@2026.03.2 True
socrates-spectral MetOffice/socrates-spectral@2026.03.2 True
ukca MetOffice/ukca@2026.03.2 True

Task Information

✅ succeeded tasks - 1172

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

@github-actions github-actions Bot removed the cla-modified The CLA has been modified as part of this PR - added by GA label Jun 24, 2026
@ss421

Copy link
Copy Markdown
Contributor Author

Matthew Hambley (@MatthewHambley) Ive updated where possible and replied to your review comments above. The changes here is similar to the one applied in #502 and I note your comment about the general build system here: #502 (review) - I assume the same comment applies here? Can you advise if anything else needed?

Ricky Wong (@mo-rickywong) - the dependent PR #502 has been merged. I have merged the head of main yesterday and retested via the LFRic and JEDI testing. Please note that there is a file that needs to be copied to $BIGDATA.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It sounds like you've merged another ticket's changes into this branch. Please either strip that off or make sure it's merged up to the head of that branch. Otherwise the merge to trunk will go bad.

@ss421

Copy link
Copy Markdown
Contributor Author

It sounds like you've merged another ticket's changes into this branch. Please either strip that off or make sure it's merged up to the head of that branch. Otherwise the merge to trunk will go bad.

This is a branch from #502 as I need the changes from that here.

Last week (24th June), I merged in the head of main since the original base branch was committed via #502. According to the GitHub auto merge button there are no conflicts with MetOffice main. There is some of #502 history in my branch but the only way I can get rid of that is to rebase. Id rather not do that as it was quite painful merging up in the first place and I didn't think that was necessary for these PR's as the history will be squashed on commit?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Just one outstanding issue.

Choose a reason for hiding this comment

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

Just spotted this on my way past. At 2MiB this is too big for the source code repository. It also seems too big for the "do anything" example. Remember, the examples are there so a new user can prove to themselves that they've compiled an executable which will "do anything." Not one which will "do something."

In light of this, is there a smaller (order a couple of KiB) version of the file which could be used. Can the need to use this file be turned off in the configuration?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This test case is based on lfric_atm and as far as I can tell there are no other suitable smaller tests. The one that was included previously was based on a gungho configuration: https://github.com/MetOffice/lfric_apps/blob/main/rose-stem/app/gungho_model/opt/rose-app-semi-implicit-for-linear.conf. It is not straight forward to support that configuration in jedi_forecasts because the base configuration has changed significantly so it was removed in favor of a more realistic case (which was being added here as the example case).

There are two options to remove the file-size issue:

  1. move the files to $BIGDATA and use a symbolic link to the files.
  2. Revert the change and put back the older configuration - the rose-stem configuration this is based on no longer exists.

In 5cee5ee I have done the later but I would prefer if it is acceptable to do the former (as in add the data and use sym-links). Can you advise on what is appropriate?

Thanks,

Steve

@ss421

Copy link
Copy Markdown
Contributor Author

Merged up to the head of main via: 79fac20. Retesting via: jedi_lfric_tests_developer group as there was lfric_atm configuration updates added since the last commit. Will follow that with a run of the full developer test-suite and will post new trac.log.

@ss421

Steven Sandbach (ss421) commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

Merged up to the head of main via: 79fac20. Retesting via: jedi_lfric_tests_developer group as there was lfric_atm configuration updates added since the last commit. Will follow that with a run of the full developer test-suite and will post new trac.log.

Retested after merge at head of branch (08cc8b5) and added the log to the description.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hi Steve, just a couple of questions around the metadata

@@ -1,4 +1,81 @@
import=jedi_common/HEAD
import=lfric-gungho/HEAD

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why are you looking to change to using gungho directly? As it currently is, jedi-common will import the gungho metadata via lfric-adjoint and lfric-linear. The settings you've added below all look to be included in the jedi-common metadata, so this is duplicating that, which is not ideal.
Is there a specific reason you want to avoid importing lfric-adjoint and lfric-linear? In which case I would suggest we import lfric-gungho directly from jedi_common and move the linear/adjoint imports into the other jedi metadata sections (jedi_tlm_tests, jedi_tlm_forecast_tl, jedi_lfric_tests, jedi_id_tlm_tests and jedi_forecast_pseudo)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The way the configurations are arranged is not quite right in my view and I would like to refactor things at some point (soon...). In the case of the forecast application, the LFRic Model part of the confirmation should be identical to the lfric_atm configuration. While it is possible that the adjoint configuration contains the same configuration as lfric_atm there is scope for them to conflict I assume. I agree with your comment on moving where the imports are included but Id prefer to postpone that till later as the current setup is works OK.

Id like to see a better setup for the configurations and I dont think we should have jedi_common but I don't have a great background in rose meta-data. I'll explain a little more because Im not sure what is the best way forward...

JEDI configuration background

There are currently four configurations and they include model configurations

  1. forecast: will run the full NL model (lfric-atm) so the lfric_atm configuration is used
  2. forecast_pseudo: does not run a model, uses lfric-xios to read a previously run model. We could just include the parts of the configuration that is required, we currently use all of the configuration, its just mostly ignored.
  3. tlm_forecast_tl: runs the linear model. This could just use the linear model configuration but as with forecast_pseudo we currently use all of the configuration, the adjoint part of the configuration is ignored.
  4. id_tlm_tests and tlm_tests: runs the linear and adjoint model to test the correctness of the adjoint (science test: dot product). The adjoint configuration which is effectively linear+adjoint due to the way the models inherit form each other.

In addition to the outlined configurations. Each of these apps include JEDI emulator specific configuration. This is all stored in a single configuration file which is ok because the JEDI and LFRic configuartions are defiantly distrinct. I would like to have the JEDI and model configurations in separate files at some point as that would make things clearer. In summary, for the forecast app we would have two configurations:

jedi_forecast_configuartion.nml and nl_model_configuration.nml

which would be run via:
$ path/to/bin/jedi_forecast jedi_forecast_configuartion.nml

and jedi_forecast_configuartion.nml points at nl_model_configuration.nml to handle the model construction.

In JEDI the configurations are arranged this way. Todate this has not been an issue issue but we will soon need to build an application with a configuration for the NL (lfric-atm) and TLM (linear+adjoint) models. If we have separate configurations then we would have something like:

jedi_tlm_test_configuartion.nml, nl_model_configuration.nml, tlm_model_configuration.nml with:
$ path/to/bin/jedi_forecast jedi_tlm_configuartion.nml

and jedi_tlm_test_configuartion.nml pointing at nl_model_configuration.nml & tlm_model_configuration.nml with separate jedi objects to handle the NL and TLM model construction.

@@ -1,18 +1 @@
import=jedi_common/vn3.1

#==============================================================================

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This file shouldn't be edited, so please revert these changes

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Reverted the change to this file and committed at: 6601d3d

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The jedi_common section is there to reduce duplication of the metadata, and so adding duplication here isn't ideal. But I realise this PR has been waiting for a while, so I'll approve and this can be tweaked further down the line.

@james-bruten-mo

Copy link
Copy Markdown
Collaborator

CLA failure due to changing behaviour of checkout action. Bypassing for this PR

@james-bruten-mo
James Bruten (james-bruten-mo) merged commit cf5fe4f into MetOffice:main Jul 21, 2026
6 of 8 checks passed
@ss421

Copy link
Copy Markdown
Contributor Author

The jedi_common section is there to reduce duplication of the metadata, and so adding duplication here isn't ideal. But I realise this PR has been waiting for a while, so I'll approve and this can be tweaked further down the line.

Thanks for reviewing and merging. I'm keen to rationalise the configurations and think I would benefit with some input from you. There will soon be some work aimed at creating multiple model configurations because we need to run and test with a setup that has the configurations for the full NL model (lfric-atm) and the TLAD (linear-adjoint). Ricky did some work on a branch to demonstrate how this might be done in the proof of concept which I will use for the basis of that. So when I have something I would appreciate your input. Thanks, Steve.

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

Labels

Linked Jedi This PR is linked to a Jedi PR - this will be managed by the DA team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants