compiler: Improve IndexDerivatives lowering #2183

FabioLuporini · 2023-08-08T07:36:57Z

Comments:

TODO: rebase and squash away some commits
~~not to be merged until we introduce git-submodule in PRO~~ DONE
~~some notebooks still to be updated~~ DONE
no new features added; mostly restructuring, improvements, tweaks, minor fixes

~~Anyway, open for early reviewing.~~

EDIT: About time to start reviewing

FabioLuporini · 2023-08-08T07:38:39Z

devito/ir/support/basic.py

@@ -1064,3 +1090,147 @@ def r_all(self):
        All Relations of the Scope.
        """
        return list(self.r_gen())
+
+
+class ExprGeometry(object):


Lifted and tweaked from passes/clusters/aliases.py

codecov · 2023-08-08T07:43:37Z

Codecov Report

Merging #2183 (9041b7c) into master (7763d03) will decrease coverage by 0.11%.
The diff coverage is 82.93%.

@@            Coverage Diff             @@
##           master    #2183      +/-   ##
==========================================
- Coverage   87.06%   86.95%   -0.11%     
==========================================
  Files         228      228              
  Lines       40299    40774     +475     
  Branches     7357     7483     +126     
==========================================
+ Hits        35088    35457     +369     
- Misses       4625     4724      +99     
- Partials      586      593       +7

Files Changed	Coverage Δ
tests/test_gpu_common.py	`1.44% <0.00%> (ø)`
devito/ir/support/guards.py	`51.77% <9.09%> (-7.74%)`	⬇️
devito/types/basic.py	`92.46% <33.33%> (-0.79%)`	⬇️
devito/tools/data_structures.py	`67.69% <36.17%> (-4.98%)`	⬇️
devito/operator/operator.py	`87.94% <50.00%> (-0.56%)`	⬇️
devito/ir/support/space.py	`89.74% <54.54%> (-0.81%)`	⬇️
devito/passes/clusters/cse.py	`96.66% <57.14%> (-3.34%)`	⬇️
devito/ir/support/utils.py	`84.83% <67.56%> (-5.50%)`	⬇️
devito/finite_differences/differentiable.py	`93.68% <70.00%> (-0.51%)`	⬇️
devito/finite_differences/tools.py	`92.96% <75.00%> (-0.76%)`	⬇️
... and 32 more

... and 3 files with indirect coverage changes

georgebisbas · 2023-09-04T12:51:43Z

devito/finite_differences/finite_difference.py

    if not expand and indices.expr is not None:
        weights = Weights(name='w', dimensions=indices.free_dim, initvalue=weights)

+        if matvec == transpose:
+            # For homogenity, always generate e.g. `x + i0` rather than `x - i0`


Homogeneity

georgebisbas · 2023-09-04T12:58:01Z

devito/ir/clusters/cluster.py

+    return Properties(properties)
+
+
+def tailor_properties(properties, ispace):


tailor_*
reduce_*
normalize_*
relax_*

better to add docstrings that would make clear what is the use of each one.
I feel the names are not really helpful to someone who would read the code.
ofc they are all used in different ways, different args and context.... just saying how it feels

georgebisbas · 2023-09-04T13:13:48Z

devito/ir/support/guards.py

+
+# *** Utils
+
+def and_smart(relation, v):


simplify_and ?

yes, will change

georgebisbas · 2023-09-04T13:15:35Z

devito/ir/support/space.py

@@ -916,6 +929,10 @@ def is_compatible(self, other):
    def itdimensions(self):
        return self.intervals.dimensions

+    @property
+    def itdims(self):


redundant?
Why not use directly itdimensions?

it's indeed a shortcut. Itdimensions is painfully long and so we ought to update it everywhere. Maybe I'll do that in this PR

georgebisbas · 2023-09-04T13:17:05Z

devito/ir/support/utils.py

+    return Extrema(minimum(expr), maximum(expr))
+
+
+def minmax_index(expr, d):


dim_extrema or similar?

georgebisbas · 2023-09-04T14:50:23Z

devito/operator/profiling.py

+        except (AttributeError, TypeError):
+            # E.g., the section has a dynamic loop size
+            points = np.nan
+


georgebisbas · 2023-09-04T14:59:09Z

devito/passes/clusters/blocking.py

+    def next(self, prefix, d, clusters):
+        # If a whole new set of Dimensions, move the tip -- this means `clusters`
+        # at `d` represents a new loop nest or kernel
+        x = any(i.dim.is_Block for i in flatten(c.ispace for c in clusters))


something different than x?

georgebisbas · 2023-09-04T15:05:31Z

devito/passes/iet/instrument.py

@@ -57,6 +62,37 @@ def track_subsections(iet, **kwargs):

    iet = Transformer(mapper).visit(iet)

+    # Multi-pass implementations


Is this PRO only?

georgebisbas · 2023-09-04T15:14:21Z

devito/types/basic.py

+
+    def _subs(self, old, new, **hints):
+        # Wrap in a try to make sure no substitution happens when
+        # old is an Indexed as only checkink `old is new` would lead to


this is from a cherry-picked commit that is now in master

https://github.com/devitocodes/devito/blob/master/devito/types/basic.py#L1467

mloubout

Will have a proper look later

mloubout · 2023-09-05T14:39:14Z

devito/finite_differences/finite_difference.py

+            # For homogenity, always generate e.g. `x + i0` rather than `x - i0`
+            # for transpose and `x + i0` for direct
+            indices = indices.transpose()
+            weights = weights._subs(indices.free_dim, -indices.free_dim)


weights.transpose() ?

It was that way, just like the indices above, but I had to change it because... I don't remember exactly

mloubout · 2023-09-05T14:45:59Z

devito/ir/support/basic.py

+           isinstance(other.access, ComponentAccess) and \
+           self.access.index != other.access.index:
+            # E.g., `uv(x).x` and `uv(x).y` -- not a real dependence!
+            return Vector(S.ImaginaryUnit)


Can we use something else there. As we want to add complex support would be better to use something like Inf or NaN

I could do that, but this is several layers of abstraction below, when the imaginary parts (in a mathematical sense) will have already been completely lowered away

mloubout · 2023-09-05T14:49:13Z

devito/ir/support/utils.py



-def sdims_max(expr):
+def _minmax(expr, callback, udims=None):


minmax usually used for min-of-max so would use _relational

review-notebook-app · 2023-09-06T12:33:22Z

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

FabioLuporini · 2023-09-18T10:42:46Z

closing as superseded by #2208

all comments here have been taken care of in #2208

FabioLuporini added the compiler label Aug 8, 2023

FabioLuporini requested a review from mloubout August 8, 2023 07:36

FabioLuporini commented Aug 8, 2023

View reviewed changes

FabioLuporini force-pushed the revamp-opt-derivs-rebased branch from 8f8871e to 4d2f5f2 Compare August 23, 2023 16:25

georgebisbas changed the title ~~Improve IndexDerivatives lowering~~ compiler: Improve IndexDerivatives lowering Sep 4, 2023

georgebisbas reviewed Sep 4, 2023

View reviewed changes

mloubout reviewed Sep 5, 2023

View reviewed changes

FabioLuporini added 22 commits September 6, 2023 12:15

compiler: Emulate e.find(type) with (faster) search(e, type)

83f892b

compiler: Add IterationSpace.prefix

a8e1743

compiler: Use internal repr for IndexSums

a7cba2d

compiler: Add Bunch.__repr__

354cf5c

compiler: Add ClusterGroup.rebuild

379c571

compiler: Add ClusterGroup.properties

f00228a

comiler: Add minmax_index()

1189dcb

compiler: Add DAG.all_predecessors

34daf46

tools: Add DAG.find_paths

d3c1d22

compiler: DROP ME AFTER REBASE WITH FUTURE PR: Fix Indexed._subs

7a34949

compiler: Generalize AffineIndexAccessFunction

09b57eb

compiler: Fix DefFunction printing

7ee89ef

compiler: aliases.Candidate -> ir.ExprGeometry

6a9e320

compiler: Generalize and enhance ExprGeometry

30243f2

compiler: Tweak minmax_index

dceaf1b

compiler: Add and_smart for guards auto-simplification

bccca77

compiler: Improve Cluster.is_dense

87d0d2c

compiler: Add IndexDerivative.base

9d60dc8

compiler: Patch AffineIndexAccessFunction

dc9bac4

compiler: Support 2-pass impls w unexpasion

c9d3f96

compiler: Improve profiling of multipass implementations

aa79d6d

compiler: Patch sync_sections

7d70ff9

FabioLuporini added 23 commits September 6, 2023 12:17

compiler: Patch infer_dtype to support vector types

e7ff141

compiler: Fix DDA involving ComponentAccesses

8ecb7cc

api: Support pattern-matching par-tile

a046fa3

compiler: Patch minimize_symbols for parlang backends

756e0a1

compiler: Tweak pow_to_mul & factorize

3e7270b

compiler: Expand along SteppingDimensions

4b7c213

compiler: Update behavior of ClusterGroup.syncs

11ff50f

compiler: Add and exploit properties.is_parallel_atomic

44b752e

compiler: Enhance DDA across IndexDerivatives

cc4d9ad

compiler: Tidy up utilities

83d0924

compiler: Make IndexDerivatives homogeneous irrespective of matvec

0d18e85

compiler: Fix 2-pass implementations with expand=False

c9f006a

compiler: Tweak aliases selection

f1466d1

compiler: Patch CireIndexDerivatives

6ed33ee

compiler: Add DAG.roots

e10d99f

compiler: Remame CIRE search/compose funcs

ee501d5

compiler: Tweak cire-schedule behavior

1827ad4

compiler: Improve DAG

729bc3f

compiler: Make collect_derivatives stable

5242a1d

compiler: Tweak group aliases detection

3f17791

compiler: Make Bunch iterable

da40050

compiler: Drop .find where possible

6420df3

examples: Update expected output

e4f186b

FabioLuporini force-pushed the revamp-opt-derivs-rebased branch from ab8fd92 to e4f186b Compare September 6, 2023 12:33

compiler: Fix deterministic codegen

9041b7c

FabioLuporini mentioned this pull request Sep 18, 2023

compiler: Revamp lowering of IndexDerivatives #2208

Merged

FabioLuporini closed this Sep 18, 2023

FabioLuporini deleted the revamp-opt-derivs-rebased branch October 16, 2023 12:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

compiler: Improve IndexDerivatives lowering #2183

compiler: Improve IndexDerivatives lowering #2183

FabioLuporini commented Aug 8, 2023 •

edited

Loading

FabioLuporini Aug 8, 2023 •

edited

Loading

codecov bot commented Aug 8, 2023 •

edited

Loading

georgebisbas Sep 4, 2023

georgebisbas Sep 4, 2023

georgebisbas Sep 4, 2023

FabioLuporini Sep 6, 2023

georgebisbas Sep 4, 2023

FabioLuporini Sep 6, 2023

FabioLuporini Sep 7, 2023

georgebisbas Sep 4, 2023

georgebisbas Sep 4, 2023

georgebisbas Sep 4, 2023

georgebisbas Sep 4, 2023

FabioLuporini Sep 6, 2023

georgebisbas Sep 4, 2023

FabioLuporini Sep 7, 2023

mloubout left a comment

mloubout Sep 5, 2023

FabioLuporini Sep 6, 2023

mloubout Sep 5, 2023

FabioLuporini Sep 6, 2023

mloubout Sep 5, 2023

FabioLuporini Sep 6, 2023

review-notebook-app bot commented Sep 6, 2023

FabioLuporini commented Sep 18, 2023

		return Properties(properties)


		def tailor_properties(properties, ispace):

		return Extrema(minimum(expr), maximum(expr))


		def minmax_index(expr, d):

		@@ -57,6 +62,37 @@ def track_subsections(iet, **kwargs):

		iet = Transformer(mapper).visit(iet)

		# Multi-pass implementations



		def sdims_max(expr):
		def _minmax(expr, callback, udims=None):

compiler: Improve IndexDerivatives lowering #2183

compiler: Improve IndexDerivatives lowering #2183

Conversation

FabioLuporini commented Aug 8, 2023 • edited Loading

FabioLuporini Aug 8, 2023 • edited Loading

Choose a reason for hiding this comment

codecov bot commented Aug 8, 2023 • edited Loading

Codecov Report

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

review-notebook-app bot commented Sep 6, 2023

FabioLuporini commented Sep 18, 2023

FabioLuporini commented Aug 8, 2023 •

edited

Loading

FabioLuporini Aug 8, 2023 •

edited

Loading

codecov bot commented Aug 8, 2023 •

edited

Loading