api: Fix handling of staggering in injection/interpolation by mloubout · Pull Request #2936 · devitocodes/devito · GitHub

No description provided.

Codecov Report

❌ Patch coverage is 99.18831% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.39%. Comparing base (68e2f74) to head (551d050).

Files with missing lines	Patch %	Lines
devito/finite_differences/derivative.py	0.00%	1 Missing and 1 partial ⚠️
tests/test_interpolation.py	99.61%	1 Missing and 1 partial ⚠️
devito/types/sparse.py	96.55%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2936      +/-   ##
==========================================
+ Coverage   83.33%   83.39%   +0.06%     
==========================================
  Files         248      248              
  Lines       51799    51881      +82     
  Branches     4467     4479      +12     
==========================================
+ Hits        43165    43267     +102     
+ Misses       7882     7861      -21     
- Partials      752      753       +1

Flag	Coverage Δ
pytest-gpu-aomp-amdgpuX	68.70% <88.04%> (+0.03%)	⬆️
pytest-gpu-gcc-	78.09% <99.18%> (+0.06%)	⬆️
pytest-gpu-icx-	78.00% <99.18%> (+0.05%)	⬆️
pytest-gpu-nvc-nvidiaX	69.23% <88.04%> (+0.03%)	⬆️

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.

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

ultra-nitpick, fixable in a separate PR -- single `, not two, for homogeneity

We seem to be wildly inconsistent on this

when can u end up in the AttributeError? add a comment?

return empty tuple

I think you can do

g_fields, g_exprs = zip(*g)

instead of these two lines

cached_property

the fact that this had to be turned into a memoized_meth makes me think that it should better be placed in the interpolator classes instead.
Accepting shifts here doesn't make much sense imho.

I think I probably agree with this

This is the symbol for the sparse position. And it's not used only for interpolation, it's used for plain guards such as here that are used for guarding generic sparse expressions based on the position

same story, imho this method doesn't belong here anymore

It does, this is a SparseFunction information which is it's position relative to an origin. The interpolator uses it, it doesn't define it.

that's quite the change

This one inject the source into v_x which changes things quite a bit over long propagation (2 seconds here) since the source position is not properly nterpolated into the staggered location of v_x

I think I probably agree with this

Does this never need to know about the shifts then?

This is a guard for pure sparse operation, there is no staggered origin involved

How come this file has been so heavily reworked?

Just reorganized it was a pain to navigate it, it's 99% the same just lifted in classes

Figured it would be something like that. Amazing, thanks!

Note: These three tests are the only new ones in this file

mloubout added 2 commits May 20, 2026 09:38

api: fix interpolation with staggered

7562312

api: fix injection witjh staggering

6552d82

mloubout added the API api (symbolics, types, ...) label May 20, 2026

mloubout force-pushed the inject-interp branch from 3f39935 to 3405f6a Compare May 20, 2026 17:05

mloubout force-pushed the inject-interp branch from 3405f6a to 2001db0 Compare May 21, 2026 01:53

mloubout force-pushed the inject-interp branch 3 times, most recently from bc601dc to 74db52a Compare May 21, 2026 03:08

FabioLuporini reviewed May 21, 2026

View reviewed changes

EdCaunt reviewed May 21, 2026

View reviewed changes

mloubout commented May 21, 2026

View reviewed changes

mloubout force-pushed the inject-interp branch from 74db52a to b9b7bec Compare May 21, 2026 12:40

api: fix inject handling of different staggering

551d050

mloubout force-pushed the inject-interp branch from b9b7bec to 551d050 Compare May 21, 2026 12:49

FabioLuporini approved these changes May 22, 2026

View reviewed changes

EdCaunt approved these changes May 22, 2026

View reviewed changes

mloubout merged commit c71560b into main May 22, 2026
42 checks passed

mloubout deleted the inject-interp branch May 22, 2026 10:53

+                      try:
+                          staggered = field.staggered
+                      except AttributeError:
+                          return None

+                      try:
+                          staggered = field.staggered
+                      except AttributeError:
+                          return None

+                      except AttributeError:
+                          return None
+                      if not staggered or all(s == 0 for s in staggered):
+                          return None

+                      for _, g in groupby(pairs, lambda f: f[0].staggered):
+                          g = list(g)
+                          g_fields = [f for f, _ in g]
+                          g_exprs = [e for _, e in g]

-                  def _pos_symbols(self):
-                      return [Symbol(name=f'pos{d}', dtype=np.int32)
-                              for d in self.grid.dimensions]
+                  @memoized_meth

+                  def _field_shifts(self, field):
+                      """
+                      Per-grid-Dimension half-cell shift induced by ``field``'s staggering

+                  def _grid_map(self):
+                      return {}
+                  @property

-                  @cached_property
-                  def _position_map(self):
+                  @memoized_meth
+                  def _position_map(self, shifts=None):

-                  "assert np.isclose(norm(rec), 31.23, atol=0, rtol=1e-3)\n",
-                  "assert np.isclose(norm(rec2), 3.5482, atol=0, rtol=1e-3)\n",
-                  "assert np.isclose(norm(rec3), 4.7007, atol=0, rtol=1e-3)"
+                  "assert np.isclose(norm(rec), 29.538, atol=0, rtol=1e-3)\n",

                       # Position map and temporaries for it
-                      pmap = self._position_map
+                      pmap = self._position_map()

+                      '(x, y)', '(x, z)', '(y, z)',
+                      '(x, y, z)'
+                  ])
+                  def test_interpolate_staggered(self, stagg):

api: Fix handling of staggering in injection/interpolation by mloubout · Pull Request #2936 · devitocodes/devito · GitHub

Uh oh!

api: Fix handling of staggering in injection/interpolation - #2936

api: Fix handling of staggering in injection/interpolation#2936mloubout merged 3 commits intomaindevitocodes/devito:mainfrom inject-interpdevitocodes/devito:inject-interpCopy head branch name to clipboard

Conversation

mloubout commented May 20, 2026

Uh oh!

codecov Bot commented May 20, 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 May 21, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

EdCaunt May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

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!

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!

EdCaunt May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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 handling of staggering in injection/interpolation#2936
mloubout merged 3 commits into
mainfrom
inject-interp

codecov Bot commented May 20, 2026 •
edited
Loading

EdCaunt May 21, 2026 •
edited
Loading

EdCaunt May 22, 2026 •
edited
Loading