Merge pull request #2950 from devitocodes/fix-buffer-stride-runtime-mismatch

compiler: Fix buffer stride runtime mismatch

Author: mloubout

devito/finite_differences/interpolation.py

@@ -14,6 +14,8 @@ def interp_mapper(source, target, dims):
     """
     mapper = {}
     for d in dims:
+        if d.is_Time:
+            continue
         try:
             s = source[d]
             t = target[d]

devito/mpi/routines.py

@@ -367,8 +367,7 @@ def _make_copy(self, f, hse, key, swap=False):
         eqns.extend([Eq(d.symbolic_max, d.symbolic_size - 1) for d in bdims])
 
         vd = CustomDimension(name='vd', symbolic_size=f.ncomp)
-        buf = Array(name='buf', dimensions=[vd] + bdims, dtype=f.c0.dtype,
-                    padding=0)
+        buf = Array(name='buf', dimensions=[vd] + bdims, dtype=f.c0.dtype)
 
         mapper = dict(zip(dims, bdims, strict=True))
         findices = [
@@ -410,9 +409,9 @@ def _make_sendrecv(self, f, hse, key, **kwargs):
         bdims = [CustomDimension(name='vd', symbolic_size=f.ncomp)] + dims
 
         bufg = Array(name='bufg', dimensions=bdims, dtype=f.c0.dtype,
-                     padding=0, liveness='eager')
+                     liveness='eager')
         bufs = Array(name='bufs', dimensions=bdims, dtype=f.c0.dtype,
-                     padding=0, liveness='eager')
+                     liveness='eager')
 
         ofsg = [Symbol(name=f'og{d.root}') for d in f.dimensions]
         ofss = [Symbol(name=f'os{d.root}') for d in f.dimensions]

devito/passes/clusters/buffering.py

@@ -376,6 +376,8 @@ def generate_buffers(clusters, key, sregistry, options, **kwargs):
             assert len(buffers) == 1, "Unexpected form of multi-level buffering"
             buffer, = buffers
             xd = buffer.indices[dim]
+            # The new buffer is derived from `buffer`, so it inherits its padding policy
+            extra_kwargs = {'is_autopaddable': buffer.is_autopaddable}
         else:
             size = infer_buffer_size(f, dim, clusters)
 
@@ -395,6 +397,7 @@ def generate_buffers(clusters, key, sregistry, options, **kwargs):
             except KeyError:
                 name = sregistry.make_name(prefix='db')
                 xd = xds[(dim, size)] = BufferDimension(name, 0, size-1, size, dim)
+            extra_kwargs = {}
 
         # The buffer dimensions
         dimensions = list(f.dimensions)
@@ -404,13 +407,9 @@ def generate_buffers(clusters, key, sregistry, options, **kwargs):
         # Finally create the actual buffer
         cls = callback or Array
         name = sregistry.make_name(prefix=f'{f.name}b')
-        # We specify the padding to match the input Function's one, so that
-        # the array can be used in place of the Function with valid strides
-        # Plain Array do not track mapped so we default to no padding
-        padding = 0 if cls is Array else f.padding
         mapper[f] = cls(name=name, dimensions=dimensions, dtype=f.dtype,
-                        padding=padding, grid=f.grid, halo=f.halo,
-                        space='mapped', mapped=f, f=f)
+                        grid=f.grid, halo=f.halo,
+                        space='mapped', mapped=f, f=f, **extra_kwargs)
 
     return mapper
 

devito/passes/iet/linearization.py

@@ -70,19 +70,7 @@ def key1(f, d):
     if f.is_regular:
         # For paddable objects the following holds:
         # `same dim + same halo + same padding_dtype => same (auto-)padding`
-        if d is f.dimensions[-1]:
-            # Only the last dimension is padded
-            try:
-                if f.padding == f.mapped.padding:
-                    # Padding set from the mapped Function
-                    # e.g. from buffering or fft temp array
-                    pad_key = f.mapped.__padding_dtype__
-                else:
-                    pad_key = f.__padding_dtype__
-            except AttributeError:
-                pad_key = f.__padding_dtype__
-        else:
-            pad_key = None
+        pad_key = f.__padding_dtype__ if d in f._padded_dimensions else None
 
         return (d, f._size_halo[d], pad_key)
     else:

devito/types/array.py

@@ -94,8 +94,6 @@ class Array(ArrayBasic):
         to ``np.float32``.
     halo : iterable of 2-tuples, optional
         The halo region of the object.
-    padding : iterable of 2-tuples, optional
-        The padding region of the object.
     liveness : str, optional
         The liveness of the object. Allowed values: 'eager', 'lazy'. Defaults
         to 'lazy'. Used to override `_mem_internal_eager` and `_mem_internal_lazy`.
@@ -163,17 +161,12 @@ def __dtype_setup__(cls, **kwargs):
         return kwargs.get('dtype', np.float32)
 
     def __padding_setup__(self, **kwargs):
-        padding = kwargs.get('padding')
-        if padding is None:
-            padding = ((0, 0),)*self.ndim
-        elif isinstance(padding, DimensionTuple):
-            padding = tuple(padding[d] for d in self.dimensions)
-        elif is_integer(padding):
-            padding = tuple((0, padding) for _ in range(self.ndim))
-        elif isinstance(padding, tuple) and len(padding) == self.ndim:
-            padding = tuple((0, i) if is_integer(i) else i for i in padding)
+        # `padding=` is never honored: derived from policy to avoid stride
+        # inconsistencies between Functions sharing dimensions/halo
+        if self.is_autopaddable:
+            padding = self.__padding_setup_smart__(**kwargs)
         else:
-            raise TypeError(f'`padding` must be int or {self.ndim}-tuple of ints')
+            padding = ((0, 0),)*self.ndim
         return DimensionTuple(*padding, getters=self.dimensions)
 
     @property
@@ -244,7 +237,42 @@ class MappedArrayMixin:
 
 
 class ArrayMapped(MappedArrayMixin, Array):
-    pass
+
+    __rkwargs__ = Array.__rkwargs__ + ('mapped', 'is_autopaddable')
+
+    def __init_finalize__(self, *args, **kwargs):
+        self._mapped = kwargs.get('mapped')
+        self._is_autopaddable = kwargs.get('is_autopaddable')
+        super().__init_finalize__(*args, **kwargs)
+
+    @property
+    def mapped(self):
+        return self._mapped
+
+    @property
+    def is_autopaddable(self):
+        if self._is_autopaddable is not None:
+            return self._is_autopaddable
+        if self.mapped is None:
+            return True
+        return self.mapped.is_autopaddable
+
+    @property
+    def __padding_dtype__(self):
+        if not self.is_autopaddable:
+            return None
+        if self.mapped is not None:
+            return self.mapped.__padding_dtype__
+        return super().__padding_dtype__
+
+    @cached_property
+    def _signature(self):
+        # Exclude `mapped` so buf-reuse can dedup buffers across distinct
+        # mapped Functions
+        ret = [type(self), self.indices]
+        attrs = set(self.__rkwargs__) - {'name', 'function', 'mapped'}
+        ret.extend(getattr(self, i) for i in attrs)
+        return frozenset(ret)
 
 
 class ArrayObject(ArrayBasic):

devito/types/basic.py

@@ -711,7 +711,7 @@ class AbstractFunction(sympy.Function, Basic, Pickable, Evaluable):
     effect if autopadding is disabled, which is the default behavior.
     """
 
-    __rkwargs__ = ('name', 'dtype', 'grid', 'halo', 'padding', 'ghost',
+    __rkwargs__ = ('name', 'dtype', 'grid', 'halo', 'ghost',
                    'alias', 'space', 'function', 'is_transient', 'avg_mode')
 
     __properties__ = ('is_const', 'is_transient')
@@ -743,17 +743,16 @@ def __new__(cls, *args, **kwargs):
             return function
 
         # If dimensions have been replaced, then it is necessary to set `function`
-        # to None. It may also be necessary to remove halo and padding so that
-        # they are rebuilt with the new dimensions
+        # to None. It may also be necessary to remove halo so that it is rebuilt
+        # with the new dimensions
         if function is not None and function.dimensions != dimensions:
             function = kwargs['function'] = None
-            for i in ('halo', 'padding'):
-                if len(kwargs[i]) != len(dimensions):
-                    kwargs.pop(i)
-                else:
-                    # Downcast from DimensionTuple so that the new `dimensions`
-                    # are used down the line
-                    kwargs[i] = tuple(kwargs[i])
+            if len(kwargs['halo']) != len(dimensions):
+                kwargs.pop('halo')
+            else:
+                # Downcast from DimensionTuple so that the new `dimensions`
+                # are used down the line
+                kwargs['halo'] = tuple(kwargs['halo'])
 
         with sympy_mutex:
             # Go straight through Basic, thus bypassing caching and machinery
@@ -890,8 +889,10 @@ def __halo_setup__(self, **kwargs):
         halo = tuple(kwargs.get('halo', ((0, 0),)*self.ndim))
         return DimensionTuple(*halo, getters=self.dimensions)
 
-    def __padding_setup__(self, padding=None, **kwargs):
-        padding = tuple(padding or ((0, 0),)*self.ndim)
+    def __padding_setup__(self, **kwargs):
+        # `padding=` is never honored: derived from policy to avoid stride
+        # inconsistencies between Functions sharing dimensions/halo
+        padding = ((0, 0),)*self.ndim
         return DimensionTuple(*padding, getters=self.dimensions)
 
     @cached_property
@@ -906,36 +907,26 @@ def __padding_dtype__(self):
             return np.float32
 
     def __padding_setup_smart__(self, **kwargs):
-        nopadding = ((0, 0),)*self.ndim
-
-        if not self.__padding_dtype__:
-            return nopadding
-
-        # The padded Dimension
-        if not self.space_dimensions:
-            return nopadding
-        d = self.space_dimensions[-1]
+        padding = [(0, 0)]*self.ndim
 
-        # Last space Dimension is not the most inner Dimension
-        if d != self.dimensions[-1]:
-            return nopadding
+        if not self.__padding_dtype__ or not self._padded_dimensions:
+            return tuple(padding)
 
         mmts = configuration['platform'].max_mem_trans_size(self.__padding_dtype__)
 
-        snp = self._size_nopad[d]
-        remainder = snp % mmts
-        if remainder == 0:
-            # Already a multiple of `mmts`, no need to pad
-            return nopadding
-        else:
+        for d in self._padded_dimensions:
+            snp = self._size_nopad[d]
+            remainder = snp % mmts
+            if remainder == 0:
+                # Already a multiple of `mmts`, no need to pad
+                continue
+
             from devito.symbolics import RoundUp  # noqa
             v = RoundUp(snp, mmts) - snp
             if v.is_Integer:
                 v = int(v)
 
-        dpadding = (0, v)
-        padding = [(0, 0)]*self.ndim
-        padding[self.dimensions.index(d)] = dpadding
+            padding[self.dimensions.index(d)] = (0, v)
 
         return tuple(padding)
 
@@ -986,6 +977,18 @@ def dimensions(self):
         """Tuple of Dimensions representing the object indices."""
         return DimensionTuple(*self._dimensions, getters=self._dimensions)
 
+    @property
+    def _padded_dimensions(self):
+        try:
+            d = self.space_dimensions[-1]
+        except IndexError:
+            return ()
+
+        if d is self.dimensions[-1]:
+            return (d,)
+        else:
+            return ()
+
     @cached_property
     def space_dimensions(self):
         """Tuple of Dimensions defining the physical space."""

devito/types/dense.py

@@ -1013,9 +1013,6 @@ class Function(DiscreteFunction):
         Controller for memory allocation. To be used, for example, when one wants
         to take advantage of the memory hierarchy in a NUMA architecture. Refer to
         `default_allocator.__doc__` for more information.
-    padding : int or tuple of ints, optional
-        Allocate extra grid points to maximize data access alignment. When a tuple
-        of ints, one int per Dimension should be provided.
 
     Examples
     --------
@@ -1271,25 +1268,12 @@ def __halo_setup__(self, **kwargs):
         return DimensionTuple(*halo, getters=self.dimensions)
 
     def __padding_setup__(self, **kwargs):
-        padding = kwargs.get('padding')
-        if padding is None:
-            if self.is_autopaddable:
-                padding = self.__padding_setup_smart__(**kwargs)
-            else:
-                padding = super().__padding_setup__(**kwargs)
-
-        elif isinstance(padding, DimensionTuple):
-            padding = tuple(padding[d] for d in self.dimensions)
-
-        elif is_integer(padding):
-            padding = tuple((0, padding) if d.is_Space else (0, 0)
-                            for d in self.dimensions)
-
-        elif isinstance(padding, tuple) and len(padding) == self.ndim:
-            padding = tuple((0, i) if is_integer(i) else i for i in padding)
-
+        # `padding=` is never honored: derived from policy to avoid stride
+        # inconsistencies between Functions sharing dimensions/halo
+        if self.is_autopaddable:
+            padding = self.__padding_setup_smart__(**kwargs)
         else:
-            raise TypeError(f"`padding` must be int or {self.ndim}-tuple of ints")
+            return super().__padding_setup__(**kwargs)
         return DimensionTuple(*padding, getters=self.dimensions)
 
     @property
@@ -1410,9 +1394,6 @@ class TimeFunction(Function):
         Controller for memory allocation. To be used, for example, when one wants
         to take advantage of the memory hierarchy in a NUMA architecture. Refer to
         `default_allocator.__doc__` for more information.
-    padding : int or tuple of ints, optional
-        Allocate extra grid points to maximize data access alignment. When a tuple
-        of ints, one int per Dimension should be provided.
 
     Examples
     --------

devito/types/misc.py

@@ -267,12 +267,6 @@ def __init_finalize__(self, *args, shift=None, **kwargs):
         # for homogeneity reasons
         self._shift = as_tuple(shift)
 
-    def __padding_setup__(self, **kwargs):
-        padding = kwargs.pop('padding', None)
-        if padding is None:
-            padding = self.__padding_setup_smart__(**kwargs)
-        return super().__padding_setup__(padding=padding, **kwargs)
-
     @property
     def shift(self):
         return self._shift

examples/userapi/01_dsl.ipynb

@@ -132,9 +132,6 @@
       "        Controller for memory allocation. To be used, for example, when one wants\n",
       "        to take advantage of the memory hierarchy in a NUMA architecture. Refer to\n",
       "        `default_allocator.__doc__` for more information.\n",
-      "    padding : int or tuple of ints, optional\n",
-      "        Allocate extra grid points to maximize data access alignment. When a tuple\n",
-      "        of ints, one int per Dimension should be provided.\n",
       "\n",
       "    Examples\n",
       "    --------\n",
@@ -700,7 +697,7 @@
      "name": "stderr",
      "output_type": "stream",
      "text": [
-      "Operator `Kernel` ran in 0.07 s\n"
+      "Operator `Kernel` ran in 0.06 s\n"
      ]
     },
     {