api: fix handling of multiple conditions for buffering by mloubout · Pull Request #2850 · devitocodes/devito

mloubout · 2026-02-16T18:55:59Z

No description provided.

codecov · 2026-02-16T19:13:17Z

Codecov Report

❌ Patch coverage is 85.61873% with 43 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.49%. Comparing base (cf29367) to head (5ac1c04).
⚠️ Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
devito/passes/clusters/buffering.py	75.00%	20 Missing and 11 partials ⚠️
devito/ir/support/guards.py	85.29%	4 Missing and 1 partial ⚠️
devito/passes/clusters/asynchrony.py	86.11%	3 Missing and 2 partials ⚠️
devito/ir/clusters/algorithms.py	77.77%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2850      +/-   ##
==========================================
+ Coverage   83.45%   83.49%   +0.03%     
==========================================
  Files         249      249              
  Lines       52219    52446     +227     
  Branches     4500     4543      +43     
==========================================
+ Hits        43580    43790     +210     
- Misses       7880     7891      +11     
- Partials      759      765       +6

Flag	Coverage Δ
pytest-gpu-aomp-amdgpuX	`68.61% <73.51%> (+<0.01%)`	⬆️
pytest-gpu-gcc-	`78.19% <75.58%> (-0.01%)`	⬇️
pytest-gpu-icx-	`78.15% <75.58%> (+0.05%)`	⬆️
pytest-gpu-nvc-nvidiaX	`69.13% <73.51%> (-0.01%)`	⬇️

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

☔ View full report in Codecov by Harness.
📢 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.

FabioLuporini · 2026-02-20T08:56:53Z

        return CondNe(*self.args, evaluate=False)

+    @property
+    def _as_min(self):


I would drop this and rather have a singledispatch handler for CondEq where necessary

FabioLuporini · 2026-02-20T08:57:31Z

+
+def relational_shift(expr, s):
+    """
+    Infer shift incurred by the expression. Generally only


I could use an example here to quickly visualise what's it trying to do

FabioLuporini · 2026-02-20T08:58:34Z

-            expr = uxreplace(expr, {d: IntDiv(index, d.symbolic_factor)})
+
+        # Merge conditionals when possible. E.g if we have an implicit_dim
+        # and there is a dimension with the same parent, we ca merged


Dimension

"ca merged"

"their conditions"

you could also make the example a bit more practical

FabioLuporini · 2026-02-20T08:59:22Z

+        for d in input_expr.implicit_dims:
+            if d not in conditionals:
+                continue
+            for cd in dict(conditionals):


list(...) is fine

FabioLuporini · 2026-02-20T09:05:57Z

+        # Replace the ConditionalDimensions in `expr`
+        for d, cond in conditionals.items():
+            # Replace dimension with index
+            index = d.index


you can spare this line

FabioLuporini · 2026-02-20T09:07:18Z

        ispace = IterationSpace(intervals, iterators)

-        # Construct the conditionals and replace the ConditionalDimensions in `expr`
+        # Construct the conditionals


I think we should place this whole block of code, which constructs/lowers the conditionals, into its own separate functions, and a docstring with some examples

review-notebook-app · 2026-05-28T17:17:37Z

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

FabioLuporini · 2026-05-30T13:11:27Z

+            shift = relational_shift(cond, d.parent)
+            expr = uxreplace(expr, {d: IntDiv(index, d.symbolic_factor) + shift})
+
+        # Merge conditionals when possible. E.g if we have an implicit_dim


btw this block imho deserves its own function

FabioLuporini · 2026-05-30T13:12:30Z

            if d is not dim:
                continue
-
+            if d in c0.guards and not c0.guards[d].has(Mod):


searching for Mod is a bit meh, I'd rather add a special guard to ir/support/guards.py and look for that instead (there's quite a few already in there!)

FabioLuporini · 2026-05-30T13:12:51Z

            _actions_from_update_memcpy(c, d, clusters, actions, sregistry)
        elif d.is_Custom and is_integer(c.ispace[d].size):
-            _actions_from_init(c, d, actions)
+            _actions_from_init(c, d, clusters, actions)


leftover, I guess

FabioLuporini · 2026-05-30T13:12:55Z



-def _actions_from_init(c, d, actions):
+def _actions_from_init(c, d, clusters, actions):


leftover, I guess

FabioLuporini · 2026-05-30T13:15:54Z

+      "\n",
+      "    * ``And`` (default): all conditions must hold (mutually exclusive merging).\n",
+      "    * ``Or``: any one condition is enough for the if-branch to fire.\n",
+      "    * ``'strict'``: this condition takes precedence over every other condition\n",


how about an int, representing priority, instead of strict? it would also make it more natural to generalise it

mloubout added the API api (symbolics, types, ...) label Feb 16, 2026

FabioLuporini reviewed Feb 20, 2026

View reviewed changes

mloubout force-pushed the multi-cond-again branch 4 times, most recently from ef708e5 to b997156 Compare May 22, 2026 15:13

mloubout force-pushed the multi-cond-again branch 5 times, most recently from 7a1a6aa to c7786ea Compare May 28, 2026 17:17

mloubout force-pushed the multi-cond-again branch 11 times, most recently from f904760 to 0500469 Compare May 29, 2026 16:45

FabioLuporini reviewed May 30, 2026

View reviewed changes

mloubout force-pushed the multi-cond-again branch from 0500469 to 30790f0 Compare May 30, 2026 14:35

mloubout force-pushed the multi-cond-again branch 15 times, most recently from 550e222 to e315b38 Compare June 4, 2026 13:39

mloubout added 5 commits June 4, 2026 09:42

compiler: fix multi-cond async

c218cbe

compiler: fix multi-cond for multi-layer

5e0e8d5

api: fix interpolate with complex dtype

91b64c6

api: fix buffering with multiple conditions

fb350a8

compiler: fix various corner case of multi buffering

45c8a8e

mloubout force-pushed the multi-cond-again branch 4 times, most recently from 797592b to ab5242a Compare June 5, 2026 15:45

compiler: support mutli-buffering

5ac1c04

mloubout force-pushed the multi-cond-again branch from ab5242a to 5ac1c04 Compare June 5, 2026 17:37



		def _actions_from_init(c, d, actions):
		def _actions_from_init(c, d, clusters, actions):

Conversation

mloubout commented Feb 16, 2026

Uh oh!

codecov Bot commented Feb 16, 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!

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!

review-notebook-app Bot commented May 28, 2026

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov Bot commented Feb 16, 2026 •

edited

Loading