api: fix interp/eval of expressions by mloubout · Pull Request #2843 · devitocodes/devito · GitHub

No description provided.

Codecov Report

❌ Patch coverage is 97.40260% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.97%. Comparing base (676b8bf) to head (3fc5b91).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
devito/finite_differences/differentiable.py	96.07%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2843      +/-   ##
==========================================
- Coverage   78.97%   78.97%   -0.01%     
==========================================
  Files         248      248              
  Lines       50901    50936      +35     
  Branches     4395     4404       +9     
==========================================
+ Hits        40199    40226      +27     
- Misses       9904     9909       +5     
- Partials      798      801       +3

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

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

there's quite a few changes here, but not as many new tests covering them?

nitpicking: blank line

nitpicking: can just be getters=tuple(mapper)

why do we need these two lines? highest_priority(self).dimensions should already return it, or there's something non-consistent about it

Because a generic Differentiable does not necessarily has an _args_diff. For. example 1/h_x is a Differentiable but it doesn't have any argument that is Differentiable

but the property is still there, it's not that you are exposing yourself to an AttributeError... so (1/h_x)._args_diff will return the empty tuple and it should just be fine no?

An empty tuple doesn't have a .staggered or .dimension property. And on top of it (1/h_x)._args_diff will return 1/h_x which wiull be an infinite recursion

same story as before, I think

... unless I'm missing something

same

blank line

small letters diff_op as it's an instance

"an expression"

so the comment says "Mul or Add" but the dispatcher only targets Mul. Is it expected?

nitpicking: can just be getters=tuple(mapper)

nitpick: blank line

Reporting the type of expr here would make this error message more useful

Not sure what you mean the message is raise type(e) ... that's type already

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

mloubout force-pushed the stagg-encore branch 3 times, most recently from f2e36fa to 171969e Compare February 5, 2026 00:55

mloubout force-pushed the stagg-encore branch 2 times, most recently from 018e9d3 to 10eac67 Compare February 5, 2026 02:42

FabioLuporini reviewed Feb 5, 2026

View reviewed changes

EdCaunt reviewed Feb 5, 2026

View reviewed changes

api: fix interp/eval of expressions

3fc5b91

mloubout force-pushed the stagg-encore branch from 10eac67 to 3fc5b91 Compare February 5, 2026 18:28

mloubout merged commit 828497e into main Feb 6, 2026
42 checks passed

mloubout deleted the stagg-encore branch February 6, 2026 02:15

-                                                          for i in self._args_diff)))
+                      if not self._args_diff:
+                          return DimensionTuple()
+                      # Get indices of all args and merge them

+                      for a in self._args_diff:
+                          for d, i in a.indices.getters.items():
+                              mapper.setdefault(d, []).append(i)
+                      # Filter unique indices

+                      # Filter unique indices
+                      mapper = {k: v[0] if len(v) == 1 else tuple(filter_ordered(v))
+                                for k, v in mapper.items()}
+                      return DimensionTuple(*mapper.values(), getters=tuple(mapper.keys()))

                   def dimensions(self):
-                      return tuple(filter_ordered(flatten(getattr(i, 'dimensions', ())
-                                                          for i in self._args_diff)))
+                      if not self._args_diff:

               def highest_priority(DiffOp):
+                  if not DiffOp._args_diff:
+                      return DiffOp
                   # We want to get the object with highest priority

+                  @cached_property
+                  def staggered(self):
+                      if not self._args_diff:

		@@ -475,12 +487,19 @@ def has_free(self, *patterns):


		def highest_priority(DiffOp):

+              @interp_for_fd.register(Mul)
+              def _(expr, x0, **kwargs):
+                  # For a expression (e.g Mul or Add), we interpolate the whole expression

+                  # Split args between those that need interp and those that don't
+                  def test0(a):
+                      return all(a.indices[d] is i for d, i in x0.items() if d in a.dimensions)

-                              f"'{expr}' must be of type Differentiable, not {type(expr)}"
-                          ) from None
+                      except Exception as e:
+                          raise type(e)(f"Error while computing finite-difference for expr={expr}: "

api: fix interp/eval of expressions by mloubout · Pull Request #2843 · devitocodes/devito · GitHub

Uh oh!

api: fix interp/eval of expressions - #2843

api: fix interp/eval of expressions#2843mloubout merged 1 commit intomaindevitocodes/devito:mainfrom stagg-encoredevitocodes/devito:stagg-encoreCopy head branch name to clipboard

Conversation

mloubout commented Feb 4, 2026

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

review-notebook-app Bot commented Feb 5, 2026

Uh oh!

FabioLuporini left a comment

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!

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!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

api: fix interp/eval of expressions#2843
mloubout merged 1 commit into
mainfrom
stagg-encore

codecov Bot commented Feb 4, 2026 •
edited
Loading