From 6ace83389954b74e1ca13928bd1c831ceef29d25 Mon Sep 17 00:00:00 2001 From: Edward Caunt Date: Fri, 11 Oct 2024 16:55:59 +0100 Subject: [PATCH 01/16] dsl: Adjust how SubDimensions are created --- devito/ir/equations/algorithms.py | 2 +- devito/types/dimension.py | 101 ++++++++++++++++++++---------- 2 files changed, 68 insertions(+), 35 deletions(-) diff --git a/devito/ir/equations/algorithms.py b/devito/ir/equations/algorithms.py index c8f207855b..a0ed4db0ba 100644 --- a/devito/ir/equations/algorithms.py +++ b/devito/ir/equations/algorithms.py @@ -261,7 +261,7 @@ def _(d, mapper, rebuilt, sregistry): # Already have a substitution for this dimension return - abstract_tkns = MultiSubDimension._symbolic_thickness(d.parent.name) + abstract_tkns = d._symbolic_thickness concrete_tkns = tuple(tkn._rebuild(name=sregistry.make_name(prefix=tkn.name)) for tkn in abstract_tkns) diff --git a/devito/types/dimension.py b/devito/types/dimension.py index d8fbeff959..b749296ee8 100644 --- a/devito/types/dimension.py +++ b/devito/types/dimension.py @@ -536,6 +536,10 @@ def _arg_check(self, *args, **kwargs): # The Dimensions below are exposed in the user API. They can only be created by # the user +class SubDimensionThickness(Scalar): + # Dummy for now + pass + class AbstractSubDimension(DerivedDimension): @@ -550,21 +554,54 @@ class AbstractSubDimension(DerivedDimension): is_AbstractSub = True - __rargs__ = (DerivedDimension.__rargs__ + - ('symbolic_min', 'symbolic_max', 'thickness')) + __rargs__ = DerivedDimension.__rargs__ + ('thickness',) __rkwargs__ = () - def __init_finalize__(self, name, parent, left, right, thickness, **kwargs): + _thicknesstype = SubDimensionThickness + + def __init_finalize__(self, name, parent, thickness, **kwargs): super().__init_finalize__(name, parent) + self._process_thicknesses(thickness) + + def _process_thicknesses(self, thickness): + + ltkn, rtkn = thickness + if not isinstance(ltkn, tuple) or not isinstance(rtkn, tuple): + # FIXME: somewhat ugly + ltkn, rtkn = tuple((sym, val) for sym, val + in zip(self._symbolic_thickness, thickness)) + + self._thickness = Thickness(ltkn, rtkn) + + # Just need to set up the interval accordingly + if self.thickness.right[1] is None: # Left SubDimension + left = self.parent.symbolic_min + right = self.parent.symbolic_min + self.ltkn - 1 + elif self.thickness.left[1] is None: # Right SubDimension + left = self.parent.symbolic_max - self.rtkn + 1 + right = self.parent.symbolic_max + else: # Middle SubDimension + left = self.parent.symbolic_min + self.ltkn + right = self.parent.symbolic_max - self.rtkn + self._interval = sympy.Interval(left, right) - self._thickness = Thickness(*thickness) - @classmethod - def _symbolic_thickness(cls, name, stype=Scalar): - return (stype(name="%s_ltkn" % name, dtype=np.int32, - is_const=True, nonnegative=True), - stype(name="%s_rtkn" % name, dtype=np.int32, - is_const=True, nonnegative=True)) + @cached_property + def _symbolic_thickness(self): + if isinstance(self._thicknesstype, SubDimensionThickness): + kwargs = {'subdim', self} + else: + kwargs = {} + + ltkn = self._thicknesstype(name="%s_ltkn" % self.parent.name, + dtype=np.int32, is_const=True, + nonnegative=True, **kwargs) + + rtkn = self._thicknesstype(name="%s_rtkn" % self.parent.name, + dtype=np.int32, is_const=True, + nonnegative=True, **kwargs) + + return (ltkn, rtkn) @cached_property def symbolic_min(self): @@ -659,36 +696,30 @@ class SubDimension(AbstractSubDimension): __rargs__ = AbstractSubDimension.__rargs__ + ('local',) - def __init_finalize__(self, name, parent, left, right, thickness, local, + def __init_finalize__(self, name, parent, thickness, local, **kwargs): - super().__init_finalize__(name, parent, left, right, thickness) + super().__init_finalize__(name, parent, thickness) self._local = local @classmethod def left(cls, name, parent, thickness, local=True): - lst, rst = cls._symbolic_thickness(parent.name) return cls(name, parent, - left=parent.symbolic_min, - right=parent.symbolic_min+lst-1, - thickness=((lst, thickness), (rst, None)), + thickness=(thickness, None), + # thickness=((lst, thickness), (rst, None)), local=local) @classmethod def right(cls, name, parent, thickness, local=True): - lst, rst = cls._symbolic_thickness(parent.name) return cls(name, parent, - left=parent.symbolic_max-rst+1, - right=parent.symbolic_max, - thickness=((lst, None), (rst, thickness)), + thickness=(None, thickness), + # thickness=((lst, None), (rst, thickness)), local=local) @classmethod def middle(cls, name, parent, thickness_left, thickness_right, local=False): - lst, rst = cls._symbolic_thickness(parent.name) return cls(name, parent, - left=parent.symbolic_min+lst, - right=parent.symbolic_max-rst, - thickness=((lst, thickness_left), (rst, thickness_right)), + thickness=(thickness_left, thickness_right), + # thickness=((lst, thickness_left), (rst, thickness_right)), local=local) @property @@ -809,11 +840,11 @@ class MultiSubDimension(AbstractSubDimension): is_MultiSub = True - __rargs__ = (DerivedDimension.__rargs__ + ('thickness',)) __rkwargs__ = ('functions', 'bounds_indices', 'implicit_dimension') - def __init_finalize__(self, name, parent, thickness, functions=None, - bounds_indices=None, implicit_dimension=None): + _thicknesstype = Symbol + + def _process_thicknesses(self, thickness): # Canonicalize thickness if thickness is None: # Using dummy left/right is the only thing we can do for such @@ -830,8 +861,8 @@ def __init_finalize__(self, name, parent, thickness, functions=None, (ltkn, _), (rtkn, _) = thickness try: - left = parent.symbolic_min + ltkn - right = parent.symbolic_max - rtkn + left = self.parent.symbolic_min + ltkn + right = self.parent.symbolic_max - rtkn except TypeError: # May end up here after a reconstruction left = sympy.S.NegativeInfinity @@ -839,7 +870,13 @@ def __init_finalize__(self, name, parent, thickness, functions=None, else: raise ValueError("MultiSubDimension expects a tuple of thicknesses") - super().__init_finalize__(name, parent, left, right, thickness) + self._thickness = Thickness(*thickness) + self._interval = sympy.Interval(left, right) + + def __init_finalize__(self, name, parent, thickness, functions=None, + bounds_indices=None, implicit_dimension=None): + + super().__init_finalize__(name, parent, thickness) self.functions = functions self.bounds_indices = bounds_indices self.implicit_dimension = implicit_dimension @@ -850,10 +887,6 @@ def __hash__(self): # which is unique return id(self) - @classmethod - def _symbolic_thickness(cls, name): - return super()._symbolic_thickness(name, stype=Symbol) - @cached_property def bound_symbols(self): return self.parent.bound_symbols From bb63ebba4f50794767eecba4b21dbf3be798c434 Mon Sep 17 00:00:00 2001 From: Edward Caunt Date: Mon, 14 Oct 2024 14:29:29 +0100 Subject: [PATCH 02/16] dsl: Tweaks to subdimensions --- devito/types/dimension.py | 21 ++++++++------------- tests/test_caching.py | 2 +- 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/devito/types/dimension.py b/devito/types/dimension.py index b749296ee8..007e20b392 100644 --- a/devito/types/dimension.py +++ b/devito/types/dimension.py @@ -538,6 +538,9 @@ def _arg_check(self, *args, **kwargs): class SubDimensionThickness(Scalar): # Dummy for now + # FIXME: If this subclasses symbol, then it needs an arg check. + # This arg check/arg vals could be used to move some functionality + # from SubDimension to SubDimensionThickness pass @@ -588,18 +591,13 @@ def _process_thicknesses(self, thickness): @cached_property def _symbolic_thickness(self): - if isinstance(self._thicknesstype, SubDimensionThickness): - kwargs = {'subdim', self} - else: - kwargs = {} - ltkn = self._thicknesstype(name="%s_ltkn" % self.parent.name, dtype=np.int32, is_const=True, - nonnegative=True, **kwargs) + nonnegative=True) rtkn = self._thicknesstype(name="%s_rtkn" % self.parent.name, dtype=np.int32, is_const=True, - nonnegative=True, **kwargs) + nonnegative=True) return (ltkn, rtkn) @@ -643,6 +641,9 @@ def tkns(self): # Shortcut for both thickness symbols return self.ltkn, self.rtkn + def __hash__(self): + return id(self) + class SubDimension(AbstractSubDimension): @@ -881,12 +882,6 @@ def __init_finalize__(self, name, parent, thickness, functions=None, self.bounds_indices = bounds_indices self.implicit_dimension = implicit_dimension - def __hash__(self): - # There is no possibility for two MultiSubDimensions to ever hash the - # same, since a MultiSubDimension carries a reference to a MultiSubDomain, - # which is unique - return id(self) - @cached_property def bound_symbols(self): return self.parent.bound_symbols diff --git a/tests/test_caching.py b/tests/test_caching.py index 8dca69fa60..810d9c81d2 100644 --- a/tests/test_caching.py +++ b/tests/test_caching.py @@ -844,7 +844,7 @@ def test_solve(self, operate_on_empty_cache): # But this is not the case anymore! assert len(_SymbolCache) == 12 clear_cache() - assert len(_SymbolCache) == 8 + assert len(_SymbolCache) == 6 clear_cache() assert len(_SymbolCache) == 2 clear_cache() From 4e92b63bd787e74f785bda20be8a5d2f6b21439f Mon Sep 17 00:00:00 2001 From: Edward Caunt Date: Tue, 15 Oct 2024 16:53:54 +0100 Subject: [PATCH 03/16] misc: Further investigations --- devito/ir/equations/algorithms.py | 15 +++++++++- devito/ir/iet/visitors.py | 8 +++++- devito/operator/operator.py | 6 ++++ devito/types/dimension.py | 47 ++++++++++++++++++++++--------- 4 files changed, 60 insertions(+), 16 deletions(-) diff --git a/devito/ir/equations/algorithms.py b/devito/ir/equations/algorithms.py index a0ed4db0ba..53f335ca8a 100644 --- a/devito/ir/equations/algorithms.py +++ b/devito/ir/equations/algorithms.py @@ -8,7 +8,7 @@ ConditionalDimension) from devito.types.array import Array from devito.types.basic import AbstractFunction -from devito.types.dimension import MultiSubDimension +from devito.types.dimension import MultiSubDimension, SubDimensionThickness from devito.data.allocators import DataReference from devito.logger import warning @@ -213,6 +213,14 @@ def _(expr, mapper, rebuilt, sregistry): _concretize_subdims(expr.implicit_dims, mapper, rebuilt, sregistry) +@_concretize_subdims.register(SubDimensionThickness) +def _(tkn, mapper, rebuilt, sregistry): + # Concretising the parent SubDimension ensures that SubDimension thicknesses + # are incremented in lockstep + if tkn.subdimension is not None: + _concretize_subdims(tkn.subdimension, mapper, rebuilt, sregistry) + + @_concretize_subdims.register(SubDimension) def _(d, mapper, rebuilt, sregistry): if d in mapper: @@ -221,6 +229,11 @@ def _(d, mapper, rebuilt, sregistry): tkns_subs = {tkn: tkn._rebuild(name=sregistry.make_name(prefix=tkn.name)) for tkn in d.tkns} + + # Add the thickness substitutions to the mapper, as they may appear without the + # SubDimension + mapper.update(tkns_subs) + left, right = [mM.subs(tkns_subs) for mM in (d.symbolic_min, d.symbolic_max)] thickness = tuple((v, d._thickness_map[k]) for k, v in tkns_subs.items()) diff --git a/devito/ir/iet/visitors.py b/devito/ir/iet/visitors.py index 505fe2e001..d8059dacf4 100644 --- a/devito/ir/iet/visitors.py +++ b/devito/ir/iet/visitors.py @@ -25,6 +25,7 @@ from devito.types.basic import AbstractFunction, Basic from devito.types import (ArrayObject, CompositeObject, Dimension, Pointer, IndexedData, DeviceMap) +from devito.types.dimension import SubDimensionThickness __all__ = ['FindApplications', 'FindNodes', 'FindSections', 'FindSymbols', @@ -965,7 +966,12 @@ def _defines_aliases(n): rules = { 'symbolics': lambda n: n.functions, 'basics': lambda n: [i for i in n.expr_symbols if isinstance(i, Basic)], - 'dimensions': lambda n: [i for i in n.expr_symbols if isinstance(i, Dimension)], + # FIXME: This displeases me greatly + 'dimensions': lambda n: list({i for i in n.expr_symbols + if isinstance(i, Dimension)} + | {i.subdimension for i in n.expr_symbols + if isinstance(i, SubDimensionThickness) + and i.subdimension is not None}), 'indexeds': lambda n: [i for i in n.expr_symbols if i.is_Indexed], 'indexedbases': lambda n: [i for i in n.expr_symbols if isinstance(i, IndexedBase)], diff --git a/devito/operator/operator.py b/devito/operator/operator.py index 24258ad671..e0b2c2c2eb 100644 --- a/devito/operator/operator.py +++ b/devito/operator/operator.py @@ -502,6 +502,12 @@ def dimensions(self): # During compilation other Dimensions may have been produced dimensions = FindSymbols('dimensions').visit(self) + + # SubDimensions may only be present in the operator in the form of + # their thicknesses. These SubDimensions should be recovered if not + # already present in the operator dimensions. + ret.update(d for d in dimensions if d.is_Sub) + ret.update(d for d in dimensions if d.is_PerfKnob) ret = tuple(sorted(ret, key=attrgetter('name'))) diff --git a/devito/types/dimension.py b/devito/types/dimension.py index 007e20b392..447b62e947 100644 --- a/devito/types/dimension.py +++ b/devito/types/dimension.py @@ -13,6 +13,7 @@ from devito.types.args import ArgProvider from devito.types.basic import Symbol, DataSymbol, Scalar from devito.types.constant import Constant +from devito.types.caching import Uncached __all__ = ['Dimension', 'SpaceDimension', 'TimeDimension', 'DefaultDimension', @@ -536,12 +537,22 @@ def _arg_check(self, *args, **kwargs): # The Dimensions below are exposed in the user API. They can only be created by # the user -class SubDimensionThickness(Scalar): - # Dummy for now - # FIXME: If this subclasses symbol, then it needs an arg check. - # This arg check/arg vals could be used to move some functionality - # from SubDimension to SubDimensionThickness - pass +class SubDimensionThickness(Constant): + # FIXME: This needs a docstring and should probably live in types/basic.py + # FIXME: SubDimension is dropped during pickling and rebuilding -> this is an issue + # FIXME: What about making it a Constant? + # FIXME: Or even a DataSymbol + __rargs__ = Constant.__rargs__ + ('subdimension',) + + def __new__(cls, *args, **kwargs): + subdimension = kwargs.pop('subdimension', None) + newobj = super().__new__(cls, *args, **kwargs) + newobj._subdimension = subdimension + return newobj + + @property + def subdimension(self): + return self._subdimension class AbstractSubDimension(DerivedDimension): @@ -560,7 +571,7 @@ class AbstractSubDimension(DerivedDimension): __rargs__ = DerivedDimension.__rargs__ + ('thickness',) __rkwargs__ = () - _thicknesstype = SubDimensionThickness + _thickness_type = SubDimensionThickness def __init_finalize__(self, name, parent, thickness, **kwargs): super().__init_finalize__(name, parent) @@ -574,6 +585,9 @@ def _process_thicknesses(self, thickness): ltkn, rtkn = tuple((sym, val) for sym, val in zip(self._symbolic_thickness, thickness)) + # FIXME: I think some weirdness may occur if one pickles then unpickles + # a SubDimension, then uses only the thicknesses of that subdimension in + # an Operator. This should be tested. self._thickness = Thickness(ltkn, rtkn) # Just need to set up the interval accordingly @@ -591,13 +605,17 @@ def _process_thicknesses(self, thickness): @cached_property def _symbolic_thickness(self): - ltkn = self._thicknesstype(name="%s_ltkn" % self.parent.name, - dtype=np.int32, is_const=True, - nonnegative=True) + if self._thickness_type == SubDimensionThickness: + kwargs = {'subdimension': self} + else: + kwargs = {} + ltkn = self._thickness_type(name="%s_ltkn" % self.parent.name, + dtype=np.int64, is_const=True, + nonnegative=True, **kwargs) - rtkn = self._thicknesstype(name="%s_rtkn" % self.parent.name, - dtype=np.int32, is_const=True, - nonnegative=True) + rtkn = self._thickness_type(name="%s_rtkn" % self.parent.name, + dtype=np.int64, is_const=True, + nonnegative=True, **kwargs) return (ltkn, rtkn) @@ -843,7 +861,8 @@ class MultiSubDimension(AbstractSubDimension): __rkwargs__ = ('functions', 'bounds_indices', 'implicit_dimension') - _thicknesstype = Symbol + # FIXME: Is this still necessary? + _thickness_type = Symbol def _process_thicknesses(self, thickness): # Canonicalize thickness From 4458e947bee5e1f0ae46dd2c1c042801d3c245c4 Mon Sep 17 00:00:00 2001 From: Edward Caunt Date: Wed, 16 Oct 2024 11:44:18 +0100 Subject: [PATCH 04/16] misc: Reworking SubDomainThickness --- devito/ir/equations/algorithms.py | 4 +- devito/ir/iet/visitors.py | 8 +- devito/types/dimension.py | 158 ++++++++++++++++-------------- 3 files changed, 85 insertions(+), 85 deletions(-) diff --git a/devito/ir/equations/algorithms.py b/devito/ir/equations/algorithms.py index 53f335ca8a..1457b79af9 100644 --- a/devito/ir/equations/algorithms.py +++ b/devito/ir/equations/algorithms.py @@ -217,8 +217,8 @@ def _(expr, mapper, rebuilt, sregistry): def _(tkn, mapper, rebuilt, sregistry): # Concretising the parent SubDimension ensures that SubDimension thicknesses # are incremented in lockstep - if tkn.subdimension is not None: - _concretize_subdims(tkn.subdimension, mapper, rebuilt, sregistry) + # TODO: Finish + pass @_concretize_subdims.register(SubDimension) diff --git a/devito/ir/iet/visitors.py b/devito/ir/iet/visitors.py index d8059dacf4..505fe2e001 100644 --- a/devito/ir/iet/visitors.py +++ b/devito/ir/iet/visitors.py @@ -25,7 +25,6 @@ from devito.types.basic import AbstractFunction, Basic from devito.types import (ArrayObject, CompositeObject, Dimension, Pointer, IndexedData, DeviceMap) -from devito.types.dimension import SubDimensionThickness __all__ = ['FindApplications', 'FindNodes', 'FindSections', 'FindSymbols', @@ -966,12 +965,7 @@ def _defines_aliases(n): rules = { 'symbolics': lambda n: n.functions, 'basics': lambda n: [i for i in n.expr_symbols if isinstance(i, Basic)], - # FIXME: This displeases me greatly - 'dimensions': lambda n: list({i for i in n.expr_symbols - if isinstance(i, Dimension)} - | {i.subdimension for i in n.expr_symbols - if isinstance(i, SubDimensionThickness) - and i.subdimension is not None}), + 'dimensions': lambda n: [i for i in n.expr_symbols if isinstance(i, Dimension)], 'indexeds': lambda n: [i for i in n.expr_symbols if i.is_Indexed], 'indexedbases': lambda n: [i for i in n.expr_symbols if isinstance(i, IndexedBase)], diff --git a/devito/types/dimension.py b/devito/types/dimension.py index 447b62e947..43f311fa2a 100644 --- a/devito/types/dimension.py +++ b/devito/types/dimension.py @@ -9,7 +9,7 @@ from devito.data import LEFT, RIGHT from devito.exceptions import InvalidArgument from devito.logger import debug -from devito.tools import Pickable, is_integer, flatten +from devito.tools import Pickable, is_integer, flatten, memoized_meth from devito.types.args import ArgProvider from devito.types.basic import Symbol, DataSymbol, Scalar from devito.types.constant import Constant @@ -537,22 +537,70 @@ def _arg_check(self, *args, **kwargs): # The Dimensions below are exposed in the user API. They can only be created by # the user -class SubDimensionThickness(Constant): - # FIXME: This needs a docstring and should probably live in types/basic.py - # FIXME: SubDimension is dropped during pickling and rebuilding -> this is an issue - # FIXME: What about making it a Constant? - # FIXME: Or even a DataSymbol - __rargs__ = Constant.__rargs__ + ('subdimension',) +class SubDimensionThickness(DataSymbol): + """A DataSymbol to represent a thickness of a SubDimension""" + # FIXME: Shouldn't store reference to the subdimension, just the + # metadata necessary to do arg_values etc + # FIXME: Should carry the root of its parent subdimension + # FIXME: Should also have some understanding of if it is associated with + # a left, right, or middle subdimension -> Should be subsumed into local + # FIXME: Should know if it is the left or right side + + # FIXME: Currently dumps an integer into the generated code + + __rkwargs__ = DataSymbol.__rkwargs__ + ('root', 'side', 'local', 'value') def __new__(cls, *args, **kwargs): - subdimension = kwargs.pop('subdimension', None) + # What is the root dimension? + try: + root = kwargs.pop('root') + except KeyError: + raise ValueError("No root dimension provided") + # Is this thickness LEFT or RIGHT? + try: + side = kwargs.pop('side') + except KeyError: + raise ValueError("No side specified") + # Is this thickness for a local SubDimension? + try: + local = kwargs.pop('local') + except KeyError: + raise ValueError("Locality of thickness not specified") + newobj = super().__new__(cls, *args, **kwargs) - newobj._subdimension = subdimension + newobj._root = root + newobj._side = side + newobj._local = local + # NOTE: Can determine correct override from the value, side, local, and root + return newobj + def __init_finalize__(self, *args, **kwargs): + self._value = kwargs.pop('value', None) + + kwargs.setdefault('is_const', True) + super().__init_finalize__(*args, **kwargs) + @property - def subdimension(self): - return self._subdimension + def root(self): + return self._root + + @property + def side(self): + return self._side + + @property + def local(self): + return self._local + + @property + def value(self): + return self._value + + def _arg_check(self, *args, **kwargs): + # TODO: Should check that the thickness isn't larger than the parent + # domain + assert False class AbstractSubDimension(DerivedDimension): @@ -579,22 +627,26 @@ def __init_finalize__(self, name, parent, thickness, **kwargs): def _process_thicknesses(self, thickness): - ltkn, rtkn = thickness - if not isinstance(ltkn, tuple) or not isinstance(rtkn, tuple): + # ltkn, rtkn = thickness + # if not isinstance(ltkn, tuple) or not isinstance(rtkn, tuple): # FIXME: somewhat ugly - ltkn, rtkn = tuple((sym, val) for sym, val - in zip(self._symbolic_thickness, thickness)) + # ltkn, rtkn = tuple((sym, val) for sym, val + # in zip(self._symbolic_thickness, thickness)) + ltkn, rtkn = self._symbolic_thickness(thickness=thickness) # FIXME: I think some weirdness may occur if one pickles then unpickles # a SubDimension, then uses only the thicknesses of that subdimension in # an Operator. This should be tested. + # FIXME: The numerical value of the thickness should be carried by the + # SubDimensionThickness symbols themselves, thereby making the Thickness + # namedtuple obsolete self._thickness = Thickness(ltkn, rtkn) # Just need to set up the interval accordingly - if self.thickness.right[1] is None: # Left SubDimension + if self.thickness.right.value is None: # Left SubDimension left = self.parent.symbolic_min right = self.parent.symbolic_min + self.ltkn - 1 - elif self.thickness.left[1] is None: # Right SubDimension + elif self.thickness.left.value is None: # Right SubDimension left = self.parent.symbolic_max - self.rtkn + 1 right = self.parent.symbolic_max else: # Middle SubDimension @@ -603,19 +655,19 @@ def _process_thicknesses(self, thickness): self._interval = sympy.Interval(left, right) - @cached_property - def _symbolic_thickness(self): - if self._thickness_type == SubDimensionThickness: - kwargs = {'subdimension': self} - else: - kwargs = {} + @memoized_meth + def _symbolic_thickness(self, thickness=None): + thickness = thickness or (None, None) + + kwargs = {'dtype': np.int64, 'is_const': True, 'nonnegative': True, + 'root': self.root, 'local': self.local} ltkn = self._thickness_type(name="%s_ltkn" % self.parent.name, - dtype=np.int64, is_const=True, - nonnegative=True, **kwargs) + side=LEFT, value=thickness[0], + **kwargs) rtkn = self._thickness_type(name="%s_rtkn" % self.parent.name, - dtype=np.int64, is_const=True, - nonnegative=True, **kwargs) + side=RIGHT, value=thickness[1], + **kwargs) return (ltkn, rtkn) @@ -636,10 +688,6 @@ def symbolic_size(self): def thickness(self): return self._thickness - @cached_property - def _thickness_map(self): - return dict(self.thickness) - @property def is_abstract(self): return all(i is None for i in flatten(self.thickness)) @@ -647,12 +695,12 @@ def is_abstract(self): @property def ltkn(self): # Shortcut for the left thickness symbol - return self.thickness.left[0] + return self.thickness.left @property def rtkn(self): # Shortcut for the right thickness symbol - return self.thickness.right[0] + return self.thickness.right @property def tkns(self): @@ -717,8 +765,8 @@ class SubDimension(AbstractSubDimension): def __init_finalize__(self, name, parent, thickness, local, **kwargs): - super().__init_finalize__(name, parent, thickness) self._local = local + super().__init_finalize__(name, parent, thickness) @classmethod def left(cls, name, parent, thickness, local=True): @@ -766,48 +814,6 @@ def bound_symbols(self): def _maybe_distributed(self): return not self.local - @cached_property - def _offset_left(self): - # The left extreme of the SubDimension can be related to either the - # min or max of the parent dimension - try: - symbolic_thickness = self.symbolic_min - self.parent.symbolic_min - val = symbolic_thickness.subs(self._thickness_map) - return SubDimensionOffset( - int(val), - self.parent.symbolic_min, - symbolic_thickness - ) - except TypeError: - symbolic_thickness = self.symbolic_min - self.parent.symbolic_max - val = symbolic_thickness.subs(self._thickness_map) - return SubDimensionOffset( - int(val), - self.parent.symbolic_max, - symbolic_thickness - ) - - @cached_property - def _offset_right(self): - # The right extreme of the SubDimension can be related to either the - # min or max of the parent dimension - try: - symbolic_thickness = self.symbolic_max - self.parent.symbolic_min - val = symbolic_thickness.subs(self._thickness_map) - return SubDimensionOffset( - int(val), - self.parent.symbolic_min, - symbolic_thickness - ) - except TypeError: - symbolic_thickness = self.symbolic_max - self.parent.symbolic_max - val = symbolic_thickness.subs(self._thickness_map) - return SubDimensionOffset( - int(val), - self.parent.symbolic_max, - symbolic_thickness - ) - @property def _arg_names(self): return tuple(k.name for k, _ in self.thickness) + self.parent._arg_names From 163d993d40e041077ec6284b279db32fdefe2316 Mon Sep 17 00:00:00 2001 From: Edward Caunt Date: Wed, 16 Oct 2024 12:02:44 +0100 Subject: [PATCH 05/16] misc: Further SubDomain fiddling --- devito/operator/operator.py | 1 + devito/types/dimension.py | 9 ++++++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/devito/operator/operator.py b/devito/operator/operator.py index e0b2c2c2eb..034a3555b7 100644 --- a/devito/operator/operator.py +++ b/devito/operator/operator.py @@ -672,6 +672,7 @@ def _prepare_arguments(self, autotune=None, **kwargs): # pointers to ctypes.Struct for p in self.parameters: try: + # FIXME: Maybe one of these should pass the grid args.update(kwargs.get(p.name, p)._arg_finalize(args, alias=p)) except AttributeError: # User-provided floats/ndarray obviously do not have `_arg_finalize` diff --git a/devito/types/dimension.py b/devito/types/dimension.py index 43f311fa2a..f4e57bbb9b 100644 --- a/devito/types/dimension.py +++ b/devito/types/dimension.py @@ -600,7 +600,13 @@ def value(self): def _arg_check(self, *args, **kwargs): # TODO: Should check that the thickness isn't larger than the parent # domain - assert False + pass + + def _arg_finalize(self, *args, **kwargs): + # FIXME: Something here is going to need a Grid + # print("Args", args) + # print("Kwargs", kwargs) + return {} class AbstractSubDimension(DerivedDimension): @@ -705,6 +711,7 @@ def rtkn(self): @property def tkns(self): # Shortcut for both thickness symbols + # FIXME: Maybe redundant now? return self.ltkn, self.rtkn def __hash__(self): From 52810d599f4526044e221b1d3f55cd51ab0b7756 Mon Sep 17 00:00:00 2001 From: Edward Caunt Date: Wed, 16 Oct 2024 13:39:29 +0100 Subject: [PATCH 06/16] compiler: Started moving SubDimension arg vals to SubDimensionThickness --- devito/operator/operator.py | 8 +++- devito/types/dimension.py | 87 +++++++++++++++++++++++++++++++++---- 2 files changed, 86 insertions(+), 9 deletions(-) diff --git a/devito/operator/operator.py b/devito/operator/operator.py index 034a3555b7..a8408e023f 100644 --- a/devito/operator/operator.py +++ b/devito/operator/operator.py @@ -31,6 +31,8 @@ split, timed_pass, timed_region, contains_val) from devito.types import (Buffer, Grid, Evaluable, host_layer, device_layer, disk_layer) +from devito.types.dimension import SubDimensionThickness + __all__ = ['Operator'] @@ -650,6 +652,11 @@ def _prepare_arguments(self, autotune=None, **kwargs): for o in self.objects: args.update(o._arg_values(grid=grid, **kwargs)) + # Process SubDimensionThicknesses + for p in self.parameters: + if isinstance(p, SubDimensionThickness): + args.update(p._arg_values(grid=grid, **kwargs)) + # Purge `kwargs` kwargs.pop('args') kwargs.pop('metadata') @@ -672,7 +679,6 @@ def _prepare_arguments(self, autotune=None, **kwargs): # pointers to ctypes.Struct for p in self.parameters: try: - # FIXME: Maybe one of these should pass the grid args.update(kwargs.get(p.name, p)._arg_finalize(args, alias=p)) except AttributeError: # User-provided floats/ndarray obviously do not have `_arg_finalize` diff --git a/devito/types/dimension.py b/devito/types/dimension.py index f4e57bbb9b..b0dfa6bfff 100644 --- a/devito/types/dimension.py +++ b/devito/types/dimension.py @@ -551,6 +551,7 @@ class SubDimensionThickness(DataSymbol): __rkwargs__ = DataSymbol.__rkwargs__ + ('root', 'side', 'local', 'value') def __new__(cls, *args, **kwargs): + # TODO: Args or kwargs? # What is the root dimension? try: root = kwargs.pop('root') @@ -602,10 +603,82 @@ def _arg_check(self, *args, **kwargs): # domain pass + def _arg_values(self, grid=None, **kwargs): + # Allow override of thickness values to disable BCs + # However, arguments from the user are considered global + # So overriding the thickness to a nonzero value should not cause + # boundaries to exist between ranks where they did not before + + # r_ltkn, r_rtkn = ( + # kwargs.get(k.name, v) for k, v in self.thickness + # ) + + # if grid is not None and grid.is_distributed(self.root): + # # Get local thickness + # if self.local: + # # dimension is of type ``left``/right`` - compute the 'offset' + # # and then add 1 to get the appropriate thickness + # if r_ltkn is not None: + # ltkn = grid.distributor.glb_to_loc(self.root, r_ltkn-1, LEFT) + # ltkn = ltkn+1 if ltkn is not None else 0 + # else: + # ltkn = 0 + + # if r_rtkn is not None: + # rtkn = grid.distributor.glb_to_loc(self.root, r_rtkn-1, RIGHT) + # rtkn = rtkn+1 if rtkn is not None else 0 + # else: + # rtkn = 0 + # else: + # # dimension is of type ``middle`` + # ltkn = grid.distributor.glb_to_loc(self.root, r_ltkn, LEFT) or 0 + # rtkn = grid.distributor.glb_to_loc(self.root, r_rtkn, RIGHT) or 0 + # else: + # ltkn = r_ltkn or 0 + # rtkn = r_rtkn or 0 + + # return {i.name: v for i, v in zip(self._thickness_map, (ltkn, rtkn))} + r_tkn = kwargs.get(self.name, self.value) + + # FIXME: Can refactor and consolidate the checks for LEFT and RIGHT + if grid is not None and grid.is_distributed(self.root): + # Get local thickness + if self.local: + # Dimension is of type `left`/`right` - compute the offset + # and then add 1 to get the appropriate thickness + if self.side is LEFT: + if self.value is not None: + ltkn = grid.distributor.glb_to_loc(self.root, r_tkn-1, LEFT) + ltkn = ltkn+1 if ltkn is not None else 0 + else: + ltkn = 0 + + elif self.side is RIGHT: + if self.value is not None: + rtkn = grid.distributor.glb_to_loc(self.root, r_tkn-1, RIGHT) + rtkn = rtkn+1 if rtkn is not None else 0 + else: + rtkn = 0 + + else: + raise ValueError("Unrecognised side %s" % self.side) + + else: + # Dimension is of type `middle` + if self.side is LEFT: + ltkn = grid.distributor.glb_to_loc(self.root, r_tkn, LEFT) or 0 + elif self.side is RIGHT: + rtkn = grid.distributor.glb_to_loc(self.root, r_tkn, RIGHT) or 0 + else: + raise ValueError("Unrecognised side %s" % self.side) + + else: + ltkn = r_tkn or 0 + rtkn = r_tkn or 0 + + + def _arg_finalize(self, *args, **kwargs): - # FIXME: Something here is going to need a Grid - # print("Args", args) - # print("Kwargs", kwargs) return {} @@ -664,16 +737,14 @@ def _process_thicknesses(self, thickness): @memoized_meth def _symbolic_thickness(self, thickness=None): thickness = thickness or (None, None) - kwargs = {'dtype': np.int64, 'is_const': True, 'nonnegative': True, 'root': self.root, 'local': self.local} + ltkn = self._thickness_type(name="%s_ltkn" % self.parent.name, - side=LEFT, value=thickness[0], - **kwargs) + side=LEFT, value=thickness[0], **kwargs) rtkn = self._thickness_type(name="%s_rtkn" % self.parent.name, - side=RIGHT, value=thickness[1], - **kwargs) + side=RIGHT, value=thickness[1], **kwargs) return (ltkn, rtkn) From 889803ba662f436fbacd96b482f411621f7e4932 Mon Sep 17 00:00:00 2001 From: Edward Caunt Date: Wed, 16 Oct 2024 13:58:53 +0100 Subject: [PATCH 07/16] compiler: Moved compiler gubbins from SubDimension to SubDimensionThickness --- devito/ir/equations/algorithms.py | 15 +++- devito/operator/operator.py | 2 + devito/types/dimension.py | 127 ++++-------------------------- 3 files changed, 28 insertions(+), 116 deletions(-) diff --git a/devito/ir/equations/algorithms.py b/devito/ir/equations/algorithms.py index 1457b79af9..c2d02a150a 100644 --- a/devito/ir/equations/algorithms.py +++ b/devito/ir/equations/algorithms.py @@ -174,6 +174,7 @@ def concretize_subdims(exprs, **kwargs): rebuilt = {} # Rebuilt implicit dims etc which are shared between dimensions _concretize_subdims(exprs, mapper, rebuilt, sregistry) + print("Concretization mapper", mapper) if not mapper: return exprs @@ -215,10 +216,16 @@ def _(expr, mapper, rebuilt, sregistry): @_concretize_subdims.register(SubDimensionThickness) def _(tkn, mapper, rebuilt, sregistry): - # Concretising the parent SubDimension ensures that SubDimension thicknesses - # are incremented in lockstep - # TODO: Finish - pass + # TODO: Can this be modified so SubDimension thicknesses always get + # generated in lockstep? -> Should concretise dimensions before thicknesses + # This way thicknesses get concretised with their parent dimensions, assuming + # their parent dimensions are actually present in the equations supplied to + # an operator + if tkn in mapper: + # Already have a substitution for this thickness + return + + mapper[tkn] = tkn._rebuild(name=sregistry.make_name(prefix=tkn.name)) @_concretize_subdims.register(SubDimension) diff --git a/devito/operator/operator.py b/devito/operator/operator.py index a8408e023f..f07a116ba9 100644 --- a/devito/operator/operator.py +++ b/devito/operator/operator.py @@ -687,6 +687,8 @@ def _prepare_arguments(self, autotune=None, **kwargs): # Execute autotuning and adjust arguments accordingly args.update(self._autotune(args, autotune or configuration['autotuning'])) + print("Args", args) + return args def _postprocess_errors(self, retval): diff --git a/devito/types/dimension.py b/devito/types/dimension.py index b0dfa6bfff..133bbf7fac 100644 --- a/devito/types/dimension.py +++ b/devito/types/dimension.py @@ -539,30 +539,19 @@ def _arg_check(self, *args, **kwargs): class SubDimensionThickness(DataSymbol): """A DataSymbol to represent a thickness of a SubDimension""" - # FIXME: Shouldn't store reference to the subdimension, just the - # metadata necessary to do arg_values etc - # FIXME: Should carry the root of its parent subdimension - # FIXME: Should also have some understanding of if it is associated with - # a left, right, or middle subdimension -> Should be subsumed into local - # FIXME: Should know if it is the left or right side - - # FIXME: Currently dumps an integer into the generated code + # TODO: This should live in types/basic.py __rkwargs__ = DataSymbol.__rkwargs__ + ('root', 'side', 'local', 'value') def __new__(cls, *args, **kwargs): - # TODO: Args or kwargs? - # What is the root dimension? try: root = kwargs.pop('root') except KeyError: raise ValueError("No root dimension provided") - # Is this thickness LEFT or RIGHT? try: side = kwargs.pop('side') except KeyError: raise ValueError("No side specified") - # Is this thickness for a local SubDimension? try: local = kwargs.pop('local') except KeyError: @@ -572,7 +561,6 @@ def __new__(cls, *args, **kwargs): newobj._root = root newobj._side = side newobj._local = local - # NOTE: Can determine correct override from the value, side, local, and root return newobj @@ -608,79 +596,33 @@ def _arg_values(self, grid=None, **kwargs): # However, arguments from the user are considered global # So overriding the thickness to a nonzero value should not cause # boundaries to exist between ranks where they did not before - - # r_ltkn, r_rtkn = ( - # kwargs.get(k.name, v) for k, v in self.thickness - # ) - - # if grid is not None and grid.is_distributed(self.root): - # # Get local thickness - # if self.local: - # # dimension is of type ``left``/right`` - compute the 'offset' - # # and then add 1 to get the appropriate thickness - # if r_ltkn is not None: - # ltkn = grid.distributor.glb_to_loc(self.root, r_ltkn-1, LEFT) - # ltkn = ltkn+1 if ltkn is not None else 0 - # else: - # ltkn = 0 - - # if r_rtkn is not None: - # rtkn = grid.distributor.glb_to_loc(self.root, r_rtkn-1, RIGHT) - # rtkn = rtkn+1 if rtkn is not None else 0 - # else: - # rtkn = 0 - # else: - # # dimension is of type ``middle`` - # ltkn = grid.distributor.glb_to_loc(self.root, r_ltkn, LEFT) or 0 - # rtkn = grid.distributor.glb_to_loc(self.root, r_rtkn, RIGHT) or 0 - # else: - # ltkn = r_ltkn or 0 - # rtkn = r_rtkn or 0 - - # return {i.name: v for i, v in zip(self._thickness_map, (ltkn, rtkn))} r_tkn = kwargs.get(self.name, self.value) - # FIXME: Can refactor and consolidate the checks for LEFT and RIGHT if grid is not None and grid.is_distributed(self.root): # Get local thickness if self.local: # Dimension is of type `left`/`right` - compute the offset # and then add 1 to get the appropriate thickness - if self.side is LEFT: - if self.value is not None: - ltkn = grid.distributor.glb_to_loc(self.root, r_tkn-1, LEFT) - ltkn = ltkn+1 if ltkn is not None else 0 - else: - ltkn = 0 - - elif self.side is RIGHT: - if self.value is not None: - rtkn = grid.distributor.glb_to_loc(self.root, r_tkn-1, RIGHT) - rtkn = rtkn+1 if rtkn is not None else 0 - else: - rtkn = 0 - + if self.value is not None: + tkn = grid.distributor.glb_to_loc(self.root, r_tkn-1, self.side) + tkn = tkn+1 if tkn is not None else 0 else: - raise ValueError("Unrecognised side %s" % self.side) - + tkn = 0 else: # Dimension is of type `middle` - if self.side is LEFT: - ltkn = grid.distributor.glb_to_loc(self.root, r_tkn, LEFT) or 0 - elif self.side is RIGHT: - rtkn = grid.distributor.glb_to_loc(self.root, r_tkn, RIGHT) or 0 - else: - raise ValueError("Unrecognised side %s" % self.side) - + tkn = grid.distributor.glb_to_loc(self.root, r_tkn, self.side) or 0 else: - ltkn = r_tkn or 0 - rtkn = r_tkn or 0 - + tkn = r_tkn or 0 + return {self.name: tkn} def _arg_finalize(self, *args, **kwargs): return {} + def _arg_apply(self, *args, **kwargs): + # TODO: What is this actually for? + pass + class AbstractSubDimension(DerivedDimension): @@ -705,20 +647,11 @@ def __init_finalize__(self, name, parent, thickness, **kwargs): self._process_thicknesses(thickness) def _process_thicknesses(self, thickness): - - # ltkn, rtkn = thickness - # if not isinstance(ltkn, tuple) or not isinstance(rtkn, tuple): - # FIXME: somewhat ugly - # ltkn, rtkn = tuple((sym, val) for sym, val - # in zip(self._symbolic_thickness, thickness)) ltkn, rtkn = self._symbolic_thickness(thickness=thickness) # FIXME: I think some weirdness may occur if one pickles then unpickles # a SubDimension, then uses only the thicknesses of that subdimension in # an Operator. This should be tested. - # FIXME: The numerical value of the thickness should be carried by the - # SubDimensionThickness symbols themselves, thereby making the Thickness - # namedtuple obsolete self._thickness = Thickness(ltkn, rtkn) # Just need to set up the interval accordingly @@ -900,39 +833,9 @@ def _arg_defaults(self, grid=None, **kwargs): return {} def _arg_values(self, interval, grid=None, **kwargs): - # Allow override of thickness values to disable BCs - # However, arguments from the user are considered global - # So overriding the thickness to a nonzero value should not cause - # boundaries to exist between ranks where they did not before - r_ltkn, r_rtkn = ( - kwargs.get(k.name, v) for k, v in self.thickness - ) - - if grid is not None and grid.is_distributed(self.root): - # Get local thickness - if self.local: - # dimension is of type ``left``/right`` - compute the 'offset' - # and then add 1 to get the appropriate thickness - if r_ltkn is not None: - ltkn = grid.distributor.glb_to_loc(self.root, r_ltkn-1, LEFT) - ltkn = ltkn+1 if ltkn is not None else 0 - else: - ltkn = 0 - - if r_rtkn is not None: - rtkn = grid.distributor.glb_to_loc(self.root, r_rtkn-1, RIGHT) - rtkn = rtkn+1 if rtkn is not None else 0 - else: - rtkn = 0 - else: - # dimension is of type ``middle`` - ltkn = grid.distributor.glb_to_loc(self.root, r_ltkn, LEFT) or 0 - rtkn = grid.distributor.glb_to_loc(self.root, r_rtkn, RIGHT) or 0 - else: - ltkn = r_ltkn or 0 - rtkn = r_rtkn or 0 - - return {i.name: v for i, v in zip(self._thickness_map, (ltkn, rtkn))} + # SubDimension thicknesses at runtime are calculated by the thicknesses + # themselves + return {} class MultiSubDimension(AbstractSubDimension): From ccdc2e0314acffae6df9334b319c06f24da23b28 Mon Sep 17 00:00:00 2001 From: Edward Caunt Date: Wed, 16 Oct 2024 16:29:23 +0100 Subject: [PATCH 08/16] tests; Fix up tests and update concretisation functionality --- devito/ir/equations/algorithms.py | 20 +++----------------- devito/ir/support/basic.py | 2 +- devito/mpi/halo_scheme.py | 3 +-- devito/operator/operator.py | 2 -- devito/types/dimension.py | 19 +++++++++---------- tests/test_caching.py | 2 +- tests/test_dimension.py | 6 +++--- tests/test_subdomains.py | 2 +- 8 files changed, 19 insertions(+), 37 deletions(-) diff --git a/devito/ir/equations/algorithms.py b/devito/ir/equations/algorithms.py index c2d02a150a..c3de723176 100644 --- a/devito/ir/equations/algorithms.py +++ b/devito/ir/equations/algorithms.py @@ -174,7 +174,6 @@ def concretize_subdims(exprs, **kwargs): rebuilt = {} # Rebuilt implicit dims etc which are shared between dimensions _concretize_subdims(exprs, mapper, rebuilt, sregistry) - print("Concretization mapper", mapper) if not mapper: return exprs @@ -216,11 +215,6 @@ def _(expr, mapper, rebuilt, sregistry): @_concretize_subdims.register(SubDimensionThickness) def _(tkn, mapper, rebuilt, sregistry): - # TODO: Can this be modified so SubDimension thicknesses always get - # generated in lockstep? -> Should concretise dimensions before thicknesses - # This way thicknesses get concretised with their parent dimensions, assuming - # their parent dimensions are actually present in the equations supplied to - # an operator if tkn in mapper: # Already have a substitution for this thickness return @@ -234,17 +228,9 @@ def _(d, mapper, rebuilt, sregistry): # Already have a substitution for this dimension return - tkns_subs = {tkn: tkn._rebuild(name=sregistry.make_name(prefix=tkn.name)) - for tkn in d.tkns} - - # Add the thickness substitutions to the mapper, as they may appear without the - # SubDimension - mapper.update(tkns_subs) - - left, right = [mM.subs(tkns_subs) for mM in (d.symbolic_min, d.symbolic_max)] - thickness = tuple((v, d._thickness_map[k]) for k, v in tkns_subs.items()) - - mapper[d] = d._rebuild(symbolic_min=left, symbolic_max=right, thickness=thickness) + tkns = tuple(t._rebuild(name=sregistry.make_name(prefix=t.name)) for t in d.tkns) + mapper.update({t0: t1 for t0, t1 in zip(d.tkns, tkns)}) + mapper[d] = d._rebuild(thickness=tkns) @_concretize_subdims.register(ConditionalDimension) diff --git a/devito/ir/support/basic.py b/devito/ir/support/basic.py index 3d6bb3ba60..bb558c8d58 100644 --- a/devito/ir/support/basic.py +++ b/devito/ir/support/basic.py @@ -1376,7 +1376,7 @@ def disjoint_test(e0, e1, d, it): if d.is_Custom: subs = {} elif d.is_Sub and d.is_left: - subs = {d.root.symbolic_min: 0, **dict([d.thickness.left])} + subs = {d.root.symbolic_min: 0, d.ltkn: d.ltkn.value} else: return False diff --git a/devito/mpi/halo_scheme.py b/devito/mpi/halo_scheme.py index 0062b5b32f..7074b4c5d5 100644 --- a/devito/mpi/halo_scheme.py +++ b/devito/mpi/halo_scheme.py @@ -89,8 +89,7 @@ def __init__(self, exprs, ispace): if d._defines & self.dimensions]) subdims = [d for d in dims if d.is_Sub and not d.local] for i in subdims: - ltk, _ = i.thickness.left - rtk, _ = i.thickness.right + ltk, rtk = i.tkns self._honored[i.root] = frozenset([(ltk, rtk)]) self._honored = frozendict(self._honored) diff --git a/devito/operator/operator.py b/devito/operator/operator.py index f07a116ba9..a8408e023f 100644 --- a/devito/operator/operator.py +++ b/devito/operator/operator.py @@ -687,8 +687,6 @@ def _prepare_arguments(self, autotune=None, **kwargs): # Execute autotuning and adjust arguments accordingly args.update(self._autotune(args, autotune or configuration['autotuning'])) - print("Args", args) - return args def _postprocess_errors(self, retval): diff --git a/devito/types/dimension.py b/devito/types/dimension.py index 133bbf7fac..756d367087 100644 --- a/devito/types/dimension.py +++ b/devito/types/dimension.py @@ -13,7 +13,6 @@ from devito.types.args import ArgProvider from devito.types.basic import Symbol, DataSymbol, Scalar from devito.types.constant import Constant -from devito.types.caching import Uncached __all__ = ['Dimension', 'SpaceDimension', 'TimeDimension', 'DefaultDimension', @@ -539,7 +538,6 @@ def _arg_check(self, *args, **kwargs): class SubDimensionThickness(DataSymbol): """A DataSymbol to represent a thickness of a SubDimension""" - # TODO: This should live in types/basic.py __rkwargs__ = DataSymbol.__rkwargs__ + ('root', 'side', 'local', 'value') @@ -647,14 +645,14 @@ def __init_finalize__(self, name, parent, thickness, **kwargs): self._process_thicknesses(thickness) def _process_thicknesses(self, thickness): - ltkn, rtkn = self._symbolic_thickness(thickness=thickness) + if any(isinstance(tkn, SubDimensionThickness) for tkn in thickness): + ltkn, rtkn = thickness + else: + ltkn, rtkn = self._symbolic_thickness(thickness=thickness) - # FIXME: I think some weirdness may occur if one pickles then unpickles - # a SubDimension, then uses only the thicknesses of that subdimension in - # an Operator. This should be tested. self._thickness = Thickness(ltkn, rtkn) - # Just need to set up the interval accordingly + # Set up the interval if self.thickness.right.value is None: # Left SubDimension left = self.parent.symbolic_min right = self.parent.symbolic_min + self.ltkn - 1 @@ -806,11 +804,11 @@ def local(self): @property def is_left(self): - return self.thickness.right[1] is None + return self.thickness.right.value is None @property def is_right(self): - return self.thickness.left[1] is None + return self.thickness.left.value is None @property def is_middle(self): @@ -827,7 +825,7 @@ def _maybe_distributed(self): @property def _arg_names(self): - return tuple(k.name for k, _ in self.thickness) + self.parent._arg_names + return tuple(k.name for k in self.thickness) + self.parent._arg_names def _arg_defaults(self, grid=None, **kwargs): return {} @@ -852,6 +850,7 @@ class MultiSubDimension(AbstractSubDimension): _thickness_type = Symbol def _process_thicknesses(self, thickness): + # FIXME: Requires homogensisation with SubDimension._process_thickness # Canonicalize thickness if thickness is None: # Using dummy left/right is the only thing we can do for such diff --git a/tests/test_caching.py b/tests/test_caching.py index 810d9c81d2..a673571e03 100644 --- a/tests/test_caching.py +++ b/tests/test_caching.py @@ -842,7 +842,7 @@ def test_solve(self, operate_on_empty_cache): # created by the finite difference (u.dt, u.dx2). We would have had # three extra references to u(t + dt), u(x - h_x) and u(x + h_x). # But this is not the case anymore! - assert len(_SymbolCache) == 12 + assert len(_SymbolCache) == 10 clear_cache() assert len(_SymbolCache) == 6 clear_cache() diff --git a/tests/test_dimension.py b/tests/test_dimension.py index 6e63159cdb..dbd3c1521e 100644 --- a/tests/test_dimension.py +++ b/tests/test_dimension.py @@ -323,15 +323,15 @@ def test_symbolic_size(self): xleft = SubDimension.left(name='xleft', parent=x, thickness=thickness) assert xleft.is_left assert not xleft.is_middle - assert xleft.symbolic_size == xleft.thickness.left[0] + assert xleft.symbolic_size == xleft.thickness.left xi = SubDimension.middle(name='xi', parent=x, thickness_left=thickness, thickness_right=thickness) assert xi.symbolic_size == (x.symbolic_max - x.symbolic_min - - xi.thickness.left[0] - xi.thickness.right[0] + 1) + xi.thickness.left - xi.thickness.right + 1) xright = SubDimension.right(name='xright', parent=x, thickness=thickness) - assert xright.symbolic_size == xright.thickness.right[0] + assert xright.symbolic_size == xright.thickness.right @pytest.mark.parametrize('opt', opts_tiling) def test_bcs(self, opt): diff --git a/tests/test_subdomains.py b/tests/test_subdomains.py index d95fec84bb..784dc1c3ad 100644 --- a/tests/test_subdomains.py +++ b/tests/test_subdomains.py @@ -197,7 +197,7 @@ def define(self, dimensions): check = np.zeros(grid.shape) mM_map = {x.symbolic_min: 0, x.symbolic_max: grid.shape[0]-1} - t_map = {k: v for k, v in xd0._thickness_map.items() if v is not None} + t_map = {tkn: tkn.value for tkn in xd0.thickness if tkn.value is not None} start = int(xd0.symbolic_min.subs({**mM_map, **t_map})) stop = int(xd0.symbolic_max.subs({**mM_map, **t_map})+1) From 72e76d08e3aee05e508326b46ac98912c4be431d Mon Sep 17 00:00:00 2001 From: Edward Caunt Date: Thu, 17 Oct 2024 11:58:55 +0100 Subject: [PATCH 09/16] tests: Fix up symbolics tests --- tests/test_symbolics.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_symbolics.py b/tests/test_symbolics.py index 353fdc934c..496e325387 100644 --- a/tests/test_symbolics.py +++ b/tests/test_symbolics.py @@ -130,15 +130,15 @@ def test_subdimension(): di = SubDimension.middle(name='di', parent=d, thickness_left=4, thickness_right=4) assert di.free_symbols == {di} - assert di.bound_symbols == {d.symbolic_min, d.symbolic_max} | set(di._thickness_map) + assert di.bound_symbols == {d.symbolic_min, d.symbolic_max} | set(di.thickness) dl = SubDimension.left(name='dl', parent=d, thickness=4) assert dl.free_symbols == {dl} - assert dl.bound_symbols == {d.symbolic_min, dl.thickness.left[0]} + assert dl.bound_symbols == {d.symbolic_min, dl.thickness.left} dr = SubDimension.right(name='dr', parent=d, thickness=4) assert dr.free_symbols == {dr} - assert dr.bound_symbols == {d.symbolic_max, dr.thickness.right[0]} + assert dr.bound_symbols == {d.symbolic_max, dr.thickness.right} def test_timefunction(): From 196460cb4a4ce3c3d040bb065abf99b52653a8a9 Mon Sep 17 00:00:00 2001 From: Edward Caunt Date: Thu, 17 Oct 2024 16:49:31 +0100 Subject: [PATCH 10/16] dsl: Continue refactoring SubDimensions --- devito/ir/equations/algorithms.py | 8 +- devito/types/dimension.py | 127 +++++++++--------------------- tests/test_caching.py | 4 +- tests/test_dimension.py | 12 +++ 4 files changed, 54 insertions(+), 97 deletions(-) diff --git a/devito/ir/equations/algorithms.py b/devito/ir/equations/algorithms.py index c3de723176..175311d22e 100644 --- a/devito/ir/equations/algorithms.py +++ b/devito/ir/equations/algorithms.py @@ -267,11 +267,9 @@ def _(d, mapper, rebuilt, sregistry): # Already have a substitution for this dimension return - abstract_tkns = d._symbolic_thickness - concrete_tkns = tuple(tkn._rebuild(name=sregistry.make_name(prefix=tkn.name)) - for tkn in abstract_tkns) - - kwargs = {'thickness': concrete_tkns} + tkns = tuple(tkn._rebuild(name=sregistry.make_name(prefix=tkn.name)) + for tkn in d.thickness) + kwargs = {'thickness': tkns} fkwargs = {} idim0 = d.implicit_dimension diff --git a/devito/types/dimension.py b/devito/types/dimension.py index 756d367087..2b22d8c9fe 100644 --- a/devito/types/dimension.py +++ b/devito/types/dimension.py @@ -9,7 +9,7 @@ from devito.data import LEFT, RIGHT from devito.exceptions import InvalidArgument from devito.logger import debug -from devito.tools import Pickable, is_integer, flatten, memoized_meth +from devito.tools import Pickable, is_integer, memoized_meth from devito.types.args import ArgProvider from devito.types.basic import Symbol, DataSymbol, Scalar from devito.types.constant import Constant @@ -542,18 +542,9 @@ class SubDimensionThickness(DataSymbol): __rkwargs__ = DataSymbol.__rkwargs__ + ('root', 'side', 'local', 'value') def __new__(cls, *args, **kwargs): - try: - root = kwargs.pop('root') - except KeyError: - raise ValueError("No root dimension provided") - try: - side = kwargs.pop('side') - except KeyError: - raise ValueError("No side specified") - try: - local = kwargs.pop('local') - except KeyError: - raise ValueError("Locality of thickness not specified") + root = kwargs.pop('root') + side = kwargs.pop('side') + local = kwargs.pop('local') newobj = super().__new__(cls, *args, **kwargs) newobj._root = root @@ -595,7 +586,6 @@ def _arg_values(self, grid=None, **kwargs): # So overriding the thickness to a nonzero value should not cause # boundaries to exist between ranks where they did not before r_tkn = kwargs.get(self.name, self.value) - if grid is not None and grid.is_distributed(self.root): # Get local thickness if self.local: @@ -638,21 +628,18 @@ class AbstractSubDimension(DerivedDimension): __rargs__ = DerivedDimension.__rargs__ + ('thickness',) __rkwargs__ = () - _thickness_type = SubDimensionThickness + _thickness_type = Symbol def __init_finalize__(self, name, parent, thickness, **kwargs): super().__init_finalize__(name, parent) - self._process_thicknesses(thickness) - - def _process_thicknesses(self, thickness): - if any(isinstance(tkn, SubDimensionThickness) for tkn in thickness): - ltkn, rtkn = thickness + thickness = thickness or (None, None) + if any(isinstance(tkn, self._thickness_type) for tkn in thickness): + self._thickness = Thickness(*thickness) else: - ltkn, rtkn = self._symbolic_thickness(thickness=thickness) + self._thickness = self._symbolic_thickness(thickness=thickness) - self._thickness = Thickness(ltkn, rtkn) - - # Set up the interval + @cached_property + def _interval(self): if self.thickness.right.value is None: # Left SubDimension left = self.parent.symbolic_min right = self.parent.symbolic_min + self.ltkn - 1 @@ -663,21 +650,14 @@ def _process_thicknesses(self, thickness): left = self.parent.symbolic_min + self.ltkn right = self.parent.symbolic_max - self.rtkn - self._interval = sympy.Interval(left, right) + return sympy.Interval(left, right) @memoized_meth - def _symbolic_thickness(self, thickness=None): - thickness = thickness or (None, None) - kwargs = {'dtype': np.int64, 'is_const': True, 'nonnegative': True, - 'root': self.root, 'local': self.local} - - ltkn = self._thickness_type(name="%s_ltkn" % self.parent.name, - side=LEFT, value=thickness[0], **kwargs) + def _symbolic_thickness(self, **kwargs): + kwargs = {'dtype': np.int32, 'is_const': True, 'nonnegative': True} - rtkn = self._thickness_type(name="%s_rtkn" % self.parent.name, - side=RIGHT, value=thickness[1], **kwargs) - - return (ltkn, rtkn) + names = ["%s_%stkn" % (self.parent.name, s) for s in ('l', 'r')] + return Thickness(*[Symbol(name=n, **kwargs) for n in names]) @cached_property def symbolic_min(self): @@ -696,9 +676,7 @@ def symbolic_size(self): def thickness(self): return self._thickness - @property - def is_abstract(self): - return all(i is None for i in flatten(self.thickness)) + tkns = thickness # Shortcut for thickness @property def ltkn(self): @@ -710,12 +688,6 @@ def rtkn(self): # Shortcut for the right thickness symbol return self.thickness.right - @property - def tkns(self): - # Shortcut for both thickness symbols - # FIXME: Maybe redundant now? - return self.ltkn, self.rtkn - def __hash__(self): return id(self) @@ -772,6 +744,8 @@ class SubDimension(AbstractSubDimension): __rargs__ = AbstractSubDimension.__rargs__ + ('local',) + _thickness_type = SubDimensionThickness + def __init_finalize__(self, name, parent, thickness, local, **kwargs): self._local = local @@ -779,24 +753,24 @@ def __init_finalize__(self, name, parent, thickness, local, @classmethod def left(cls, name, parent, thickness, local=True): - return cls(name, parent, - thickness=(thickness, None), - # thickness=((lst, thickness), (rst, None)), - local=local) + return cls(name, parent, thickness=(thickness, None), local=local) @classmethod def right(cls, name, parent, thickness, local=True): - return cls(name, parent, - thickness=(None, thickness), - # thickness=((lst, None), (rst, thickness)), - local=local) + return cls(name, parent, thickness=(None, thickness), local=local) @classmethod def middle(cls, name, parent, thickness_left, thickness_right, local=False): - return cls(name, parent, - thickness=(thickness_left, thickness_right), - # thickness=((lst, thickness_left), (rst, thickness_right)), - local=local) + return cls(name, parent, thickness=(thickness_left, thickness_right), local=local) + + @memoized_meth + def _symbolic_thickness(self, thickness=None): + kwargs = {'dtype': np.int64, 'is_const': True, 'nonnegative': True, + 'root': self.root, 'local': self.local} + + names = ["%s_%stkn" % (self.parent.name, s) for s in ('l', 'r')] + return Thickness(*[SubDimensionThickness(name=n, side=s, value=t, **kwargs) + for n, s, t in zip(names, (LEFT, RIGHT), thickness)]) @property def local(self): @@ -846,39 +820,6 @@ class MultiSubDimension(AbstractSubDimension): __rkwargs__ = ('functions', 'bounds_indices', 'implicit_dimension') - # FIXME: Is this still necessary? - _thickness_type = Symbol - - def _process_thicknesses(self, thickness): - # FIXME: Requires homogensisation with SubDimension._process_thickness - # Canonicalize thickness - if thickness is None: - # Using dummy left/right is the only thing we can do for such - # an abstract MultiSubDimension - thickness = ((None, None), (None, None)) - - left = sympy.S.NegativeInfinity - right = sympy.S.Infinity - elif isinstance(thickness, tuple): - if all(isinstance(i, Symbol) for i in thickness): - ltkn, rtkn = thickness - thickness = ((ltkn, None), (rtkn, None)) - elif all(isinstance(i, tuple) and len(i) == 2 for i in thickness): - (ltkn, _), (rtkn, _) = thickness - - try: - left = self.parent.symbolic_min + ltkn - right = self.parent.symbolic_max - rtkn - except TypeError: - # May end up here after a reconstruction - left = sympy.S.NegativeInfinity - right = sympy.S.Infinity - else: - raise ValueError("MultiSubDimension expects a tuple of thicknesses") - - self._thickness = Thickness(*thickness) - self._interval = sympy.Interval(left, right) - def __init_finalize__(self, name, parent, thickness, functions=None, bounds_indices=None, implicit_dimension=None): @@ -887,6 +828,12 @@ def __init_finalize__(self, name, parent, thickness, functions=None, self.bounds_indices = bounds_indices self.implicit_dimension = implicit_dimension + @cached_property + def _interval(self): + left = self.parent.symbolic_min + self.ltkn + right = self.parent.symbolic_max - self.rtkn + return sympy.Interval(left, right) + @cached_property def bound_symbols(self): return self.parent.bound_symbols diff --git a/tests/test_caching.py b/tests/test_caching.py index a673571e03..bbbb98d316 100644 --- a/tests/test_caching.py +++ b/tests/test_caching.py @@ -842,9 +842,9 @@ def test_solve(self, operate_on_empty_cache): # created by the finite difference (u.dt, u.dx2). We would have had # three extra references to u(t + dt), u(x - h_x) and u(x + h_x). # But this is not the case anymore! - assert len(_SymbolCache) == 10 + assert len(_SymbolCache) == 8 clear_cache() - assert len(_SymbolCache) == 6 + assert len(_SymbolCache) == 4 clear_cache() assert len(_SymbolCache) == 2 clear_cache() diff --git a/tests/test_dimension.py b/tests/test_dimension.py index dbd3c1521e..84df27ecef 100644 --- a/tests/test_dimension.py +++ b/tests/test_dimension.py @@ -787,6 +787,18 @@ def test_expandingbox_like(self, opt): assert np.all(u.data[:, :, :2] == 0.) assert np.all(u.data[:, :, -2:] == 0.) + def test_standalone_thickness(self): + x = Dimension('x') + ix = SubDimension.left('ix', x, 2) + f = Function(name="f", dimensions=(ix,), shape=(5,), dtype=np.int32) + + eqns = Eq(f[ix.symbolic_max], 1) + + op = Operator(eqns) + assert 'x_ltkn0' in str(op.ccode) + op(x_m=0) + assert np.all(f.data == np.array([0, 1, 0, 0, 0])) + class TestConditionalDimension: From 96e29590df048326b4d478f6611a0053f2807928 Mon Sep 17 00:00:00 2001 From: Edward Caunt Date: Fri, 18 Oct 2024 15:04:13 +0100 Subject: [PATCH 11/16] compiler: Moved thickness arg vals in operator --- devito/operator/operator.py | 8 ++++---- devito/types/dimension.py | 1 - 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/devito/operator/operator.py b/devito/operator/operator.py index a8408e023f..1c9989c4e6 100644 --- a/devito/operator/operator.py +++ b/devito/operator/operator.py @@ -648,15 +648,15 @@ def _prepare_arguments(self, autotune=None, **kwargs): for d in reversed(toposort): args.update(d._arg_values(self._dspace[d], grid, **kwargs)) - # Process Objects - for o in self.objects: - args.update(o._arg_values(grid=grid, **kwargs)) - # Process SubDimensionThicknesses for p in self.parameters: if isinstance(p, SubDimensionThickness): args.update(p._arg_values(grid=grid, **kwargs)) + # Process Objects + for o in self.objects: + args.update(o._arg_values(grid=grid, **kwargs)) + # Purge `kwargs` kwargs.pop('args') kwargs.pop('metadata') diff --git a/devito/types/dimension.py b/devito/types/dimension.py index 2b22d8c9fe..4d622ac7c3 100644 --- a/devito/types/dimension.py +++ b/devito/types/dimension.py @@ -23,7 +23,6 @@ Thickness = namedtuple('Thickness', 'left right') -SubDimensionOffset = namedtuple('SubDimensionOffset', 'value extreme thickness') class Dimension(ArgProvider): From 5274f815e322ac5ea8d6e692fe6e013c4c658a21 Mon Sep 17 00:00:00 2001 From: Edward Caunt Date: Fri, 18 Oct 2024 15:14:43 +0100 Subject: [PATCH 12/16] compiler: Remove leftovers from operator --- devito/operator/operator.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/devito/operator/operator.py b/devito/operator/operator.py index 1c9989c4e6..0a7e0dc7da 100644 --- a/devito/operator/operator.py +++ b/devito/operator/operator.py @@ -504,12 +504,6 @@ def dimensions(self): # During compilation other Dimensions may have been produced dimensions = FindSymbols('dimensions').visit(self) - - # SubDimensions may only be present in the operator in the form of - # their thicknesses. These SubDimensions should be recovered if not - # already present in the operator dimensions. - ret.update(d for d in dimensions if d.is_Sub) - ret.update(d for d in dimensions if d.is_PerfKnob) ret = tuple(sorted(ret, key=attrgetter('name'))) From 376680869e0ea412bc857c47dbc2977fe6d040ba Mon Sep 17 00:00:00 2001 From: Edward Caunt Date: Fri, 18 Oct 2024 15:21:39 +0100 Subject: [PATCH 13/16] dsl: Rework SubDimension types for better inheritance --- devito/types/dimension.py | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/devito/types/dimension.py b/devito/types/dimension.py index 4d622ac7c3..d3e531594a 100644 --- a/devito/types/dimension.py +++ b/devito/types/dimension.py @@ -639,16 +639,8 @@ def __init_finalize__(self, name, parent, thickness, **kwargs): @cached_property def _interval(self): - if self.thickness.right.value is None: # Left SubDimension - left = self.parent.symbolic_min - right = self.parent.symbolic_min + self.ltkn - 1 - elif self.thickness.left.value is None: # Right SubDimension - left = self.parent.symbolic_max - self.rtkn + 1 - right = self.parent.symbolic_max - else: # Middle SubDimension - left = self.parent.symbolic_min + self.ltkn - right = self.parent.symbolic_max - self.rtkn - + left = self.parent.symbolic_min + self.ltkn + right = self.parent.symbolic_max - self.rtkn return sympy.Interval(left, right) @memoized_meth @@ -771,6 +763,19 @@ def _symbolic_thickness(self, thickness=None): return Thickness(*[SubDimensionThickness(name=n, side=s, value=t, **kwargs) for n, s, t in zip(names, (LEFT, RIGHT), thickness)]) + @cached_property + def _interval(self): + if self.thickness.right.value is None: # Left SubDimension + left = self.parent.symbolic_min + right = self.parent.symbolic_min + self.ltkn - 1 + elif self.thickness.left.value is None: # Right SubDimension + left = self.parent.symbolic_max - self.rtkn + 1 + right = self.parent.symbolic_max + else: # Middle SubDimension + return super()._interval + + return sympy.Interval(left, right) + @property def local(self): return self._local @@ -827,12 +832,6 @@ def __init_finalize__(self, name, parent, thickness, functions=None, self.bounds_indices = bounds_indices self.implicit_dimension = implicit_dimension - @cached_property - def _interval(self): - left = self.parent.symbolic_min + self.ltkn - right = self.parent.symbolic_max - self.rtkn - return sympy.Interval(left, right) - @cached_property def bound_symbols(self): return self.parent.bound_symbols From f5786d2fbe212563985ec84674bdbaac82d70197 Mon Sep 17 00:00:00 2001 From: Edward Caunt Date: Fri, 18 Oct 2024 15:56:18 +0100 Subject: [PATCH 14/16] misc: Fix up class names --- devito/ir/equations/algorithms.py | 4 ++-- devito/operator/operator.py | 6 +++--- devito/types/dimension.py | 23 ++++++++++------------- 3 files changed, 15 insertions(+), 18 deletions(-) diff --git a/devito/ir/equations/algorithms.py b/devito/ir/equations/algorithms.py index 175311d22e..8913f81dde 100644 --- a/devito/ir/equations/algorithms.py +++ b/devito/ir/equations/algorithms.py @@ -8,7 +8,7 @@ ConditionalDimension) from devito.types.array import Array from devito.types.basic import AbstractFunction -from devito.types.dimension import MultiSubDimension, SubDimensionThickness +from devito.types.dimension import MultiSubDimension, Thickness from devito.data.allocators import DataReference from devito.logger import warning @@ -213,7 +213,7 @@ def _(expr, mapper, rebuilt, sregistry): _concretize_subdims(expr.implicit_dims, mapper, rebuilt, sregistry) -@_concretize_subdims.register(SubDimensionThickness) +@_concretize_subdims.register(Thickness) def _(tkn, mapper, rebuilt, sregistry): if tkn in mapper: # Already have a substitution for this thickness diff --git a/devito/operator/operator.py b/devito/operator/operator.py index 0a7e0dc7da..9bfdcdc255 100644 --- a/devito/operator/operator.py +++ b/devito/operator/operator.py @@ -31,7 +31,7 @@ split, timed_pass, timed_region, contains_val) from devito.types import (Buffer, Grid, Evaluable, host_layer, device_layer, disk_layer) -from devito.types.dimension import SubDimensionThickness +from devito.types.dimension import Thickness __all__ = ['Operator'] @@ -642,9 +642,9 @@ def _prepare_arguments(self, autotune=None, **kwargs): for d in reversed(toposort): args.update(d._arg_values(self._dspace[d], grid, **kwargs)) - # Process SubDimensionThicknesses + # Process Thicknesses for p in self.parameters: - if isinstance(p, SubDimensionThickness): + if isinstance(p, Thickness): args.update(p._arg_values(grid=grid, **kwargs)) # Process Objects diff --git a/devito/types/dimension.py b/devito/types/dimension.py index d3e531594a..6f7ae89c67 100644 --- a/devito/types/dimension.py +++ b/devito/types/dimension.py @@ -22,7 +22,7 @@ 'VirtualDimension', 'Spacing', 'dimensions'] -Thickness = namedtuple('Thickness', 'left right') +SubDimensionThickness = namedtuple('SubDimensionThickness', 'left right') class Dimension(ArgProvider): @@ -535,16 +535,12 @@ def _arg_check(self, *args, **kwargs): # The Dimensions below are exposed in the user API. They can only be created by # the user -class SubDimensionThickness(DataSymbol): +class Thickness(DataSymbol): """A DataSymbol to represent a thickness of a SubDimension""" __rkwargs__ = DataSymbol.__rkwargs__ + ('root', 'side', 'local', 'value') - def __new__(cls, *args, **kwargs): - root = kwargs.pop('root') - side = kwargs.pop('side') - local = kwargs.pop('local') - + def __new__(cls, *args, root=None, side=None, local=False, **kwargs): newobj = super().__new__(cls, *args, **kwargs) newobj._root = root newobj._side = side @@ -633,7 +629,7 @@ def __init_finalize__(self, name, parent, thickness, **kwargs): super().__init_finalize__(name, parent) thickness = thickness or (None, None) if any(isinstance(tkn, self._thickness_type) for tkn in thickness): - self._thickness = Thickness(*thickness) + self._thickness = SubDimensionThickness(*thickness) else: self._thickness = self._symbolic_thickness(thickness=thickness) @@ -648,7 +644,7 @@ def _symbolic_thickness(self, **kwargs): kwargs = {'dtype': np.int32, 'is_const': True, 'nonnegative': True} names = ["%s_%stkn" % (self.parent.name, s) for s in ('l', 'r')] - return Thickness(*[Symbol(name=n, **kwargs) for n in names]) + return SubDimensionThickness(*[Symbol(name=n, **kwargs) for n in names]) @cached_property def symbolic_min(self): @@ -735,7 +731,7 @@ class SubDimension(AbstractSubDimension): __rargs__ = AbstractSubDimension.__rargs__ + ('local',) - _thickness_type = SubDimensionThickness + _thickness_type = Thickness def __init_finalize__(self, name, parent, thickness, local, **kwargs): @@ -756,12 +752,13 @@ def middle(cls, name, parent, thickness_left, thickness_right, local=False): @memoized_meth def _symbolic_thickness(self, thickness=None): - kwargs = {'dtype': np.int64, 'is_const': True, 'nonnegative': True, + kwargs = {'dtype': np.int32, 'is_const': True, 'nonnegative': True, 'root': self.root, 'local': self.local} names = ["%s_%stkn" % (self.parent.name, s) for s in ('l', 'r')] - return Thickness(*[SubDimensionThickness(name=n, side=s, value=t, **kwargs) - for n, s, t in zip(names, (LEFT, RIGHT), thickness)]) + return SubDimensionThickness(*[Thickness(name=n, side=s, value=t, **kwargs) + for n, s, t + in zip(names, (LEFT, RIGHT), thickness)]) @cached_property def _interval(self): From 4e0318af0fa21af2269fb70af893e053b1f8388a Mon Sep 17 00:00:00 2001 From: Edward Caunt Date: Mon, 28 Oct 2024 13:47:24 +0200 Subject: [PATCH 15/16] misc: Adjust tests and tidy variable names for clarity --- devito/ir/equations/algorithms.py | 2 +- tests/test_dle.py | 6 +++--- tests/test_symbolic_coefficients.py | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/devito/ir/equations/algorithms.py b/devito/ir/equations/algorithms.py index 8913f81dde..e0281e1139 100644 --- a/devito/ir/equations/algorithms.py +++ b/devito/ir/equations/algorithms.py @@ -229,7 +229,7 @@ def _(d, mapper, rebuilt, sregistry): return tkns = tuple(t._rebuild(name=sregistry.make_name(prefix=t.name)) for t in d.tkns) - mapper.update({t0: t1 for t0, t1 in zip(d.tkns, tkns)}) + mapper.update({tkn0: tkn1 for tkn0, tkn1 in zip(d.tkns, tkns)}) mapper[d] = d._rebuild(thickness=tkns) diff --git a/tests/test_dle.py b/tests/test_dle.py index 45583d4346..7f1ae73186 100644 --- a/tests/test_dle.py +++ b/tests/test_dle.py @@ -146,7 +146,7 @@ def test_cache_blocking_structure_subdims(): # zi is rebuilt with name z, so check symbolic max and min are preserved # Also check the zi was rebuilt assert not tree[4].dim.is_Block and tree[4].dim is not zi and\ - str(tree[4].dim.symbolic_min) == 'z_ltkn0 + z_m' and\ + str(tree[4].dim.symbolic_min) == 'z_m + z_ltkn0' and\ str(tree[4].dim.symbolic_max) == 'z_M - z_rtkn0' and\ tree[4].dim.parent is z @@ -1374,7 +1374,7 @@ def test_nested_cache_blocking_structure_subdims(self, blocklevels): if blocklevels == 1: assert not tree[4].dim.is_Block and tree[4].dim is not zi and\ - str(tree[4].dim.symbolic_min) == 'z_ltkn0 + z_m' and\ + str(tree[4].dim.symbolic_min) == 'z_m + z_ltkn0' and\ str(tree[4].dim.symbolic_max) == 'z_M - z_rtkn0' and\ tree[4].dim.parent is z elif blocklevels == 2: @@ -1385,7 +1385,7 @@ def test_nested_cache_blocking_structure_subdims(self, blocklevels): assert tree[5].dim.is_Block and tree[5].dim.parent is tree[3].dim and\ tree[5].dim.root is y assert not tree[6].dim.is_Block and tree[6].dim is not zi and\ - str(tree[6].dim.symbolic_min) == 'z_ltkn0 + z_m' and\ + str(tree[6].dim.symbolic_min) == 'z_m + z_ltkn0' and\ str(tree[6].dim.symbolic_max) == 'z_M - z_rtkn0' and\ tree[6].dim.parent is z diff --git a/tests/test_symbolic_coefficients.py b/tests/test_symbolic_coefficients.py index 30a89b155b..00dd80c765 100644 --- a/tests/test_symbolic_coefficients.py +++ b/tests/test_symbolic_coefficients.py @@ -200,8 +200,8 @@ def test_staggered_equation(self): eq_f = Eq(f, f.dx2(weights=weights)) - expected = 'Eq(f(x + h_x/2), 1.0*f(x - h_x/2) - 2.0*f(x + h_x/2)'\ - ' + 1.0*f(x + 3*h_x/2))' + expected = 'Eq(f(x + h_x/2), f(x - h_x/2) - 2.0*f(x + h_x/2)'\ + ' + f(x + 3*h_x/2))' assert(str(eq_f.evaluate) == expected) @pytest.mark.parametrize('stagger', [True, False]) From 4182ae3f13b729ea35acce9fa004ad33e96ab75c Mon Sep 17 00:00:00 2001 From: Edward Caunt Date: Mon, 28 Oct 2024 15:21:15 +0200 Subject: [PATCH 16/16] tests: Tweak caching tests to reflect lazy touch of Dimension bounds --- tests/test_caching.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/tests/test_caching.py b/tests/test_caching.py index bbbb98d316..143e949168 100644 --- a/tests/test_caching.py +++ b/tests/test_caching.py @@ -663,8 +663,8 @@ def test_sparse_function(self, operate_on_empty_cache): i = u.inject(expr=u, field=u) # created: rux, ruy (radius dimensions) and spacings - # posx, posy, px, py, u_coords (as indexified), - ncreated = 2+1+2+2+2+1 + # posx, posy, px, py, u_coords (as indexified), x_m, x_M, y_m, y_M + ncreated = 2+1+2+2+2+1+4 # Note that injection is now lazy so no new symbols should be created assert len(_SymbolCache) == cur_cache_size i.evaluate @@ -684,14 +684,16 @@ def test_sparse_function(self, operate_on_empty_cache): # in the first clear_cache they were still referenced by their "parent" objects # (e.g., ru* by ConditionalDimensions, through `condition`) - assert len(_SymbolCache) == init_cache_size + 8 + assert len(_SymbolCache) == init_cache_size + 12 clear_cache() # Now we should be back to the original state except for - # pos* that belong to the abstract class - assert len(_SymbolCache) == init_cache_size + 2 + # pos* that belong to the abstract class and the dimension + # bounds (x_m, x_M, y_m, y_M) + assert len(_SymbolCache) == init_cache_size + 6 clear_cache() - # Now we should be back to the original state - assert len(_SymbolCache) == init_cache_size + # Now we should be back to the original state plus the dimension bounds + # (x_m, x_M, y_m, y_M). + assert len(_SymbolCache) == init_cache_size + 4 def test_after_indexification(self): """