PSyclone icon indicating copy to clipboard operation
PSyclone copied to clipboard

[LFRic] [PSyAD] Adjoint kernels with multiple code implementations have incorrect metadata

Open DrTVockerodtMO opened this issue 1 year ago • 6 comments

Whilst PSyAD seems to now be able to handle multiple code implementations in the tangent linear kernels (e.g.: kernels that have r_single and r_double code implementations), I have found issues regarding the metadata of the generated adjoint kernels. Some of the aspects of the metadata are inherited from the tangent linear kernel as opposed to being transformed into the adjoint names. I have supplied an example of the places where I have needed to edit the metadata in the adjoint kernel generated from combine_w2_field_kernel_mod.F90:

@@ -4,8 +4,8 @@
   use fs_continuity_mod, only : w2, w2h, w2v
   use kernel_mod, only : kernel_type
   implicit none
-  interface combine_w2_field_code
-  module procedure combine_w2_field_code_r_single, combine_w2_field_code_r_double
+  interface adj_combine_w2_field_code
+  module procedure adj_combine_w2_field_code_r_single, adj_combine_w2_field_code_r_double
 end interface
   type, public, extends(kernel_type) :: adj_combine_w2_field_kernel_type
   type(ARG_TYPE) :: META_ARGS(3) = (/ &
@@ -17,7 +17,7 @@
 
   private
 
-  public :: combine_w2_field_code
+  public :: adj_combine_w2_field_code
 
   contains
   subroutine adj_combine_w2_field_code_r_double(nlayers, uvw, uv, w, ndf_w2, undf_w2, map_w2, ndf_w2h, undf_w2h, map_w2h, ndf_w2v, &

The incorrect metadata results in compilation errors when building in LFRic.

DrTVockerodtMO avatar Feb 01 '24 15:02 DrTVockerodtMO

Thanks for this @DrTVockerodtMO - I'll take a look. I suspect some of the problems will be fixed by #2435 (adds support for interfaces) but possibly not all.

arporter avatar Feb 20 '24 10:02 arporter

First thing to note is that PSyclone/examples/psyad/lfric/tangent_linear/combine_w2_field_kernel_mod.F90 must be outdated because it doesn't contain an interface like the example that @DrTVockerodtMO provides.

arporter avatar Feb 20 '24 10:02 arporter

Well, #2435 does a little better - it does at least rename the routines listed within the interface:

interface combine_w2_field_code
  module procedure :: adj_combine_w2_field_code_r_single, adj_combine_w2_field_code_r_double
end interface combine_w2_field_code

however, the interface itself is not renamed. I'll do some digging.

arporter avatar Feb 20 '24 10:02 arporter

The problem is that we don't yet appear to properly support mixed precision kernels: https://github.com/stfc/PSyclone/blob/afed0a940a9955fb531fb368f5d3da8fbab6fcdd/src/psyclone/psyad/domain/lfric/lfric_adjoint.py#L139-L158 The referenced issue (#2236) has been closed in favour of #2347 which itself has been closed saying that "everything is now working." The fact that metadata.procedure_name is None for an interface indicates that we don't yet support this. This is actually the subject of #1946 so really we should have a TODO for that issue here.

arporter avatar Feb 20 '24 11:02 arporter

The problem is that we don't yet appear to properly support mixed precision kernels:

https://github.com/stfc/PSyclone/blob/afed0a940a9955fb531fb368f5d3da8fbab6fcdd/src/psyclone/psyad/domain/lfric/lfric_adjoint.py#L139-L158

The referenced issue (#2236) has been closed in favour of #2347 which itself has been closed saying that everything is now working.

Thank you for investigating this. It is quite possible that our version of psyad here does not have this metadata update yet (iirc we have PSyclone 2.5.0).

DrTVockerodtMO avatar Feb 20 '24 11:02 DrTVockerodtMO

I believe you don't quite yet have 2.5.0 (Iva's team are in the process of deploying it as we speak). However, unfortunately that isn't going to help. This needs a bit more work on it.

arporter avatar Feb 20 '24 12:02 arporter