Skip to content

Allow lfric_atm to compile for oasis coupled models #486

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Paul Earnshaw (pdearnshaw) wants to merge 6 commits into
base: main
Choose a base branch
from
+20 −1
Open

Allow lfric_atm to compile for oasis coupled models #486

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0ec2417
Copied in changes from head of final subversion branch.
pdearnshaw Jan 7, 2026
d53ad32
Added missing variable for coupled model configurations.
pdearnshaw Jan 20, 2026
06e575d
Merging in stable vn3.1 and resolving conflicts associated with lfric…
pdearnshaw Apr 8, 2026
ffd66a8
#92. Fixed some changes from attempt to merge in stable, which actual…
pdearnshaw Apr 21, 2026
919fa0a
#92. Added name to contributors file.
pdearnshaw May 7, 2026
d189844
Merge tag 'vn3.1.1' into 92_coupled_compilation_fix
pdearnshaw May 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CONTRIBUTORS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,5 @@
| thomasmelvin | Thomas Melvin | Met Office | 2026-01-15 |
| tinyendian | Wolfgang Hayek | Earth Sciences New Zealand | 2026-02-02 |
| DanStoneMO | Daniel Stone | Met Office | 2026-02-26 |
| ericaneininger | Erica Neininger | Met Office | 2026-03-02 |
| ericaneininger | Erica Neininger | Met Office | 2026-03-02 |
| pdearnshaw | Paul Earnshaw | Met Office | 2026-05-07 |
9 changes: 9 additions & 0 deletions applications/lfric_atm/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,15 @@ export INTERNAL_DEPENDENCIES = $(CORE_ROOT_DIR)/infrastructure \
$(APPS_ROOT_DIR)/interfaces/jules_interface \
$(APPS_ROOT_DIR)/interfaces/physics_schemes_interface

# If the COUPLED compiler directive has been set then add appropriate
# coupled model settings
ifdef COUPLED
ifeq ("$(COUPLED)","True")
Comment on lines +48 to +49

@MatthewHambley Matthew Hambley (MatthewHambley) May 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Normally we would just test for the existence of the macro. What is the reason for requiring it to have a particular value?

@pdearnshaw Paul Earnshaw (pdearnshaw) May 20, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The COUPLED here is not a macro, it is an environment variable. What we want in this test is to make sure firstly that it has been set, and then that it has been set to True. The idea being that if it is not set, or set to False then the coupled model infrastructure is not applied.

As I understand it the ifdef statement can test for environment variables or keyword-like arguments in the make command, it cannot act on PRE_PROCESS_MACRO entries. So the choice in how to identify if we wanted to compile as a coupled model was to use an optional environment variable.

Happy to consider other ways of doing this.

@MatthewHambley Matthew Hambley (MatthewHambley) Jun 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The PRE_PROCESS_MACROS are converted into a set of -D<key>=<value> arguments to the compiler. So they will appear to the preprocessor as macros. In which case you can just test on their presence. Unless you think there will be cases where people are setting that value to False.

There's no correctness reason not to do it the way you have, it just runs the risk of people getting unexpected behaviour if they say COUPLED=YES.

I don't mind which you do so feel free to resolve this conversation when you have made a decision.

export INTERNAL_DEPENDENCIES += $(CORE_ROOT_DIR)/components/coupling
export PRE_PROCESS_MACROS += MCT COUPLED
endif
endif

META_VN ?= HEAD
export META_FILE_DIR = $(PROJECT_DIR)/rose-meta/lfric-$(PROJECT_NAME)/$(META_VN)

Expand Down
9 changes: 9 additions & 0 deletions applications/lfric_atm/source/lfric_atm.f90 → applications/lfric_atm/source/lfric_atm.F90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@
program lfric_atm

use cli_mod, only: parse_command_line
#ifdef MCT
use coupler_mod, only: set_cpl_name
#endif
Comment thread
mike-hobson marked this conversation as resolved.
use driver_collections_mod, only: init_collections, final_collections
use driver_comm_mod, only: init_comm, final_comm
use driver_config_mod, only: init_config, final_config
Expand All @@ -39,6 +42,9 @@ program lfric_atm
type(modeldb_type) :: modeldb

character(*), parameter :: application_name = "lfric_atm"
#ifdef MCT
character(*), parameter :: cpl_component_name = "lfric"
#endif
Comment thread
mike-hobson marked this conversation as resolved.
character(:), allocatable :: filename
integer(tik) :: id_setup
type(namelist_type), pointer :: io_nml
Expand Down Expand Up @@ -71,6 +77,9 @@ program lfric_atm

call modeldb%io_contexts%initialise(application_name, 100)

#ifdef MCT
call set_cpl_name(modeldb, cpl_component_name)
#endif
Comment thread
mike-hobson marked this conversation as resolved.
call init_comm( application_name, modeldb )

call init_config( filename, gungho_required_namelists, &
Expand Down
Loading