Add Halo Exchange to Dynamics by TomMelt · Pull Request #924 · nextsimhub/nextsimdg

TomMelt · 2025-09-09T14:04:11Z

Add Halo Exchange to Dynamics

Fixes #120

Task List

Linked an issue above that captures the requirements of this PR
~~Defined the tests that specify a complete and functioning change~~ will be done in separate follow up PR (see issue create idealised test for the MPI dynamics #1071
Implemented the source code change that satisfies the tests
Commented all code so that it can be understood without additional context
No new warnings are generated or they are mentioned below
The documentation has been updated (or an issue has been created to do so)
Relevant labels (e.g., enhancement, bug) have been applied to this PR
This change conforms to the conventions described in the README

Change Description

This PR adds halo exchange functionality to the dynamics and for advected fields.

After adding halo exchange into the dynamics we fix fields like hice for example.

Figure 1 hice (left) before halo exchange is added to dyanmics and (right) after.

After adding halo exchange into the advectField function we also fix it for advected fields like tbottom, tsurf and tinterior, for example.

Figure 2 tbottom (left) before halo exchange is added to advectField and (right) after.

Test Description

None currently

Documentation Impact

None currently

Other Details

⚠️ Open Problems:

AFAIK for Interpolations::DG2CG(smesh, cgVector, DGVector) values inside the "inner" domain (i.e., not halo cells) depends on vertices set outside the inner cells i.e., from neighbouring points on the lattice. This means that you have to halo exchange the DGVector before you halo exchange the CGVector. It is not equivalent to first interpolate from DGVector onto CGVector and then halo exchange the CGVector.
We still don't have a solution for how to set vertices outside the main domain (so inner halo boundaries are fine, but points along the edge are not (also periodic boundaries would be an issue)
When should halo exchange take place?
- @Thanduriel raised point that we perhaps need to exchange after dynamics rather than before
- @timspainNERSC think it should be before and possibly after
- @TomMelt thinks just before should be enough
- @einola either should work

Todo

add halo exchange automatically for advected fields
Need some scientific/statistical/sense tests to ensure correctness (raised in issue Create differential tests on output data prior to merging MPI and OpenMP solutions #247) -- perhaps an idealized test case? will be done in separate follow up PR (see issue create idealised test for the MPI dynamics #1071

Thanduriel · 2026-02-16T12:47:45Z

After looking at the code again I now believe that the update is correct as is. The exchange always happens right after a field is updated, ensuring that the correct results are available for subsequent computations.

The only exception are the advected fields in the dynamics that are also modified by the thermodynamics. Here we need to ensure that the halo cells contain the correct values before the advection, either by doing the column physics on the halo cells or by doing an exchange beforehand. Another exchange afterwards should not be strictly necessary but if we do another exchange, I think it might be possible to skip the exchange of stresses (s11,s12,s22), since that computation works purely local on individual cells .

Thanduriel

Looks correct to me as-is but I have a few thoughts on possible changes.

Thanduriel · 2026-02-17T08:10:49Z

+#ifdef USE_MPI
+            haloDGVector.exchangeHalos(s11);
+            haloDGVector.exchangeHalos(s12);
+            haloDGVector.exchangeHalos(s22);


I think this exchange is redundant. If all the inputs are correct on the halo cells the computed stress should be as well. Maybe you can try this? I expect the results to be bit-wise identical.

einola

We should modify advectDynamicsField as per my comment on line R132.

There are also questions about redundant exchanges.

einola · 2026-02-27T14:38:06Z

A possible test would be to run config_benchmark.cfg with one of the high-resolution setups (e.g., init_benchmark_256x256.nc) and simply check for conservation of volume. That is to say, to check if the total sum of hice in the domain is fixed.

This does not check the thermodynamics.

@joewallwork

# Add corner neighbours into halo exchange logic ### Task List - [x] Linked an issue above that captures the requirements of this PR - [x] Defined the tests that specify a complete and functioning change - [x] Implemented the source code change that satisfies the tests - [x] Commented all code so that it can be understood without additional context - [x] No new warnings are generated or they are mentioned below - [x] The documentation has been updated (or an issue has been created to do so) - [x] Relevant labels (e.g., enhancement, bug) have been applied to this PR - [x] This change conforms to the conventions described in the README --- # Change Description This PR has the following key changes: - adds support for corner neighbours to the existing halo exchange logic (which previously just exchanged edges) - add support for `CGVector` and `DGVectorHolder` exchange (this is required for corners and edges) - updates metadata tests and closed-boundary (CB) halo test to work with corner neighbours - removes the Periodic-Boundary (PB) halo exchange test for now, this will be addressed in a subsequent PR #1044 --- # Test Description ### `HaloExchangePB_test.cpp` `HaloExchangeCB_test.cpp` creates test data for all the array types: - `HField` - `VertexField` - `DGField` - `DGVector` - `CGVector` I modified the previous version, so that the model data are created in such a way that the value of each cell can be calculated based on its indices. Therefore we can check after exchange that all of the cells have been exchanged successfully. This also now allows us to change the number of ranks arbitrarily (assuming you also provide the appropriate `partition_metadata` file. ### `ModelMetadataCB_test.cpp` and `ModelMetadataPB_test.cpp` Small modifications have been made to the periodic and closed boundary `ModelMetadata` tests. The changes now add the additional corner neighbour metadata and check that it is ready correctly. ### `partition_metadata_3_cb.cdl` and `partition_metadata_3_pb.cdl` These files are now generated at compile time by running the `decomp` tool on a new file `halo_test_mask.cdl`. This is in line with similar changes introduced by @joewallwork on the XIOS tests. --- # Other Details * `domain_decop` git commit has now been bumped to the latest version on main. This version of the decomp tool contains the corner neighbour metadata. ### Further work - Integrating halo exchange to nextsim is handled by PR #924

Thanduriel

Looking good now!

What is the plan for the VP dynamics? Applying the changes from the BrittleCGDynamicsKernel shouldn't be much work now. I approve this regardless but if we don't do it right away, maybe create an issue?

TomMelt · 2026-04-13T09:17:08Z

Looking good now!

What is the plan for the VP dynamics? Applying the changes from the BrittleCGDynamicsKernel shouldn't be much work now. I approve this regardless but if we don't do it right away, maybe create an issue?

Thanks see issue #1072

TomMelt self-assigned this Sep 9, 2025

TomMelt added the ICCS Tasks or reviews for the ICCS team label Sep 9, 2025

TomMelt changed the base branch from develop to halo-exchange September 9, 2025 14:07

TomMelt requested a review from niravshah241 September 18, 2025 13:05

TomMelt force-pushed the halo-dynamics branch from 7bf17ca to f13e95c Compare September 30, 2025 13:15

Base automatically changed from halo-exchange to develop October 2, 2025 09:32

TomMelt force-pushed the halo-dynamics branch from f13e95c to c211ca0 Compare October 2, 2025 10:25

TomMelt force-pushed the halo-dynamics branch from c211ca0 to f0055d4 Compare October 14, 2025 09:41

github-advanced-security AI found potential problems Jan 5, 2026

View reviewed changes

Comment thread .github/workflows/build_docs.yml Fixed

TomMelt force-pushed the halo-dynamics branch from a15788e to 1559ff0 Compare January 6, 2026 09:16

Thanduriel mentioned this pull request Feb 10, 2026

Full GPU support #1040

Merged

TomMelt mentioned this pull request Feb 16, 2026

Halo add corners #1005

Merged

8 tasks

TomMelt force-pushed the halo-dynamics branch from 1559ff0 to 07afe13 Compare February 16, 2026 10:39

TomMelt changed the base branch from develop to halo-add-corners February 16, 2026 10:40

TomMelt requested review from Thanduriel, einola and timspainNERSC February 16, 2026 11:16

TomMelt changed the title ~~Halo dynamics~~ Add Halo Exchange to Dynamics Feb 16, 2026

TomMelt force-pushed the halo-dynamics branch from 4d7cb40 to 87dadeb Compare February 16, 2026 15:32

TomMelt marked this pull request as ready for review February 16, 2026 15:34

Thanduriel requested changes Feb 17, 2026

View reviewed changes

TomMelt added the enhancement New feature or request label Feb 23, 2026

einola requested changes Feb 27, 2026

View reviewed changes

Comment thread dynamics/src/include/BrittleCGDynamicsKernel.hpp Outdated

Comment thread dynamics/src/include/BrittleCGDynamicsKernel.hpp Outdated

Comment thread dynamics/src/include/BrittleCGDynamicsKernel.hpp Outdated

Comment thread dynamics/src/CGDynamicsKernel.cpp Outdated

Base automatically changed from halo-add-corners to develop March 16, 2026 10:28

TomMelt force-pushed the halo-dynamics branch 2 times, most recently from a951eb5 to c31114b Compare March 16, 2026 13:20

TomMelt changed the base branch from develop to fix-halo-constructor-dgvh March 16, 2026 13:20

Base automatically changed from fix-halo-constructor-dgvh to develop March 23, 2026 08:45

TomMelt force-pushed the halo-dynamics branch from 73dfa2f to 78918c7 Compare March 23, 2026 08:58

TomMelt commented Mar 23, 2026

View reviewed changes

Comment thread dynamics/src/include/DynamicsKernel.hpp Outdated

feat: add halo exchange into dynamics

fb6ad47

TomMelt force-pushed the halo-dynamics branch from 29eefc9 to fb6ad47 Compare March 23, 2026 16:19

TomMelt requested review from Thanduriel and einola April 13, 2026 08:17

Thanduriel approved these changes Apr 13, 2026

View reviewed changes

TomMelt mentioned this pull request Apr 13, 2026

create idealised test for the MPI dynamics #1071

Open

einola approved these changes Apr 13, 2026

View reviewed changes

TomMelt merged commit 04fa7c6 into develop Apr 13, 2026
9 checks passed

TomMelt deleted the halo-dynamics branch April 13, 2026 13:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Halo Exchange to Dynamics#924

Add Halo Exchange to Dynamics#924
TomMelt merged 1 commit intodevelopfrom
halo-dynamics

TomMelt commented Sep 9, 2025 •

edited

Loading

Uh oh!

Uh oh!

Thanduriel commented Feb 16, 2026 •

edited

Loading

Uh oh!

Thanduriel left a comment

Uh oh!

Uh oh!

Uh oh!

Thanduriel Feb 17, 2026

Uh oh!

einola left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

einola commented Feb 27, 2026

Uh oh!

Uh oh!

Thanduriel left a comment

Uh oh!

TomMelt commented Apr 13, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

TomMelt commented Sep 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Add Halo Exchange to Dynamics

Task List

Change Description

Test Description

Documentation Impact

Other Details

⚠️ Open Problems:

Todo

Uh oh!

Uh oh!

Thanduriel commented Feb 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Thanduriel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Thanduriel Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

einola left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

einola commented Feb 27, 2026

Uh oh!

Uh oh!

Thanduriel left a comment

Choose a reason for hiding this comment

Uh oh!

TomMelt commented Apr 13, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

TomMelt commented Sep 9, 2025 •

edited

Loading

Thanduriel commented Feb 16, 2026 •

edited

Loading