misc: Miscellaneous corner cases fixes #2730

mloubout · 2025-09-08T14:37:21Z

Corner cases triggered in pro by fft

EdCaunt

All seems reasonable. Typo in PR name "miscelanous" -> "miscellaneous"

codecov · 2025-09-08T14:55:46Z

Codecov Report

❌ Patch coverage is 86.66667% with 20 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.07%. Comparing base (379c77d) to head (edeb3b6).
⚠️ Report is 7 commits behind head on main.

Files with missing lines	Patch %	Lines
devito/passes/iet/languages/utils.py	66.66%	5 Missing and 1 partial ⚠️
devito/passes/clusters/buffering.py	82.60%	2 Missing and 2 partials ⚠️
tests/test_dtypes.py	90.90%	3 Missing and 1 partial ⚠️
devito/passes/iet/linearization.py	66.66%	3 Missing ⚠️
devito/mpi/routines.py	0.00%	2 Missing ⚠️
devito/arch/archinfo.py	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2730      +/-   ##
==========================================
+ Coverage   83.03%   83.07%   +0.03%     
==========================================
  Files         248      248              
  Lines       50499    50623     +124     
  Branches     4440     4460      +20     
==========================================
+ Hits        41930    42053     +123     
- Misses       7807     7808       +1     
  Partials      762      762

Flag	Coverage Δ
pytest-gpu-aomp-amdgpuX	`68.69% <57.54%> (-0.05%)`	⬇️
pytest-gpu-nvc-nvidiaX	`69.20% <56.60%> (-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.

devito/passes/iet/languages/C.py

devito/passes/iet/languages/CXX.py

FabioLuporini · 2025-10-17T12:31:05Z

devito/passes/clusters/buffering.py

+            if len(ispaces) > 1:
+                raise CompilationError("Unsupported `buffering` over different "
+                                       "IterationSpaces")
+            ispaces = {i.insert(self.dim, self.bdims).reorder() for i in ispaces}


this now really seems the same thing that you do above...

IDK, I need to look at this in the morning when I'm fresh, but there's a few things that I still don't understand

for example the one I just wrote above
and also why you need .reorder()

FabioLuporini

comm

FabioLuporini · 2025-10-23T12:10:06Z

devito/passes/clusters/buffering.py

+        # using it in an expression, such as HaloTouch Clusters
+        def key(c):
+            bufferdim = any(i in c.ispace.dimensions for i in self.bdims)
+            timeonly = all(d.is_Time for d in c.ispace.dimensions)


we cannot use is_Time

and why is this necessary now?

FabioLuporini · 2025-10-23T12:10:45Z

devito/passes/iet/languages/CXX.py

 """


+def split_pointer(i, idx):


A short comment would help

the comment is below in the atomic

devito/passes/clusters/buffering.py

EdCaunt · 2025-10-27T11:49:30Z

devito/ir/cgen/printer.py

    def _print_IndexedPointer(self, expr):
-        return f"{expr.base}{''.join(f'[{self._print(i)}]' for i in expr.index)}"
+        base = self._print(expr.base)
+        return f"{base}{''.join(f'[{self._print(i)}]' for i in expr.index)}"


Nitpick: Can we split this into two lines? The nested fstring is not super easy to read imo

devito/passes/iet/languages/C.py

EdCaunt · 2025-10-27T12:11:12Z

devito/passes/iet/languages/openmp.py

 from devito.passes.iet.languages.utils import joins
-from devito.passes.iet.languages.C import CBB
-from devito.passes.iet.languages.CXX import CXXBB
+from devito.passes.iet.languages.C import CBB, atomic_add as c_atomic_add


Would it be worth attaching the C/CXX atomic add functions to their corresponding BB classes to avoid this?

It is attached to their BB, this is the parprgama that need to call it

There is no need for atomic without OMP so it should be set here

EdCaunt · 2025-10-27T12:16:40Z

devito/passes/iet/languages/utils.py

+       and np.issubdtype(rhs.dtype, np.complexfloating)):
+        # Complex i, complex j
+        # Atomic add real and imaginary parts separately
+        lhsr, rhsr = real(lhs), Real(rhs)


Lowercase real() and imag() get redefined lower in the function - maybe this wants tweaking? I assume the different real and imag functions for the LHS and RHS are because the expressions Real/Imag get lowered to cannot be assigned to?

EdCaunt · 2025-10-27T12:17:59Z

devito/passes/iet/languages/utils.py

+    lhs, rhs = i.expr.lhs, i.expr.rhs
+    if (np.issubdtype(lhs.dtype, np.complexfloating)
+       and np.issubdtype(rhs.dtype, np.complexfloating)):
+        # Complex i, complex j


Ultra nitpick: j is never defined in these comments

docker/Dockerfile.nvidia

EdCaunt · 2025-10-27T12:36:53Z

tests/test_dtypes.py

+
+@skipif(['noomp', 'device'])
+@pytest.mark.parametrize('dtypeu', [np.float32, np.complex64, np.complex128])
+def test_complex_reduction(dtypeu: np.dtype[np.complexfloating]) -> None:


On a similar note, do we explicitly test complex interpolation/injection terms anywhere? I think we have a simple one for complex injection with CUDA, but doesn't look like we have a more general one

Well this tests injection. Interpolation are just standard add/mul so there isn't any blocker, this is problematic because of the atomic

EdCaunt · 2025-10-27T12:37:35Z

conftest.py

             not get_advisor_path()):
            skipit = "Only `icx+advisor` should be tested here"
            break
+        # Slip if not using openmp


Typo: "slip" -> "skip"

mloubout added the misc label Sep 8, 2025

mloubout force-pushed the locarray branch from 2bf271b to bff395e Compare September 8, 2025 14:39

EdCaunt approved these changes Sep 8, 2025

View reviewed changes

EdCaunt changed the title ~~misc: miscelanous corner cases fixes~~ misc: miscellaneous corner cases fixes Sep 8, 2025

mloubout changed the title ~~misc: miscellaneous corner cases fixes~~ misc: Miscellaneous corner cases fixes Sep 8, 2025

mloubout force-pushed the locarray branch from bff395e to 7212fa3 Compare September 9, 2025 03:47

georgebisbas approved these changes Sep 9, 2025

View reviewed changes

mloubout force-pushed the locarray branch 10 times, most recently from f560923 to ade9ad9 Compare September 17, 2025 19:15

FabioLuporini reviewed Sep 19, 2025

View reviewed changes

devito/passes/iet/languages/C.py Show resolved Hide resolved

devito/passes/iet/languages/C.py Show resolved Hide resolved

devito/passes/iet/languages/CXX.py Show resolved Hide resolved

mloubout force-pushed the locarray branch 3 times, most recently from 67ca8d1 to f25095c Compare September 29, 2025 13:52

mloubout force-pushed the locarray branch 8 times, most recently from 3bc81fc to b979285 Compare October 6, 2025 18:33

mloubout force-pushed the locarray branch from 9d598d0 to 41fa7b9 Compare October 16, 2025 19:49

FabioLuporini reviewed Oct 17, 2025

View reviewed changes

mloubout force-pushed the locarray branch 4 times, most recently from 14060ae to fa92d42 Compare October 21, 2025 13:41

FabioLuporini reviewed Oct 23, 2025

View reviewed changes

devito/passes/clusters/buffering.py Outdated Show resolved Hide resolved

FabioLuporini reviewed Oct 23, 2025

View reviewed changes

devito/passes/clusters/buffering.py Show resolved Hide resolved

mloubout force-pushed the locarray branch 8 times, most recently from 23a609e to 445a783 Compare October 23, 2025 16:30

FabioLuporini approved these changes Oct 24, 2025

View reviewed changes

EdCaunt reviewed Oct 27, 2025

View reviewed changes

mloubout force-pushed the locarray branch from 445a783 to 8ee4994 Compare October 27, 2025 20:46

mloubout added 6 commits October 29, 2025 12:47

compiler: add local array with fixed size type

4c4b7ef

compiler: fix complex reductions for gnu

2c5433b

compiler: simplify complex reductions

b8434e1

compiler: fix buffering ispace

db36802

compiler: lift buffer ispace into property

a4060b6

comiler: better handle buffer ispace

edeb3b6

mloubout force-pushed the locarray branch from 8ee4994 to edeb3b6 Compare October 29, 2025 16:47

mloubout merged commit 876310b into main Oct 31, 2025
62 of 65 checks passed

mloubout deleted the locarray branch October 31, 2025 14:17

misc: Miscellaneous corner cases fixes #2730

misc: Miscellaneous corner cases fixes #2730

Uh oh!

Conversation

mloubout commented Sep 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

EdCaunt left a comment

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Sep 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mloubout Oct 27, 2025 • 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!

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!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

mloubout commented Sep 8, 2025 •

edited

Loading

codecov bot commented Sep 8, 2025 •

edited

Loading

mloubout Oct 27, 2025 •

edited

Loading