Allow applications partitioning meshes to specify partition constraints (imposed by multigrid) by mo-rickywong · Pull Request #290 · MetOffice/lfric_core

Ricky Wong (mo-rickywong) · 2026-02-27T13:17:47Z

PR Summary

Sci/Tech Reviewer: Juan Manuel Castillo Sanchez (@ukmo-juan-castillo)
Code Reviewer: Mike Hobson (@mike-hobson)

ticket:4630 relocated checks/code-path for constraints on partitions when using MultiGrid within the the partitioners. This is a check that the partitioner (or infrastructure) should not be performing (it's a Gungho Science feature).

The contraints on multigrid are being triggered if the mesh file from which a mesh was read, contained intergrid maps, regardless of whether the application had any interest in multigrid (e.g. Lfric2Lfric re-gridding). This change provides logic for applications to specify whether they are concerned with "Multigrid" constraints.

Ideally, the checks should be relocated to Gungho Science. This change achieves the desired outcome without compounding ticket:4630 complexity.

Linked PRs

Update App calls to partition type api lfric_apps#324

Blocking PRs

is blocked-by Lfric2lfric multi cpu lfric_apps#201

Code Quality Checklist

I have performed a self-review of my own code
My code follows the project's style guidelines
Comments have been included that aid understanding and enhance the readability of the code
My changes generate no new warnings
All automated checks in the CI pipeline have completed successfully

Testing

I have tested this change locally, using the LFRic Core rose-stem suite
If required (e.g. API changes) I have also run the LFRic Apps test suite using this branch
If any tests fail (rose-stem or CI) the reason is understood and acceptable (e.g. kgo changes)
I have added tests to cover new functionality as appropriate (e.g. system tests, unit tests, etc.)
Any new tests have been assigned an appropriate amount of compute resource and have been allocated to an appropriate testing group (i.e. the developer tests are for jobs which use a small amount of compute resource and complete in a matter of minutes)

All rose-stem tests run green. without issue.

Security Considerations

I have reviewed my changes for potential security issues
Sensitive data is properly handled (if applicable)
Authentication and authorisation are properly implemented (if applicable)

Performance Impact

Performance of the code has been considered and, if applicable, suitable performance measurements have been conducted

AI Assistance and Attribution

Some of the content of this change has been produced with the assistance of Generative AI tool name (e.g., Met Office Github Copilot Enterprise, Github Copilot Personal, ChatGPT GPT-4, etc) and I have followed the Simulation Systems AI policy (including attribution labels)

Documentation

Where appropriate I have updated documentation related to this change and confirmed that it builds correctly

PSyclone Approval

If you have edited any PSyclone-related code (e.g. PSyKAl-lite, Kernel interface, optimisation scripts, LFRic data structure code) then please contact the TCD Team

Sci/Tech Review

I understand this area of code and the changes being added
The proposed changes correspond to the pull request description
Documentation is sufficient (do documentation papers need updating)
Sufficient testing has been completed

(Please alert the code reviewer via a tag when you have approved the SR)

Code Review

All dependencies have been resolved
Related Issues have been properly linked and addressed
CLA compliance has been confirmed
Code quality standards have been met
Tests are adequate and have passed
Documentation is complete and accurate
Security considerations have been addressed
Performance impact is acceptable

Juan Manuel Castillo Sanchez (ukmo-juan-castillo) · 2026-03-05T15:00:16Z

I run the lfric_core rose stem tests successfully, everything seems in order:

Test Suite Results - lfric_core - EnforcePartitionRestrictions/run1

Suite Information

Item	Value
Suite Name	EnforcePartitionRestrictions/run1
Suite User	juan.m.castillo
Workflow Start	2026-03-05T14:18:13
Groups Run	all

Dependency	Reference	Main Like
lfric_core	mo-rickywong/lfric_core@EnforcePartitionRestrictions	False
SimSys_Scripts	MetOffice/SimSys_Scripts@2025.12.1	True

Task Information

✅ succeeded tasks - 384

Juan Manuel Castillo Sanchez (ukmo-juan-castillo)

Everything seems in order, just a minor comment

Juan Manuel Castillo Sanchez (ukmo-juan-castillo) · 2026-03-05T14:47:23Z

    ! Equal divisions are only required if there are maps between meshes
-    if (any_maps) then
+    if (check_constraints) then
+      call log_event("Performing partition constraints.", log_level_debug)


The print line should probably be: 'Checking partition constraints'

Juan Manuel Castillo Sanchez (ukmo-juan-castillo)

Since the last time I looked at this code, the only thing that has been done has been updating to head of the trunk and addressing my comments. Rose stem tests also pass successfully. I am more than happy to approve.

Mike Hobson (mike-hobson)

I have just a few comments that need some attention.

Mike Hobson (mike-hobson) · 2026-04-07T12:00:00Z

  !> @param[in]    num_cells_y      The panel's size in the y direction
-  !> @param[in]    any_maps         Whether there exist maps between meshes that
-  !>                                must having aligning partitions
+  !> @param[in]    check_constaints Check meshes have aligned partitions


"constaint" -> "constraint". But that will mess up the alignment of the other comments - so you may need to sort that, too. A search for "constaint" reveals 6 other mis-spellings in comments that need correcting, along with their associated alignments.

Mike Hobson (mike-hobson) · 2026-04-07T12:37:02Z

                              generate_inner_halos,    &
-                              partitioner_ptr )
+                              partitioner_ptr,         &
+                              enforce_constraints )


There are no Doxygen comments for the new argument: enforce_constraints. Please can you add some

Mike Hobson (mike-hobson) · 2026-04-07T12:51:37Z

    ! Equal divisions are only required if there are maps between meshes
-    if (any_maps) then
+    if (check_constraints) then
+      call log_event("Checking partition constraints.", log_level_debug)


Do we really need another debug log message for this?

Mike Hobson (mike-hobson) · 2026-04-07T13:35:06Z

                                      max_stencil_depth, &
                                      mapping_factor, &
                                      generate_inner_halos, &
+                                      enforce_constraints, &


enforce_constraints needs a Doxygen comment.

Mike Hobson (mike-hobson) · 2026-04-07T13:38:41Z

                                  decomposition, &
                                  max_stencil_depth, &
                                  generate_inner_halos, &
+                                  enforce_constraints, &


Again, enforce_constraints needs a Doxygen comment.

Mike Hobson (mike-hobson) · 2026-04-07T13:57:46Z

                                             max_stencil_depth,     &
                                             mapping_factor,        &
                                             generate_inner_halos,  &
+                                             enforce_constraints,   &


Doxygen comment

Mike Hobson (mike-hobson)

Thanks for those changes. They all look good, now. I'm happy to approve this for merging.

Commit changes to allow control of partition constraints

3cf8da1

github-actions Bot assigned Ricky Wong (mo-rickywong) Feb 27, 2026

This was referenced Feb 27, 2026

Select partitioning of multigrid meshes #288

Closed

Update App calls to partition type api MetOffice/lfric_apps#324

Merged

github-actions Bot requested a review from Juan Manuel Castillo Sanchez (ukmo-juan-castillo) March 4, 2026 15:06

Juan Manuel Castillo Sanchez (ukmo-juan-castillo) reviewed Mar 5, 2026

View reviewed changes

This was referenced Mar 5, 2026

Lfric2lfric no multi partition MetOffice/lfric_apps#291

Closed

Decide if partitioning all meshes in a multigrid mesh file #289

Closed

Ricky Wong (mo-rickywong) added 3 commits March 24, 2026 12:13

Merge branch 'main' into EnforcePartitionRestrictions

768d513

Fix merge issue

646940e

Update comment

96ce428

Ricky Wong (mo-rickywong) marked this pull request as ready for review March 25, 2026 14:41

Ricky Wong (mo-rickywong) requested review from a team and Mike Hobson (mike-hobson) as code owners March 25, 2026 14:41

Ricky Wong (mo-rickywong) requested a review from Juan Manuel Castillo Sanchez (ukmo-juan-castillo) March 25, 2026 14:43

Juan Manuel Castillo Sanchez (ukmo-juan-castillo) approved these changes Mar 27, 2026

View reviewed changes

Mike Hobson (mike-hobson) requested changes Apr 15, 2026

View reviewed changes

Updates in response to Code review comments

4bc7800

Ricky Wong (mo-rickywong) requested review from Mike Hobson (mike-hobson) April 20, 2026 14:06

Mike Hobson (mike-hobson) approved these changes Apr 21, 2026

View reviewed changes

Merge branch 'main' into EnforcePartitionRestrictions

0ecacab

Mike Hobson (mike-hobson) approved these changes Apr 22, 2026

View reviewed changes

Mike Hobson (mike-hobson) merged commit a963e38 into MetOffice:main Apr 22, 2026
7 checks passed

James Bruten (james-bruten-mo) added this to the Summer 2026 milestone May 29, 2026

Navigation Menu

Allow applications partitioning meshes to specify partition constraints (imposed by multigrid) - #290

Allow applications partitioning meshes to specify partition constraints (imposed by multigrid)#290
Mike Hobson (mike-hobson) merged 6 commits into
MetOffice:mainfrom
mo-rickywong:EnforcePartitionRestrictions

Ricky Wong (mo-rickywong) commented Feb 27, 2026 •

edited

Loading

Uh oh!

Juan Manuel Castillo Sanchez (ukmo-juan-castillo) commented Mar 5, 2026

Uh oh!

Juan Manuel Castillo Sanchez (ukmo-juan-castillo) left a comment

Uh oh!

Juan Manuel Castillo Sanchez (ukmo-juan-castillo) Mar 5, 2026

Uh oh!

Ricky Wong (mo-rickywong) Mar 24, 2026

Uh oh!

Juan Manuel Castillo Sanchez (ukmo-juan-castillo) left a comment

Uh oh!

Mike Hobson (mike-hobson) left a comment

Uh oh!

Mike Hobson (mike-hobson) Apr 7, 2026

Uh oh!

Mike Hobson (mike-hobson) Apr 7, 2026

Uh oh!

Mike Hobson (mike-hobson) Apr 7, 2026

Uh oh!

Mike Hobson (mike-hobson) Apr 7, 2026

Uh oh!

Mike Hobson (mike-hobson) Apr 7, 2026

Uh oh!

Mike Hobson (mike-hobson) Apr 7, 2026

Uh oh!

Mike Hobson (mike-hobson) left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

Ricky Wong (mo-rickywong) commented Feb 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

Linked PRs

Blocking PRs

Code Quality Checklist

Testing

Security Considerations

Performance Impact

AI Assistance and Attribution

Documentation

PSyclone Approval

Sci/Tech Review

Code Review

Uh oh!

Juan Manuel Castillo Sanchez (ukmo-juan-castillo) commented Mar 5, 2026

Test Suite Results - lfric_core - EnforcePartitionRestrictions/run1

Suite Information

Task Information

Uh oh!

Juan Manuel Castillo Sanchez (ukmo-juan-castillo) left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Juan Manuel Castillo Sanchez (ukmo-juan-castillo) left a comment

Choose a reason for hiding this comment

Uh oh!

Mike Hobson (mike-hobson) left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mike Hobson (mike-hobson) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Ricky Wong (mo-rickywong) commented Feb 27, 2026 •

edited

Loading