lfric2lfric: Allow multiple meshes to be created and reorganise configs - #655
lfric2lfric: Allow multiple meshes to be created and reorganise configs#655cjohnson-pi wants to merge 3 commits into
Conversation
|
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. |
Mike Hobson (mike-hobson)
left a comment
There was a problem hiding this comment.
The ability to support providing multiple meshes to a task is definitely something we need. I'm no expert on how this should be done in the suites. James knows far more about this than I do and is the code reviewer, so I'll leave it to him to make any technical judgements.
When I added a coupling test to the Core test suite, this needed to use two meshes and James set this up for me. We should make sure that whatever the method used to specify two meshes is, it is consistent across all tasks that need it in Apps and Core repository test suites.
Over to James Bruten (@james-bruten-mo) for a proper review
James Bruten (james-bruten-mo)
left a comment
There was a problem hiding this comment.
Hi Christine,
This looks reasonable. For the populate_graph_sections file, could you have a look at doing it in the same way as we've done in core to help with maintainability. I've linked to the relevant changeset below.
For populate_gen_weights_graph, I think it does what you've described in the PR description. But I'm unclear why we need to wait for the first mesh to be built before building the 2nd one?
| {% if task_values["resolution"] is iterable and task_values["resolution"] is not string %} | ||
| {% set resolutions = task_values["resolution"] %} | ||
| {% else %} | ||
| {% set resolutions = [task_values["resolution"]] %} | ||
| {% endif %} |
There was a problem hiding this comment.
| {% if task_values["resolution"] is iterable and task_values["resolution"] is not string %} | |
| {% set resolutions = task_values["resolution"] %} | |
| {% else %} | |
| {% set resolutions = [task_values["resolution"]] %} | |
| {% endif %} | |
| {% if task_values["resolution"] is string %} | |
| {% set resolutions = [task_values["resolution"]] %} | |
| {% endif %} |
I think you can simplify this by just checking whether it's a string
There was a problem hiding this comment.
Yes I could just check for a string. But I'll still need to have both parts, because sometimes its a string (for cases with 2 meshes) and sometimes its not (for cases with 1 mesh).
| {% endif %} | ||
|
|
||
|
|
||
| {# ################################### #} |
There was a problem hiding this comment.
For this file, Mike and I did something very similar in lfric core a while ago. Could you look to do it in the same way here (I think they're functionally identical), so that the 2 files remain as similar as possible. The changeset for lfric_core is at MetOffice/lfric_core@03f9e4d#diff-239288cf861d4d3cf49db1b9cb5fb35fb70a23d90e4c7d9679df64e7c9b0df39
There was a problem hiding this comment.
Thanks for pointing me to that change. I will have a go at trying to align my changes with that.
Many thanks for the review. Ah you're right, we don't need to wait for the first mesh to be built before the second one. But the reason I did it like this is that both meshes need to have completed before the lfric2lfric task. And the logic for ( mesh1 and mesh2 and lfric2lfric build) => lfric2lfric seemed too complicated. So I decided the mesh1 => mesh2 => lfric2lfric would solve this requirement. |
PR Summary
App tasks (e.g. lfric_atm) specify which mesh to run with (in the
resolutionsetting). A mesh task is then created to create that mesh. But this only allows for one mesh to be created per app task. lfric2lfric requires 2 meshes, and so this PR allows for more multiple meshes to be created per task.The lfric2lfric tasks are also reorganised so that there will be 3 main types of configuration file:
rose-app-src_mesh_levels.conf(the source data e.g. data filename, mesh, vertical levels - all this info is kept together as the data will always be for a particular mesh and levels )rose-app-dst_mesh.conf(the destination mesh)rose-app-dst_levels.conf(the destination vertical levels - this is separate to the mesh as they are not necessarily together)At the moment only horizontal regridding is applied, so there are no dst_levels configurations on this PR.
The reorganisation of the configs means that some of the
fail-ifs in the rose-meta have needed to be removed so that thevalidate rose-metatask suceeds.Sci/Tech Reviewer: Mike Hobson (@mike-hobson)
Code Reviewer: James Bruten (@james-bruten-mo)
This makes it a little easier to add new tasks, and avoids assumed dependencies on other tasks e.g. at the moment some lfric2lfric tasks are using meshes created by other tasks. This might cause failures if those meshes are not created in time or if the associated tasks are removed.
Single mesh (including multigrid) tasks (as current implementation):
Multiple-mesh (double or more) tasks (new):
To enable this, the following files are changed.
rose-stem/templates/graph/populate_gen_weights_graph.cylc
• Lfric2lfric weight generation needs to use 2 meshes rather than 1.
• This is setup as:
build_mesh => run_mesh1 => run_mesh2 => generate_weightsrose-stem/templates/graph/populate_graph_sections.cylc
• Lfric2lfric regridding needs to use 2 meshes rather than 1.
• This is set up as:
(This assumes that building the application takes longer than building and creating the meshes, as also assumed currently).
rose-stem/templates/runtime/generate_runtime_application.cylc
• Remove adding the mesh .conf file to the lfric2lfric task options (its only needed to generate the meshes)
blocks #660
This is a small part split from #530
Code Quality Checklist
Testing
trac.log
Test Suite Results - lfric_apps - multiple_meshes/run5
Suite Information
Task Information
✅ succeeded tasks - 1592
Security Considerations
Performance Impact
AI Assistance and Attribution
Documentation
PSyclone Approval
Sci/Tech Review
(Please alert the code reviewer via a tag when you have approved the SR)
Code Review