compiler: fix buffering with multiple conditions #2826

mloubout · 2026-01-16T15:16:51Z

Test in corresponding pro PR

codecov · 2026-01-16T15:48:24Z

Codecov Report

❌ Patch coverage is 85.71429% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.96%. Comparing base (ca12d81) to head (94dcaeb).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
devito/types/parallel.py	33.33%	1 Missing and 1 partial ⚠️
devito/ir/support/guards.py	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2826      +/-   ##
==========================================
- Coverage   78.98%   78.96%   -0.02%     
==========================================
  Files         248      248              
  Lines       50815    50824       +9     
  Branches     4391     4392       +1     
==========================================
- Hits        40134    40132       -2     
- Misses       9879     9890      +11     
  Partials      802      802

☔ 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.

devito/ir/support/guards.py

FabioLuporini · 2026-01-16T16:26:52Z

devito/passes/clusters/buffering.py

-            if c.scope.reads.get(self.f):
-                return c
-        return None
+            if c.scope.reads.get(self.f) and c.guards not in first_c:


this looks very fragile at first glance

How so? This just split the first read and last write per guard so that each different condition is respected

btw, you don't need the extra "and c.guards ..."

just do

first_c.setdefault(c.guards) = c

and, for homogeneity, could you call it just mapper?

Yes you do need the and, only the first cluster with the guard encountered is kept if you don't have the hand it'll replace it with the next one.

that's what the setdefault in my snippet is for -- to avoid that extra guard

FabioLuporini · 2026-01-16T16:27:11Z

devito/passes/clusters/buffering.py

-            if c.scope.writes.get(self.f):
-                return c
-        return None
+            if c.scope.writes.get(self.f) and c.guards not in last_c:


same as above

basically same comments above apply

EdCaunt · 2026-01-19T09:50:25Z

devito/passes/clusters/buffering.py

        func = vmax if self.is_forward_buffering else vmin
-        return func(*[Vector(i) for i in self.indices])[0]
+        for c in self.lastwrite:
+            idx = func(*[Vector(i) for i in extract_indices(self.f, self.dim, [c])])[0]


Would it be worth adding a method that wraps extract_indices to tidy this section up?

or at least hoist the extract_indices bit to the line above to improve readability

FabioLuporini · 2026-01-19T13:43:47Z

devito/types/parallel.py

-        if isinstance(npthreads, NPThreads):
-            npthreads = kwargs.get(npthreads.name, npthreads.size)
+        for th in search(npthreads, NPThreads):
+            npthreads = npthreads._subs(th, kwargs.get(th.name, th.size))


why subs?

Can be an expression, e.g nthreads0 + nthreads1 +.... And need to replace all

mh, what would the reproducer be?

The MFE in the pro PR

ah ok perfect

FabioLuporini · 2026-01-19T13:43:59Z

devito/types/parallel.py

        # If a symbolic object, it must be resolved
-        if isinstance(npthreads, NPThreads):
-            npthreads = kwargs.get(npthreads.name, npthreads.size)
+        for th in search(npthreads, NPThreads):


why search?

FabioLuporini · 2026-01-19T13:45:49Z

devito/passes/clusters/buffering.py

            * `time-1` in the case of `foo(u[time-1], u[time], u[time+1])`
               with a backwards-propagating `time` Dimension.
        """
+        last_idxs = {}


mapper or retval for homogeneity (ultra-nitpicking)

FabioLuporini · 2026-01-20T08:45:26Z

devito/passes/clusters/buffering.py

+            idx = func(*[Vector(i) for i in indices])[0]
+            mapper[c] = idx
+
+        return mapper


technically if you return a frozendict it'd be better but no big deal as long as we don't do crazy things at the caller site

I would suggest making it frozendict since it serves as a guard against cryptic failures caused by accidental slip-ups which mutate this mapper down the line

FabioLuporini

yes GTG thanks

mloubout added the compiler label Jan 16, 2026

mloubout force-pushed the buffering-multi-guards branch from 19bfb04 to e5dc2af Compare January 16, 2026 15:30

FabioLuporini reviewed Jan 16, 2026

View reviewed changes

mloubout force-pushed the buffering-multi-guards branch from e5dc2af to 93a78f7 Compare January 16, 2026 19:36

EdCaunt reviewed Jan 19, 2026

View reviewed changes

FabioLuporini reviewed Jan 19, 2026

View reviewed changes

mloubout force-pushed the buffering-multi-guards branch from 93a78f7 to 1a18c27 Compare January 20, 2026 04:15

FabioLuporini reviewed Jan 20, 2026

View reviewed changes

mloubout force-pushed the buffering-multi-guards branch from 1a18c27 to 71eb6f6 Compare January 20, 2026 11:45

compiler: fix buffering with multiple conditions

94dcaeb

mloubout force-pushed the buffering-multi-guards branch from 71eb6f6 to 94dcaeb Compare January 20, 2026 12:11

FabioLuporini approved these changes Jan 20, 2026

View reviewed changes

mloubout merged commit ca50531 into main Jan 20, 2026
42 checks passed

mloubout deleted the buffering-multi-guards branch January 20, 2026 16:39

compiler: fix buffering with multiple conditions #2826

compiler: fix buffering with multiple conditions #2826

Uh oh!

Conversation

mloubout commented Jan 16, 2026

Uh oh!

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

Uh oh!

Codecov Report

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!

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!

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!

FabioLuporini left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov bot commented Jan 16, 2026 •

edited

Loading