LFRic_atm build failure due to interplay of hard-coded PSyclone command line options for transmute and compiler bug

[jasonjunweilyu]

BOM has LFRic CI rose-stem testing pipeline failure on NCI-GADI due to LFRic_atm build error after enabling the existing METO transmute scripts (ifx is the compiler we are using) :

kernel/conv_gr_kernel_mod.f90(1260): warning #5151: Continuation character illegal as first non_blank in statement
                                                                                                                          !$omp  &
---------------------------------------------------------------------------------------------------------------------------------^
kernel/conv_gr_kernel_mod.f90(1261): error #5082: Syntax error, found '&' when expecting one of: METADIRECTIVE END BEGIN TARGET TASKGROUP REQUIRES NOTHING INTEROP DISPATCH ...
!$omp& do schedule(static) collapse(1)
-------^
kernel/conv_gr_kernel_mod.f90(1266): warning #5151: Continuation character illegal as first non_blank in statement

Through investigation with Joerg @hiker and Dave @davelee2804 , we found that this is because:

1. PSyclone output (due to indentation and line wrapping) contains the equivalent of

!$omp  &
!$omp& do schedule(static) collapse(1)

2. This seems to be acceptable according to the standard, but is only accepted by gfortran and cce, not by ifx or nvfortran, which led to the build error above.
3. Changing PSyclone command line option by removing -l all, or adding -l off to the end can solve this problem. However, the PSyclone command line options are now hardcoded in the Make build system:
   https://github.com/MetOffice/lfric_apps/blob/115f2af65a84e37cf926f5039eae33d25f03b2d0/interfaces/physics_schemes_interface/build/psyclone_transmute.mk#L57

Joerg has filed a bug report to Intel to see if they can make changes for ifx. However, since now LFRic build still breaks for us and probably other sites as well, is it possible to consider adding a flag now to makefiles that allows customized PSyclone command line options?

Tagging @MetBenjaminWent and @christophermaynard here as they provided valuable help in finding the cause of the error and discussing possible solutions.
Adding @yaswant , @t00sa , @james-bruten-mo here, as this support request is related to build system.

[MetBenjaminWent]

A straightforward fix would be to allow the user to override the argument options passed to the PSyclone command in Transmute (also per file would be very important but thats a little pass my Make-fu today).

I'll have a look at this today.

[andrewcoughtrie]

There should be no reason not to add an option for that PSyclone argument, it is used in a number of places both in LFRic Core and Apps so if it is going to be added it should be in all places.

See here: https://github.com/MetOffice/lfric_core/blob/main/infrastructure/build/psyclone/psyclone_psykal.mk for the main location of the PSyclone calls.

The main logic and definition of the new option should be in that psyclone_psykal.mk file and used throughout the code anywhere PSyclone is called.

@MatthewHambley may have some thoughts especially if this needs to be on a per file basis.

[MetBenjaminWent]

In apps the Transmute method (I think where the issue is occurring), is different to the Core one.

[andrewcoughtrie]

The flag is still used in multiple places if it's the -l all that is the problem, we shouldn't be allowing manual override in one place and not others, that will just lead to confusion of what the options do.

[MetBenjaminWent]

Its a reasonable position. I worry that given we can't make extensive changes to the build system at this time, we might be deterred from making any at all that required more comprehensive adjustments.
Also the two commands are affecting source in two different ways. Having them both settable in the same place is a good call (say the application, with a default in the command makefile), but having the both adjustable in different ways would be the better method in my opinion.

[jasonjunweilyu]

Hi @MetBenjaminWent and @andrewcoughtrie , thanks a lot for your quick response and valuable help. I have tested that with https://github.com/MetBenjaminWent/lfric_apps/tree/psyclone_transmute_flags_override and PSYCLONE_TRANSMUTE_EXTRAS=-l all --backend disable-indentation, the NCI Gadi rose-stem CI pipeline now succeeds without problem using ifx as compiler. Looking forward to the merging of MetOffice/lfric_apps#246!

[MetBenjaminWent]

Thanks! I think you'll override PSYCLONE_TRANSMUTE_EXTRAS in your site cylc file currently, under the build, for example in here:
https://github.com/MetOffice/lfric_apps/blob/main/rose-stem/site/nci/common/suite_config_gadi.cylc

[hiker]

Additional comment: Intel does not consider the behaviour of their compiler a bug. I need to sit down with Fortran and OpenMP standard to verify this claim ;) But it appears that this unexpected behaviour will not be fixed and therefore be experienced by other sites as well (anyone using intel, ifort, or nvfortran with transmute scripts).

In my fab scripts, I am looking at adding the backend-disable flag automatically in these cases.

[tinyendian]

The OpenMP standard seems to allow this, I just had a look at OpenMP 6 (https://www.openmp.org/wp-content/uploads/OpenMP-API-Specification-6-0.pdf, Section 5.1.1 "Free Source Form Directives"):
[...] continuation directive lines can have an ampersand after the directive sentinel with optional white space before and after the ampersand.

Intel's compiler manual says this (https://www.intel.com/content/www/us/en/docs/fortran-compiler/developer-guide-reference/2025-3/syntax-rules-for-compiler-directives.html):
Each continued line of the directive has the directive prefix optionally preceded by blanks or tabs, followed by an ampersand optionally preceded by blanks or tabs.

Sounds like the compiler should accept !$omp& and !$omp &, unless I misunderstood something?

With regards to ifort, I haven't seen this issue, but I'm currently debugging a segfault with lfric_atm when building with the old Intel 2023 compiler - not entirely clear yet, but it could be a compiler bug, in which case we'll probably switch to ifx as well. I currently have to disable transmute scripts in our builds due to the various PSyclone v3.2 related failures I reported last week.