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 57.07317% with 88 lines in your changes missing coverage. Please review.
✅ Project coverage is 53.07%. Comparing base (347d2c7) to head (30790f0).

Files with missing lines	Patch %	Lines
tests/test_buffering.py	9.75%	37 Missing ⚠️
devito/passes/clusters/buffering.py	72.17%	20 Missing and 12 partials ⚠️
devito/ir/equations/equation.py	52.94%	7 Missing and 1 partial ⚠️
devito/ir/clusters/algorithms.py	33.33%	3 Missing and 3 partials ⚠️
devito/types/relational.py	83.33%	2 Missing and 1 partial ⚠️
devito/passes/clusters/asynchrony.py	60.00%	1 Missing and 1 partial ⚠️

❗ There is a different number of reports uploaded between BASE (347d2c7) and HEAD (30790f0). Click for more details.

HEAD has 11 uploads less than BASE

Flag BASE (347d2c7) HEAD (30790f0)

18 10

pytest-gpu-gcc- 6 4

pytest-gpu-icx- 3 2

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #2850       +/-   ##
===========================================
- Coverage   83.38%   53.07%   -30.32%     
===========================================
  Files         248      248               
  Lines       51960    52065      +105     
  Branches     4480     4509       +29     
===========================================
- Hits        43326    27631    -15695     
- Misses       7879    23469    +15590     
- Partials      755      965      +210

Flag	Coverage Δ
pytest-gpu-aomp-amdgpuX	`68.58% <68.90%> (-0.06%)`	⬇️
pytest-gpu-gcc-	`43.20% <16.09%> (-34.90%)`	⬇️
pytest-gpu-icx-	`43.04% <16.09%> (-34.97%)`	⬇️
pytest-gpu-nvc-nvidiaX	`69.11% <68.90%> (-0.07%)`	⬇️

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.

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 7 times, most recently from 516047a to f354a19 Compare May 29, 2026 13:47

mloubout added 5 commits May 29, 2026 09:49

compiler: fix multi-cond async

5c778ca

compiler: fix multi-cond for multi-layer

8cbdc7f

api: fix interpolate with complex dtype

2e2cc4c

api: fix buffering with multiple conditions

fa27859

compiler: fix various corner case of multi buffering

9cc52bb

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

FabioLuporini reviewed May 30, 2026

View reviewed changes

compiler: support mutli-buffering

30790f0

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



		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