compiler: fix halo placement for non out dim exchange by mloubout · Pull Request #2892 · devitocodes/devito

mloubout · 2026-04-21T20:17:16Z

No description provided.

codecov · 2026-04-21T20:41:39Z

Codecov Report

❌ Patch coverage is 50.00000% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.46%. Comparing base (d9dd186) to head (076af37).
⚠️ Report is 5 commits behind head on main.

Files with missing lines	Patch %	Lines
tests/test_mpi.py	11.76%	15 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2892      +/-   ##
==========================================
- Coverage   83.48%   83.46%   -0.02%     
==========================================
  Files         248      248              
  Lines       51551    51570      +19     
  Branches     4437     4441       +4     
==========================================
+ Hits        43038    43045       +7     
- Misses       7761     7774      +13     
+ Partials      752      751       -1

Flag	Coverage Δ
pytest-gpu-aomp-amdgpuX	`68.77% <92.30%> (+0.01%)`	⬆️
pytest-gpu-gcc-	`78.06% <50.00%> (-0.04%)`	⬇️
pytest-gpu-icx-	`77.69% <50.00%> (-0.03%)`	⬇️
pytest-gpu-nvc-nvidiaX	`69.32% <92.30%> (+0.04%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

EdCaunt · 2026-04-22T12:48:46Z

+        np.random.seed(0)
+        v = TimeFunction(name="v", grid=grid, space_order=4,
+                         time_order=1, save=Buffer(1))
+        v.data[:] = np.random.randn(*grid.shape)


Would it be worth using a sinusoidal function here and comparing to the analytical solution? Would need a bigger grid however, and the existing check is already pretty strong

not a big fan of randn either

It's random with a seed. I tend to prefer random values for tests, forces things to be more stable but can change it if makes people uncomfortable

FabioLuporini · 2026-04-23T08:46:12Z

                        dims.add(ai.parent)
                    else:
                        dims.add(ai)
+                elif ai is not None and not ai._defines & set(f.dimensions):


most importantly, I don't understand why the prior if wouldn't catch this? if ai is rrecx and it's being used to index into say x, then d should be x and f.grid.is_distributed(d) in the condition above should have returned True already... no? what am I missing here?

That's from when i tried to fix is_distributed to actually check if it's decomposed but it broke a lot of stuff so had to revert it. But right now is_distributed is meaningless since it always returns true.

FabioLuporini · 2026-04-23T08:47:07Z

            fmapper[f] = fmapper.get(f, hse).merge(hse)
        return HaloScheme.build(fmapper, self.honored)

+    def _is_iter_carried(self, scope):


why did you hoist this? it doesn't feel right that data dependence analysis belong to HaloScheme, but again, I may be missing something

I moved it here because I was checking it it would be enough to use in the communication pass. There is no strong reason to but it makes it potentially esier to use if needed in a different cluster pass instead of having to improt it form iet

FabioLuporini · 2026-04-23T08:50:08Z

+    for f in hs.fmapper:
+        for a in c.scope.getwrites(f):
+            is_write = set(a.access.indices) & loc_vals
+            is_dist = any(c.grid.distributor.topology.get(d, 1) > 1


any(f.grid.is_distributed(d) for d in hs.dimensions)

NOTE: f. and not c. is slightly more robust because not all Clusters may have an underlying Grid

No that the main issue is_distributed is useless as it always returns true for a grid dimension irrespective of it it's decomposed or not

FabioLuporini · 2026-04-23T08:53:18Z

+    for f in hs.fmapper:
+        for a in c.scope.getwrites(f):
+            is_write = set(a.access.indices) & loc_vals
+            is_dist = any(c.grid.distributor.topology.get(d, 1) > 1


you can hoist this condition within the f loop and return False immediately if it doesn't hold;

then you can turn the current is_write condition into a one-liner if any(set(...) for a in ...)

FabioLuporini · 2026-04-23T08:55:21Z

-    @pytest.mark.skipif(TestTTI is None, reason="Requires installing the tests")
    @pytest.mark.parallel(mode=1)
    def test_halo_structure(self, mode):
+        from test_dse import TestTTI


from .test_dse import TestTTI

or it doesn't matter? I will never remember...

I think both work? THis is just moved from a top import becuase somehow importing it at the top runs all the tests in TestTTI at import

FabioLuporini · 2026-04-23T08:57:11Z

+        np.random.seed(0)
+        v = TimeFunction(name="v", grid=grid, space_order=4,
+                         time_order=1, save=Buffer(1))
+        v.data[:] = np.random.randn(*grid.shape)


not a big fan of randn either

FabioLuporini

LGTM

mloubout added MPI mpi-related compiler labels Apr 21, 2026

mloubout force-pushed the halo-comm-order branch from 27a8ff6 to 5178529 Compare April 22, 2026 01:21

mloubout changed the title ~~compiler: fix halo placement for non out dimm exchange~~ compiler: fix halo placement for non out dim exchange Apr 22, 2026

mloubout force-pushed the halo-comm-order branch 4 times, most recently from b802a82 to b4d0594 Compare April 22, 2026 04:04

EdCaunt reviewed Apr 22, 2026

View reviewed changes

mloubout force-pushed the halo-comm-order branch 6 times, most recently from 3a8c97a to ca423ac Compare April 23, 2026 02:25

FabioLuporini reviewed Apr 23, 2026

View reviewed changes

mloubout force-pushed the halo-comm-order branch from ca423ac to f3fcfaa Compare April 23, 2026 11:52

mloubout added 2 commits April 23, 2026 07:53

compiler: fix halo placement for non out dimm exchange

7633a1a

compiler: refine distributed dimension check for smarter halotouch

86fa36f

mloubout force-pushed the halo-comm-order branch from f3fcfaa to 86fa36f Compare April 23, 2026 11:53

compiler: switch to better halo placement

013bb51

mloubout force-pushed the halo-comm-order branch from 8a227ea to 595e27c Compare April 23, 2026 17:59

compiler: fix terminal detection from dynamic dim bounds

076af37

mloubout force-pushed the halo-comm-order branch from 595e27c to 076af37 Compare April 23, 2026 17:59

FabioLuporini approved these changes Apr 23, 2026

View reviewed changes

mloubout merged commit 8d1995d into main Apr 23, 2026
42 checks passed

mloubout deleted the halo-comm-order branch April 23, 2026 23:54

Conversation

mloubout commented Apr 21, 2026

Uh oh!

codecov Bot commented Apr 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

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!

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!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

FabioLuporini left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov Bot commented Apr 21, 2026 •

edited

Loading