PSyclone icon indicating copy to clipboard operation
PSyclone copied to clipboard

LFRic changed name of field_r*types

Open hiker opened this issue 1 year ago • 9 comments

At around r50490 lfric replaced the existing field_{r32,r64}_mod and integer_field with a template, and the build system includes a templaterator. As a result, the name of these files has changed. This does not really affect much of PSyclone, it relies on field_mod.F90:

 #if (RDEF_PRECISION == 32)
 use field_real32_mod, only: field_type         => field_real32_type, &
                              field_proxy_type   => field_real32_proxy_type, &
                              field_pointer_type => field_real32_pointer_type

But a lot of psydata libraries use the original name (to be able to work with 32- and 64-bit fields), they need to be changed. And all PSycl, seeone tests, examples, and tutorials(?) need to be changed.

If/when we agree to go ahead with this, we should move the infrastructure files to externals (so that in the future when lfric_core is on git, we can use a submodule), and also take care of pre-processing (in case of case-insensitive file systems), see #2239.

The templaterator itself seems to be a small-ish python script using jinja2, so we might be able to use our own script to create the source files from the template, without having to add the templaterator(?)

hiker avatar Jun 19 '24 13:06 hiker

That all sounds OK to me. I was thinking about suggesting we bite the bullet and bring in the infrastructure wholesale (with permission from the Met Office) but that might just complicate things further?

arporter avatar Jun 20 '24 07:06 arporter

Yes, the license issue might be the biggest problem. @TeranIvy , any idea if we can add the infrastructure folder (I believe this is all we need, likely even only a subset, since it also contains build tools)? We try to be on current LFRic, so I kind of need to fix the psydata stuff (for kernel extraction). Easy enough to do it with a search&replace, but then I have these changes in my tree, which is annoying :)

hiker avatar Jun 20 '24 10:06 hiker

The license is not a problem, and the Met Office doesn't have a problem. I have approval for LFRic to go public, so taking a mirror now should be fine. Hoping to do a port to a public GitHub repo in 2025 - possibly with a mirror copy earlier than that.

stevemullerworth avatar Jun 26 '24 11:06 stevemullerworth

Great, then I can have a look! Thanks a lot!

hiker avatar Jun 27 '24 14:06 hiker

One thing we need to be careful of is not to shoot ourselves in the foot when it comes to our ability to test with other compilers, especially nvidia.

arporter avatar Jun 27 '24 14:06 arporter

Besides the name change (and I can't see this being a problem with any compiler ;) ), the templaterator is a pre-processing step, i.e. nothing is different from what PSyclone of fparser will see.

Are you thinking of any specific issues?

hiker avatar Jun 27 '24 14:06 hiker

I thought you were talking about bringing in the whole infrastructure. If you're not then it's all fine :-)

arporter avatar Jun 27 '24 14:06 arporter

I don't think we should include the whole lfric_core directory (though I kind of said it this way). There is a lot of unnecessary stuff in there (applications, mesh tools, rose_stem components, unit-tests) - Seems to be around 100 MB, only 2.2 MB with actual source files that we would need for compiling lfric (well, plus whatever we need to actually compile this, but I will likely just use a standard Makefile and some jinja?? Well, I'll check).

hiker avatar Jun 28 '24 08:06 hiker

Just to show what needs to be changed in psydata: The following patch works for one psydata file:

diff value_range_check.jinja value_range_check.jinja-field-real32
55,56c55,56
<     use field_r{{prec}}_mod, only : field_r{{prec}}_type, &
<                               field_r{{prec}}_proxy_type
---
>     use field_real{{prec}}_mod, only : field_real{{prec}}_type, &
>                               field_real{{prec}}_proxy_type
123c123
<         type(field_r{{prec}}_type), intent(in)                   :: value
---
>         type(field_real{{prec}}_type), intent(in)                :: value
125c125
<         type(field_r{{prec}}_proxy_type)                         :: value_proxy
---
>         type(field_real{{prec}}_proxy_type)                      :: value_proxy

An in-between solution could be to add the new field types to the existing infrastructure files (i.e. we would have both field_r32 and field_real32), but I would prefer a more permanent solution.

hiker avatar Aug 25 '24 02:08 hiker

It seems not to be easy to just checkout part of a repository (e.g. https://gist.github.com/ZhuoyunZhong/2c08c8549616e03b7f508fea64130558). It uses the sparse-checkout option. But my understanding is that this is actually a manual step, i.e. you still declare a full repo as the submodule, but then manually only checkout some bits. Since this is a manual option, it might not be that ideal?

Note that unless LFRic switches to github, we can't really use a submodule anyway.

Other options would be:

  1. We just copy the infrastructure file into a separate directory (I would still suggest under external???). It would then be easy to update this whenever the infrastructure file changes and make us independent of the future svn to git switch.
  2. We put the required lfric infrastructure files into a standalone git repo (till lfric migrates to git :( ), and use this as a submodule.
  3. We accept the fact that there will be too many files in a subrepository. But to support our builds, we use a symlink from the subrepository with the lfric infrastructure into a subdirectory somewhere. This would allow us to easy include and manage our own build system for just the infrastructure files.

My feeling is that the easiest long term option would be a mixture:

  1. Putting lfric infrastructure into a (temporary) git repo (so we don't need to switch later from svn to git, we only need to change the URL)
  2. We add our own build directory with a link for the source files.
  3. We add instructions on how to use sparse-checkout to minimise the required disk space for infrastructure.

hiker avatar Dec 17 '24 12:12 hiker

We agreed to add the infrastructure source files as a copy (not sub-repo or svn link or so) to external/lfric_infrastructure, and then have a script that updates these files for us. This way we can fix our infrastructure version (to avoid that changes to the infrastructure affect our CI), and don't need to store any secrets (and also minimise required memory for developers).

hiker avatar Apr 28 '25 10:04 hiker

I have started to write a script that will copy and preprocess the files from lfric_core. This seems to work fine, except that we now can't avoid a dependency to NetCDF (the original infrastructure had some refactoring applied which created a mesh base class without NetCDF dependencies). That is likely acceptable (since all integration tests compile lfric, so we have NetCDF).

We also need to update all runnable LFRic examples (eg14, all under eg17, training on branch 1623_..., maybe more). When creating the original infrastructure files, I refactored all function to take pointer to objects (e.g. meshes), meaning we didn't need the various collection objects that LFRic has. Now in order to use lfric_core, we need to use these IDs, so we need to update all examples.

hiker avatar Apr 30 '25 10:04 hiker

We have two runable examples under examples/lfric/eg17 - one using the unit-tests to setup the mesh, one to read the global mesh from a NetCDF file. The interface to NetCDF file seems to have changed quite a bit :( To some degree, showing how to read NetCDF from a file isn't really PSyclone's task, so we could delete the netcdf example. But the tutorial (tutorial/practicals/LFRic/building_code/3_time_evolution) seems to use the same outdated function, and it would have to be rewritten :(

hiker avatar Jul 01 '25 12:07 hiker

I made some progress, but are basically stumped with three issues:

  1. I can't create a non-netcdf version of the infrastructure library. As far as I can tell I would have to add some #ifdef to do that. I believe we agreed that this is acceptable, and while I might mot like that, that might be ok?
  2. We have lfric/eg17/full_example_netcdf, an example showing how a NetCDF file is read in to define the mesh (the full_example is using a unit-test dummy grid :) ). The old grid has the wrong attributes. I tried various other grids from LFRic, and even tried to re-create the mesh we had with a more current version of mesh_tools. But some LFRic meshes have the 'fill' value missing, and others as well as the newly create one abort with:
psyclone/examples/lfric/eg17/full_example_netcdf$ ./example 
20250805165052.868+1000:ERROR: Base mesh with 538976288 vertices per cell not supported.
  1. I have even more problem with Iva's tutorial (psyclone/tutorial/practicals/LFRic/building_code/3_time_evolution). The gungho library it includes seems to be totally out of date, I can't even get it to compile ... whole module files and classes seem to be missing.
gfortran -Wall -g -fcheck=bound -I ../../../../../external/lfric_infrastructure/preprocessed/PSYKE -I ../../../../../external/lfric_infrastructure/preprocessed/apps -I ../../../../../external/lfric_infrastructure/preprocessed/components -I ../../../../../external/lfric_infrastructure/preprocessed/configuration -I ../../../../../external/lfric_infrastructure/preprocessed/field -I ../../../../../external/lfric_infrastructure/preprocessed/function_space -I ../../../../../external/lfric_infrastructure/preprocessed/function_space/dofmap -I ../../../../../external/lfric_infrastructure/preprocessed/io -I ../../../../../external/lfric_infrastructure/preprocessed/kernel_metadata -I ../../../../../external/lfric_infrastructure/preprocessed/key_value -I ../../../../../external/lfric_infrastructure/preprocessed/mesh -I ../../../../../external/lfric_infrastructure/preprocessed/operator -I ../../../../../external/lfric_infrastructure/preprocessed/quadrature -I ../../../../../external/lfric_infrastructure/preprocessed/scalar -I ../../../../../external/lfric_infrastructure/preprocessed/time -I ../../../../../external/lfric_infrastructure/preprocessed/utilities -c configuration_mod.f90

configuration_mod.f90:50:7:

   50 |   use domain_size_config_mod, only : read_domain_size_namelist, &
      |       1

I started to try to fix things, but I had to start to randomly remove things that I couldn't get to work, .. :P

IMHO:

We delete example lfric/eg17/full_example_netcdf, since we still have full_example to show how to run it.

But I have no idea about the tutorial. The example is at least some 'real LFRic' code (a bit more than hello world), but then I think I need someone's help (@TeranIvy ??) to fix this.

hiker avatar Aug 05 '25 07:08 hiker

Can this be closed now @hiker?

arporter avatar Sep 08 '25 11:09 arporter