Offline partitioning of mesh in weekly tests - #688
Offline partitioning of mesh in weekly tests#688Matthew Walker (mattatmet) wants to merge 20 commits into
Conversation
iboutle
left a comment
There was a problem hiding this comment.
A couple of minor comments
iboutle
left a comment
There was a problem hiding this comment.
Thanks - looks good to me now!
James Bruten (james-bruten-mo)
left a comment
There was a problem hiding this comment.
rose-stem changes look good
mo-marqh
left a comment
There was a problem hiding this comment.
minor task and group changes requested, to maintain usage consistency
Thanks for the comments mo-marqh, I've made those changes and pushed them. Cheers. |
|
i have found a couple of further issues that i think require some thought. i tried running this code using a custom decomposition task, alongside another task: && this fails in 2 different ways:
|
|
noting that i tried to fix
but this fails on a single task. Mesh generation runs, and makes many mesh files, but not all, and fails with `Program received signal SIGSEGV: Segmentation fault - invalid memory reference.`Using these code additions to the config: custom partitioning patch code |
|
I suggest we provide a multi-partition test configuration within the test suite, using I suggest a group, containing four tasks:
This should result in 2 different meshes being created and each patched into the two relevant tasks. The mesh creation within the This group can be added to the |
No particular objection to this, but I'm not really sure what this is adding above the C224_MG_op test that we're already running in the weekly group - what is the difference between run1 and run2 here? |
|
Just a note here as well - #702 went on main yesterday. When this is merged up to the head of main, please ensure all references to GAL9 are changed to GAL10 in any task names |
It seems useful to me to ensure that the So, it could be useful to use This might make the test code cleaner |
Thanks mo-marqh - I'm still struggling to follow exactly what you're wanting to test here though. If it's the generation of the OP meshes, then that should be done as a task in the mesh generation app, rather than hijacking lfric_atm. |
that's fair iboutle I think it could be useful for Matthew Walker (@mattatmet) to have this use case to code against, but then it can likely be removed once it is working, rather than adding to the |
| {% set mesh_run_task = "run_mesh_"~ | ||
| task_values["resolution"]~"_"~ | ||
| task_values["mpi_parts"]~"P_"~ | ||
| task_values["panel_decomp"]~"_"~ | ||
| task_values["xproc"]~"X_"~ | ||
| task_values["yproc"]~"Y_"~ | ||
| task_ns.platform~"_"~ | ||
| site_vars["mesh_build"][task_ns.platform] %} |
There was a problem hiding this comment.
Thanks for sorting this, it looks good. A couple of thoughts,
| {% set mesh_run_task = "run_mesh_"~ | |
| task_values["resolution"]~"_"~ | |
| task_values["mpi_parts"]~"P_"~ | |
| task_values["panel_decomp"]~"_"~ | |
| task_values["xproc"]~"X_"~ | |
| task_values["yproc"]~"Y_"~ | |
| task_ns.platform~"_"~ | |
| site_vars["mesh_build"][task_ns.platform] %} | |
| {% set mesh_run_task = "run_mesh_"~ | |
| task_values["resolution"]~"-"~ | |
| task_values["mpi_parts"]~"P-"~ | |
| task_values["panel_decomp"]~"-"~ | |
| task_values["xproc"]~"X-"~ | |
| task_values["yproc"]~"Y_"~ | |
| task_ns.platform~"_"~ | |
| site_vars["mesh_build"][task_ns.platform] %} |
Could we split these sections by "-" rather than "_".
I also wonder whether it would be better to save this string as a new variable in the task_values dictionary, and then reading that in the 2 generate runtime files, rather than having this line of code repeated?
There was a problem hiding this comment.
Thanks for these comments James, I've made some changes, let me know what you think.
Just thinking more about this - it would probably be safe to make this change on the branch now. Each task is a new task, so naming each new task as gal10 instead of gal9 would be consistent with what we want at the head of main, will work as-is, and should merge cleanly with the head of main... |
James Bruten (james-bruten-mo)
left a comment
There was a problem hiding this comment.
Thanks Matt, looks good
|
Update: after looking into the seg fault Mark found with one of the custom partitioning jobs, I think I narrowed it down to a race condition in the mesh generator that was only triggered by very high partition numbers. I've addressed that in core and linked the PR (MetOffice/lfric_core#450) to this one. |
mo-marqh
left a comment
There was a problem hiding this comment.
many thanks Matthew Walker (@mattatmet)
this PR now addresses all of the initial intent and the scope creep from review
Minimal new testing is added, with only 1 new test, a C224 with offline partitioning, whilst existing tests are adapted.
This change passes Sci/Tech review
|
ready for code review Benjamin Went (@MetBenjaminWent) |
PR Summary
Sci/Tech Reviewer: mo-marqh
Code Reviewer: Benjamin Went (@MetBenjaminWent)
Overview
Issue #575 details the problems being faced by large ensembles of lfric_atm jobs with large rank counts. The main issue is the initialisation of the model which requires significant amounts of data, putting strain on the parallel file system. A component of this problem is the initialisation of the mesh which currently requires each rank to read in a single mesh file. When each rank of each ensemble is reading the same file at approximately the same time it can put significant strain on metadata servers. Prepartitioning the mesh in advance such that each rank is reading a different file can help to alleviate this problem.
The infrastructure for prepartioning and using prepartioned meshes already exists in the code. This PR makes use of this by integrating this functionality into the rose-stem
lfric_atm_ex1a_weeklytesting suite through the addition of two new workflows. Each uses the mesh task to prepartition the mesh for C224 and C896 resolutions.To make use of the OpenMP parallelism in the
cubedsphere_mesh_generator.F90which prepartitions our mesh,mesh_cpusandmesh_memorytask values have been passed topopulate_graph_sections.cylc. These values can therefore be set in the task definition files, allowing tuning depending on mesh resolution.Performance Tests
For these tests, an additional timing calliper has been added around the mesh initialisation section of$12.34 \pm 0.13$ seconds and the later was $0.57 \pm 0.01$ seconds.
gungho_model_mod.F90. Using this timer with Vernier, we were able to show the performance improvement that this change allows. We ran four standard C896 lfric_atm jobs plus four lfric_atm jobs with a prepartitioned mesh (generated in the mesh step of the workflow). The former had an average Max Total time for the mesh initialisation calliper ofRelated Issues
Code Quality Checklist
Testing
trac.log
Test Suite Results - lfric_apps - pr_688_dev_apps/run2
Suite Information
Task Information
✅ succeeded tasks - 1216
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