compiler: Yet another batch of compilation tweaks #2396

FabioLuporini · 2024-07-02T11:20:23Z

These are mostly to improve code generation and performance of elastic-like PDEs.

This is necessary to ensure reconstruction works as expected

FabioLuporini · 2024-07-02T11:21:46Z

devito/ir/support/basic.py

            except AttributeError:
                # E.g., `self=R<f,[cy]>` and `self.itintervals=(y,)` => `sai=None`
                pass

+            # In some cases, the distance degenerates because `self` and


Note: aside from the "Case 3", the rest is just lifted from approximately 40 lines below

codecov · 2024-07-02T11:22:46Z

Codecov Report

Attention: Patch coverage is 74.08638% with 78 lines in your changes missing coverage. Please review.

Project coverage is 86.72%. Comparing base (f67e7af) to head (2d0f60a).

Files	Patch %	Lines
tests/test_gpu_openmp.py	0.00%	22 Missing ⚠️
tests/test_gpu_openacc.py	0.00%	14 Missing ⚠️
devito/passes/iet/engine.py	57.89%	7 Missing and 1 partial ⚠️
devito/types/basic.py	72.00%	4 Missing and 3 partials ⚠️
devito/passes/iet/languages/openacc.py	14.28%	6 Missing ⚠️
devito/ir/support/basic.py	86.48%	2 Missing and 3 partials ⚠️
tests/test_dle.py	86.48%	4 Missing and 1 partial ⚠️
devito/passes/iet/parpragma.py	76.47%	4 Missing ⚠️
devito/core/gpu.py	0.00%	2 Missing ⚠️
devito/ir/iet/visitors.py	83.33%	1 Missing ⚠️
... and 4 more

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2396      +/-   ##
==========================================
+ Coverage   86.70%   86.72%   +0.01%     
==========================================
  Files         234      235       +1     
  Lines       44424    44521      +97     
  Branches     8219     8242      +23     
==========================================
+ Hits        38518    38611      +93     
- Misses       5185     5189       +4     
  Partials      721      721

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

mloubout

Mostly cleanup, look goog.

Few minorcomments

mloubout · 2024-07-02T12:26:37Z

devito/ir/support/basic.py

@@ -2,6 +2,7 @@
 from functools import cached_property

 from sympy import S
+import sympy


jusit import Interval above no?

I would, but to make a neat distinction with our own Interval...

devito/passes/iet/languages/utils.py

mloubout · 2024-07-02T12:34:55Z

tests/test_linearize.py

@@ -611,4 +611,4 @@ def test_different_dtype():

    # Check generated code has different strides for different dtypes
    assert "bL0(x,y) b[(x)*y_stride0 + (y)]" in str(op1)
-    assert "L0(x,y) f[(x)*y_stride1 + (y)]" in str(op1)
+    assert "L0(x,y) f[(x)*y_stride0 + (y)]" in str(op1)


Can we parametrize this test with autopadding so y_stride1 case is there for "mixed" padding

it's not possible by construction anymore; autopadding is now a global choice, in essence. This means that if you have an fp32 and an fp64 the data will be padded to either fp32 or fp64; in the former case, the performance will be suboptimal. But regardless, we will always only generate one stride per dimension. See types/basic.py::AbstractFunction::__padding_dtype__ logic

That said, I've now parametrized the test as you suggested, but the expected outcome is always y_stride0

devito/ir/support/basic.py

georgebisbas

I do not have something more to add.

devito/ir/support/basic.py

devito/core/gpu.py

devito/ir/clusters/algorithms.py

EdCaunt · 2024-07-03T11:20:01Z

devito/ir/iet/visitors.py

@@ -480,7 +480,7 @@ def visit_Expression(self, o):
            code = c.Assign(lhs, rhs)

        if o.pragmas:


These four lines are repeated twice. Worth constructing a utility function?

which four lines? I only see two, starting at if o.pragmas -- which is too little to deserve a separate function

if o.pragmas: code = c.Module(self._visit(o.pragmas) + (code,)) return code

but tbh, you're probably right

EdCaunt · 2024-07-03T11:28:56Z

devito/ir/support/basic.py

+
+    if d.is_Custom:
+        subs = {}
+    elif d.is_Sub and d.is_left:


Why can middle/right subdimensions be ignored? Genuinely curious

we just don't treat them as we don't -- practically speaking need to

as per the docstring:

"A rudimentary test ..."

and

"Our implementation focuses on tiny yet relevant cases"

so basically this is a simplistic implementation, to be refined in the future if we ever will have to

EdCaunt · 2024-07-03T11:30:34Z

devito/ir/support/basic.py

+    m = it.symbolic_min.subs(subs)
+    M = it.symbolic_max.subs(subs)
+
+    p00 = e0._subs(d, m)


Explanatory comment of the 00, 01, etc would be helpful here

just dummy variable names because you have two "dimensions":

e0 and e1

m and M

which leads to four objects

EdCaunt · 2024-07-03T11:40:14Z

devito/passes/iet/linearization.py

+        # Some objects may contain an extremely large number of elements, so we
+        # conservatively use int64 to avoid potential overflows regardless of
+        # what the user requested via `index-mode`
+        if f.is_SparseTimeFunction:


Just SparseFunction I would say, given that the issue occurs due to a large number of points, rather than a large nt iirc

order of millions is fine, order of billions (due to time...) isn't

Sure, but those are different array dimensions right? Unless the array is linearised?

EdCaunt · 2024-07-03T11:43:06Z

devito/symbolics/manipulation.py

@@ -286,12 +286,12 @@ def pow_to_mul(expr):
        except TypeError:
            # E.g., a Symbol, or possibly a generic expression
            return expr
-        if exp > 10 or exp < -10 or int(exp) != exp or exp == 0:
+        if exp > 10 or exp < -10 or exp == 0:
            # Large and non-integer powers remain untouched


"non-integer" part of this comment needs moving to the elif

good catch, fixing

EdCaunt · 2024-07-03T11:47:24Z

devito/types/basic.py

-            padding = [(0, 0)]*self.ndim
-            padding[self.dimensions.index(d)] = dpadding
+        # The padded Dimension
+        candidates = self.space_dimensions


Could shorten to:

if not self.space_dimensions: return nopadding d = self.space_dimensions[-1]

right, fixing

EdCaunt · 2024-07-03T11:49:28Z

tests/test_dimension.py

@@ -210,6 +210,21 @@ def test_modulo_dims_generation_v2(self):
        assert np.all(f.data[3] == 2)
        assert np.all(f.data[4] == 4)

+    def test_degenerate_to_zero(self):


Docstring or a comment explaining what this does would help understanding at a glance

adding docstring

devito/__init__.py

FabioLuporini added 14 commits July 2, 2024 11:18

compiler: Ensure Function.alias is just a bool

3f63463

compiler: Fix Bundle abstraction

875af51

compiler: Improve iet engine's update_args

887bbf0

compiler: Systematically use Pragma, not cgen.Pragma

4ab1f1c

This is necessary to ensure reconstruction works as expected

compiler: Cleanup blocking

b658103

compiler: Fix rcompile

dad8dc8

compiler: Patch pow-to-mul

cba2ea9

compiler: Switch from local to global autopadding

60f5d43

compiler: Avoid ModuloDimension when size==1

1566fec

compiler: Simplify par-tile selection

ad63a44

compiler: Refactor TimedAccess.distance

a5955a1

compiler: Add rudimentary disjoint_test for DDA

660a438

compiler: Improve par-tile scheduling

bd7c628

compiler: Add AbstractFunction.c0

c988fa9

FabioLuporini added the compiler label Jul 2, 2024

FabioLuporini requested review from mloubout, georgebisbas and EdCaunt July 2, 2024 11:20

FabioLuporini commented Jul 2, 2024

View reviewed changes

mloubout reviewed Jul 2, 2024

View reviewed changes

georgebisbas reviewed Jul 2, 2024

View reviewed changes

devito/ir/support/basic.py Show resolved Hide resolved

FabioLuporini added 2 commits July 3, 2024 08:20

tests: Strengthen test_different_dtype

0851d5c

compiler: Enforce int64 on linearized SparseTimeFunctions

afe0719

FabioLuporini force-pushed the moar-elastic-tweaks branch from b55adb2 to afe0719 Compare July 3, 2024 09:06

georgebisbas approved these changes Jul 3, 2024

View reviewed changes

devito/ir/support/basic.py Show resolved Hide resolved

EdCaunt reviewed Jul 3, 2024

View reviewed changes

compiler: Polish code

efa27df

mloubout requested changes Jul 3, 2024

View reviewed changes

devito/__init__.py Outdated Show resolved Hide resolved

api: Enhance autopadding setup

2d0f60a

mloubout approved these changes Jul 3, 2024

View reviewed changes

FabioLuporini mentioned this pull request Jul 4, 2024

compiler: Default to index-mode=int32 #2399

Merged

EdCaunt approved these changes Jul 8, 2024

View reviewed changes

mloubout merged commit 156db8e into master Jul 8, 2024
30 of 31 checks passed

mloubout deleted the moar-elastic-tweaks branch July 8, 2024 13:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

compiler: Yet another batch of compilation tweaks #2396

compiler: Yet another batch of compilation tweaks #2396

FabioLuporini commented Jul 2, 2024

FabioLuporini Jul 2, 2024

codecov bot commented Jul 2, 2024 •

edited

Loading

mloubout left a comment

mloubout Jul 2, 2024

FabioLuporini Jul 3, 2024

mloubout Jul 2, 2024

FabioLuporini Jul 3, 2024

georgebisbas left a comment

EdCaunt Jul 3, 2024

FabioLuporini Jul 3, 2024

EdCaunt Jul 3, 2024

EdCaunt Jul 3, 2024

FabioLuporini Jul 3, 2024

EdCaunt Jul 3, 2024

FabioLuporini Jul 3, 2024

EdCaunt Jul 3, 2024

FabioLuporini Jul 3, 2024

EdCaunt Jul 3, 2024

EdCaunt Jul 3, 2024

FabioLuporini Jul 3, 2024

EdCaunt Jul 3, 2024

FabioLuporini Jul 3, 2024

EdCaunt Jul 3, 2024

FabioLuporini Jul 3, 2024

		@@ -480,7 +480,7 @@ def visit_Expression(self, o):
		code = c.Assign(lhs, rhs)

		if o.pragmas:

compiler: Yet another batch of compilation tweaks #2396

compiler: Yet another batch of compilation tweaks #2396

Conversation

FabioLuporini commented Jul 2, 2024

Choose a reason for hiding this comment

codecov bot commented Jul 2, 2024 • edited Loading

Codecov Report

mloubout left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

georgebisbas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jul 2, 2024 •

edited

Loading