#474: LFRic-Nudging: Part-A: Introduce ability to read nudging datafiles #474
#474: LFRic-Nudging: Part-A: Introduce ability to read nudging datafiles #474Mohit Dalvi (mcdalvi) wants to merge 19 commits into
Conversation
… time-axis definitions
…r the new arguments
|
Is there any chance main could be merged into this branch? I'm currently unable to test the changes with JEDI due to the conflicts |
|
Apologies, I was not aware creating a PR triggers all this activity ! I have changed it to draft till all my testing is complete. |
…at are defined in source code; implement upgrade macro
…2bit as done for other azspice runs; remove unused additions to ancil reading code
Input test files for $BIGDATAThe input files for the lfric_atm_nwp_gal9_nudging_xx tests have been generated using ERA5 reanalysts data provided by by the Copernicus Climate Change Service distributed via the Copernicus Data Service. Dataset information, including usage terms can be found here: https://cds.climate.copernicus.eu/datasets/reanalysis-era5-complete?tab=documentation The files proposed to be added to $BIGDATA are: Both the files contain the following attributes: |
iboutle
left a comment
There was a problem hiding this comment.
Hi Mohit
I know this isn't fully ready yet, but I'm on leave next week so wanted to give some comments and code-owner approval so you can get this into code review by next Friday. Hopefully nothing too challenging, but a few requests and suggestions below.
Cheers
Ian
…definition and diagnostic list - this requires creating the fields in main%none
James Bruten (james-bruten-mo)
left a comment
There was a problem hiding this comment.
Thanks
DanStoneMO
left a comment
There was a problem hiding this comment.
Tested this and can confirm this works fine with JEDI, no linked JEDI PR will be needed
| <axis id="lw_bands_radiation_levels" name="lw_bands_radiation_levels" /> | ||
| <axis id="photolysis_pathways" name="photolysis_pathways" /> | ||
| <axis id="photol_species" name="photol_species" /> | ||
| <axis id="photol_species" name="photol_species" /> |
There was a problem hiding this comment.
Only thing to note from me is this rogue space here
There was a problem hiding this comment.
Thanks DanStoneMO . I have fixed that in my copy
Thomas Bendall (tommbendall)
left a comment
There was a problem hiding this comment.
Science Review
This PR adds the technical infrastructure for nudging to ERA or AIFS data, and will be followed by a PR to add the science to perform the nudging itself (including only nudging certain length scales).
Thanks so much for this Mohit -- it looks great. I have two more major thoughts, and then a handful of other suggestions that I've commented in the code.
- Do you think we should reorder the ERA ancil data vertically before we add it to BIG_DATA_DIR? (I'm assuming we haven't done this yet)... That would make the vertical interpolation more efficient when we get to that. If that's too much work before the deadline then we can potentially do it later when adding AIFS data?
- I have an annoying suggestion to change the settings of the
nudging-era-coarsetest, which is to set the aerosols to be on the dynamics mesh rather than keeping them on a coarse mesh. I just thought this would be a bit cleaner in the test-suite, so have added suggestions on how to do this
…e mesh only for nudging. Use ERA files inverted from original top-bottom order (level=1=top) to bottom-top order
|
Thomas Bendall (@tommbendall) I have reversed the ERA file data using 'ncpdq -a hybrid' and using the new files in the tests now. Test results updated. |
Thomas Bendall (tommbendall)
left a comment
There was a problem hiding this comment.
Thanks for all of this! The changes look good to me and I'm happy to approve this.
Matthew Hambley (MatthewHambley)
left a comment
There was a problem hiding this comment.
A few things to consider while we think about the linked ticket.
| type(mesh_type), pointer :: mesh => null() | ||
| type(mesh_type), pointer :: twod_mesh => null() |
There was a problem hiding this comment.
We no longer require initialisation on declaration of pointers. In fact we now prohibit it. Please use nullify(pointer) at the top of the procedure code, if it is not otherwise immediately initialised.
There was a problem hiding this comment.
Done b906772. also updated neighbouring pointer declarations for consistency.
| if (present(twod)) field_spec%twod=twod | ||
| if (present(empty)) field_spec%empty=empty | ||
| if (present(coarse)) field_spec%coarse=coarse | ||
| if (present(coarse_mesh_name)) field_spec%mesh_name=coarse_mesh_name |
There was a problem hiding this comment.
Why is the generic field_spec%mesh_name being assigned from the specific course_mesh_name. If the type member is generic, the initialising variable should be too. If the initialising variable is specific, the type member should also be.
There was a problem hiding this comment.
Code updated to use coarse_mesh_name throughout. b906772
| else | ||
| dim = 1 | ||
| end if | ||
| case ('ecmwf_levels') |
There was a problem hiding this comment.
Does this actually have anything to do with ECMWF or is that just the source of the data which happens to be being used at the moment?
There was a problem hiding this comment.
Changed to nudging_levels in all references now. b906772
|
|
||
| type(field_maker_type) :: creator | ||
|
|
||
| call log_event('GungHo: Creating nudging fields...', LOG_LEVEL_INFO) |
There was a problem hiding this comment.
Does this need to be "info" level. We already suffer the logging being much too talkative. This goes for all other new event logs.
There was a problem hiding this comment.
Changed to LOG_LEVEL_TRACE.
| call processor%apply(make_spec( & | ||
| 'surface_pressure_nudging_ext_ref', main%none, W3, & | ||
| coarse=coarse_nudging, & | ||
| coarse_mesh_name=nudging_mesh_name, & | ||
| twod=.true., time_axis=axis%nudging & | ||
| )) |
There was a problem hiding this comment.
Do we always want to nudge surface pressure? The other fields are conditional on configuration.
There was a problem hiding this comment.
Surface pressure in the model is not nudged. This field read from the datafiles will be used to re-create the source model vertical profile while remapping the nudging data onto model mesh.
Since the remapping is needed when any one of theta and wind is being nudged (i.e. create_nudging_fields is called) there is no condition around reading this field itself.
There was a problem hiding this comment.
A comment to that effect will help future developers.
| if (coarse_rad_aerosol) then | ||
| mesh_name = aerosol_mesh_name | ||
| else | ||
| mesh_name = '' | ||
| end if |
There was a problem hiding this comment.
Wasn't this set above, outside the conditional? Do we have code duplication here?
| !> @brief Map moisture array enumerators to moisture array. | ||
| type(time_axis_dict_type), parameter :: time_axis_dict & | ||
| = time_axis_dict_type(525,529,536) | ||
| = time_axis_dict_type(525,529,536,542) |
There was a problem hiding this comment.
Where do these numbers come from? UM code? A comment to explain this would be handy.
There was a problem hiding this comment.
These numbers are part of the field_spec design itself. See original comments lfric_core:#4208, which seem to indicate they have to be unique for each field, within a different range for each field category.
There was a problem hiding this comment.
I see that, in which case, a comment explaining that this is where these numbers are defined and they are opaque identifiers with no intrinsic meaning would help future developers.
A better solution still would be to define them as integer, parameter :: usefule_name = 525, then use them here. If you make these parameters public, they can be used outside the module. If that is their purpose. Even if it isn't, keeping them private but having useful names would help developers of this module.
thomasmelvin
left a comment
There was a problem hiding this comment.
All looks good to me
Matthew Hambley (MatthewHambley)
left a comment
There was a problem hiding this comment.
There are also changes requested for core which may effect this change.
| @@ -0,0 +1,112 @@ | |||
| !----------------------------------------------------------------------------- | |||
| ! (c) Crown copyright 2025 Met Office. All rights reserved. | |||
There was a problem hiding this comment.
Wrong year, although we are not including year in the copyright statement any more.
| use external_forcing_config_mod, only : theta_forcing_nudging, & | ||
| theta_forcing, & | ||
| wind_forcing_nudging, & | ||
| wind_forcing |
There was a problem hiding this comment.
Please use the new configuration API. You can find documentation of this at https://metoffice.github.io/lfric_core/how_to_use_it/lfric_datamodel/modeldb.html#accessing-configuration-data but please ask if you have further questions.
There was a problem hiding this comment.
Matthew Hambley (@MatthewHambley)
Re: using modeldb to access configuration values: I have attempted to implement this for process_nudging_fields. However process_nudging_fields is also called in gungho_model_mod/before_context_close, same as the LBC fields. This before_contaxt_close routine name is actually passed as a procedure pointer to init_io -> lfric_core:driver_io and finally called from lfric_xios_context_mod. I cannot see a way to use the modeldb argument at this level in the code hierarchy.
Thomas Bendall (@tommbendall)
| !> @param[in] mesh The current 3d mesh | ||
| !> @param[in] twod_mesh The current 2d mesh (not used here) | ||
| !> @param[in] mapper Provides access to the field collections | ||
| !> @param[in] clock The model clock | ||
| subroutine create_nudging_fields(mesh, twod_mesh, mapper, clock) |
There was a problem hiding this comment.
If the 2D mesh isn't being used, why pass it in? If it is needed later, then it can be obtained from the mesh collection. Something like mesh_collection%get_mesh(mesh, extrusion=TWOD) but check the API.
Given that you also need the clock and field collections, it might be more straight forward to pass modeldb in. Note that it also contains the configuration so no need to pass that either.
| !> @brief Map moisture array enumerators to moisture array. | ||
| type(time_axis_dict_type), parameter :: time_axis_dict & | ||
| = time_axis_dict_type(525,529,536) | ||
| = time_axis_dict_type(525,529,536,542) |
There was a problem hiding this comment.
I see that, in which case, a comment explaining that this is where these numbers are defined and they are opaque identifiers with no intrinsic meaning would help future developers.
A better solution still would be to define them as integer, parameter :: usefule_name = 525, then use them here. If you make these parameters public, they can be used outside the module. If that is their purpose. Even if it isn't, keeping them private but having useful names would help developers of this module.
| logical(l_def) :: coarse = .false. ! Is it coarse? | ||
| character(str_def) :: coarse_mesh_name = '' ! Name of the coarse mesh, or blank string |
There was a problem hiding this comment.
In the long run I don't think "coarseness" is a useful property. Instead some measure of resolution would provide more generalised information. However, given you are following the existing patter, I wont insist on it for this change. Please have a think about the general case and if there is one, raise an issue and link it here.
| end subroutine create_nudging_fields | ||
|
|
||
| !> @brief Iterate over active nudging fields and apply an arbitrary | ||
| !! processor to the field specifiers. |
There was a problem hiding this comment.
Please document the subroutine arguments.
| use multires_coupling_config_mod, only : coarse_nudging, & | ||
| nudging_mesh_name |
There was a problem hiding this comment.
As noted above, we are migrating to configuration objects.
| call processor%apply(make_spec( & | ||
| 'surface_pressure_nudging_ext_ref', main%none, W3, & | ||
| coarse=coarse_nudging, & | ||
| coarse_mesh_name=nudging_mesh_name, & | ||
| twod=.true., time_axis=axis%nudging & | ||
| )) |
There was a problem hiding this comment.
A comment to that effect will help future developers.
| if (coarse_rad_aerosol) then | ||
| mesh_name = aerosol_mesh_name | ||
| else | ||
| mesh_name = '' | ||
| end if | ||
|
|
There was a problem hiding this comment.
Can you just pass the mesh object rather than messing about with names?
…places where *process_nudging_fields* is called
|
Your CLA signature was found on the base branch, but you appear to have modified the CONTRIBUTORS.md file in this PR. Please do not edit the CONTRIBUTORS.md file. If you have already signed the CLA, revert changes to the file and your signature will be picked up. |



PR Summary
Sci/Tech Reviewer: Thomas Bendall (@tommbendall)
Code Reviewer: Matthew Hambley (@MatthewHambley)
This PR introduces the ability to read in datafiles that will be used for Nudging the Momentum models (Ref: Nudging NWP to AI/ML models - possibly targeting PS49).
The handling of Nudging data is modelled on existing lateral boundary condition handling in LFRic since they are both required to be interpolated from high time resolution file data on every model timestep.
Nudging changes summary:
Ability to read in nudging data for temperature, U-wind, V-wind and Log surface pressure (required for vertical remapping to model levels).
New [namelist:nudging], which will be active when theta or wind are 'forced by nudging' (in namelist:external_forcing). A corresponding upgrade_macro to add the namelist and I/O related items.
Introduce routine create_nudging_fields to create the nudging input fields as '2-D with user-specified (nudge_data) levels'. Corresponding grid_def_nudging.xml and grid_def_nudging_coarse.xml added to lfric_atm/metadata/.
Introduce ability to read in nudging data on native ('dynamics') or a coarser grid, with rationalising of routines that create the model fields ('make_spec', 'create_model_data') to accept both aerosol and nudging coarse mesh definitions.
Note that the above requires a LFRIC_CORE change to allow defining 3-D coarse grid fields as ''multigrid_ll'' x ndata.
Define a nudging_time_axis which will be populated based on the actual number of time records in the specified nudging file.
Introduce two new rose-stem tests lfric_atm_nwp_gal9_nudging-C48 that reads nudging files on the model grid and lfric_atm_nwp_gal9_nudging_coarse-C48 that reads in data at a lower resolution (C12) grid. This currently uses nudging data created from ERA5 source, but aim is to expand this to AIFS and other models.
At this stage the reference fields are only made available to LFRic and do not modify temperature/ winds.
linked #369: Allow coarse-grid fields with ndata levels: vn3.1 read coarse 2d lfric_core#369
Code Quality Checklist
Testing
Test Branch: test2_vn31_nudging_data
trac.log
Test Suite Results - lfric_apps - test2_vn31_nudging_data/run4
Suite Information
Task Information
✅ succeeded tasks - 1259
Security Considerations
Performance Impact
AI Assistance and Attribution
VsCode - code completion/ snippets.
Documentation
PSyclone Approval
Sci/Tech Review
(Please alert the code reviewer via a tag when you have approved the SR)
Code Review