From 25dc268d0946a3277c7f9334258146ded74a4c49 Mon Sep 17 00:00:00 2001 From: mloubout Date: Mon, 25 Oct 2021 10:58:49 -0400 Subject: [PATCH 1/4] types: add sympy1.9 support for AbstractTensor --- .github/workflows/pytest-core-nompi.yml | 18 ++++++++++---- devito/types/basic.py | 31 +++++++++++++++++++++---- requirements.txt | 2 +- 3 files changed, 40 insertions(+), 11 deletions(-) diff --git a/.github/workflows/pytest-core-nompi.yml b/.github/workflows/pytest-core-nompi.yml index 9b761a75d2..a399d612bc 100644 --- a/.github/workflows/pytest-core-nompi.yml +++ b/.github/workflows/pytest-core-nompi.yml @@ -29,7 +29,7 @@ jobs: name: [ pytest-ubuntu-py37-gcc5-omp, pytest-ubuntu-py38-gcc6-omp, - pytest-ubuntu-py36-gcc7-omp-sympy17, + pytest-ubuntu-py36-gcc7-omp, pytest-ubuntu-py37-gcc7-noomp, pytest-ubuntu-py37-gcc8-omp, pytest-ubuntu-py38-gcc9-omp, @@ -43,48 +43,56 @@ jobs: os: ubuntu-18.04 arch: "gcc-5" language: "openmp" + sympy: 1.8 - name: pytest-ubuntu-py38-gcc6-omp python-version: 3.8 os: ubuntu-18.04 arch: "gcc-6" language: "openmp" + sympy: 1.7 - - name: pytest-ubuntu-py36-gcc7-omp-sympy17 + - name: pytest-ubuntu-py36-gcc7-omp python-version: 3.6 os: ubuntu-18.04 arch: "gcc-7" language: "openmp" + sympy: 1.8 - name: pytest-ubuntu-py37-gcc7-noomp python-version: 3.7 os: ubuntu-18.04 arch: "gcc-7" language: "C" + sympy: 1.8 - name: pytest-ubuntu-py37-gcc8-omp python-version: 3.7 os: ubuntu-18.04 arch: "gcc-8" language: "openmp" + sympy: 1.9 - name: pytest-ubuntu-py38-gcc9-omp python-version: 3.8 os: ubuntu-20.04 arch: "gcc-9" language: "openmp" + sympy: 1.8 - name: pytest-osx-py37-clang-omp python-version: 3.7 os: macos-latest arch: "osx" language: "C" + sympy: 1.8 - name: pytest-docker-py36-gcc-omp python-version: 3.6 os: ubuntu-18.04 arch: "gcc" language: "openmp" + sympy: 1.8 - set: base test-set: 'not adjoint' @@ -140,9 +148,9 @@ jobs: pip install --upgrade pip pip install -e . - - name: Alternative sympy version - if: matrix.name == 'pytest-ubuntu-py36-gcc7-omp-sympy17' - run: pip install sympy==1.7 + - name: Sympy version + if: matrix.name != 'pytest-docker-py36-gcc-omp' + run: pip install sympy==${{matrix.sympy}} - name: Test with pytest run: | diff --git a/devito/types/basic.py b/devito/types/basic.py index 6b66d25dd7..bfb366a6d0 100644 --- a/devito/types/basic.py +++ b/devito/types/basic.py @@ -536,8 +536,8 @@ def _new(cls, *args, **kwargs): newobj = super(AbstractTensor, cls)._new(args[2]) # Filter grid and dimensions - grids = {getattr(c, 'grid', None) for c in newobj._mat} - {None} - dimensions = {d for c in newobj._mat + grids = {getattr(c, 'grid', None) for c in newobj.flat()} - {None} + dimensions = {d for c in newobj.flat() for d in getattr(c, 'dimensions', ())} - {None} # If none of the components are devito objects, returns a sympy Matrix if len(grids) == 0 and len(dimensions) == 0: @@ -551,7 +551,7 @@ def _new(cls, *args, **kwargs): dimensions = tuple(dimensions) # Initialized with constructed object - newobj.__init_finalize__(newobj.rows, newobj.cols, newobj._mat, + newobj.__init_finalize__(newobj.rows, newobj.cols, newobj.flat(), grid=grid, dimensions=dimensions) else: # Initialize components and create new Matrix from standard @@ -562,6 +562,27 @@ def _new(cls, *args, **kwargs): return newobj + @classmethod + def _fromrep(cls, rep): + """ + This the new constructor mechanism for matrices in sympy 1.9. + Standard new object go through `_new` but arithmetic operation directly us + the representation one. + This class method is only accessible from an existing AbstractTensor + that contains a grid. + """ + newobj = super(AbstractTensor, cls)._fromrep(rep) + grids = {getattr(c, 'grid', None) for c in newobj.flat()} - {None} + grid = grids.pop() + newobj.__init_finalize__(newobj.rows, newobj.cols, newobj.flat(), grid=grid) + return newobj + + def flat(self): + try: + return super(AbstractTensor, self).flat() + except AttributeError: + return self._mat + def __init_finalize__(self, *args, **kwargs): pass @@ -583,8 +604,8 @@ def _eval_matrix_mul(self, other): # expected behavior is to produce an n x m matrix of zeros if self.cols != 0 and other.rows != 0: self_cols = self.cols - mat = self._mat - other_mat = other._mat + mat = self.flat() + other_mat = other.flat() for i in range(new_len): row, col = i // other.cols, i % other.cols row_indices = range(self_cols*row, self_cols*(row+1)) diff --git a/requirements.txt b/requirements.txt index 28a8bb3820..b5ed893a21 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,6 +1,6 @@ pip>=9.0.1 numpy>1.16 -sympy>=1.7,<=1.8 +sympy>=1.7,<=1.9 scipy flake8>=2.1.0 nbval From abd125b05a617d70c6b0af1e7e27d6481d514274 Mon Sep 17 00:00:00 2001 From: mloubout Date: Mon, 25 Oct 2021 13:43:55 -0400 Subject: [PATCH 2/4] pickle: handle sympy1.9 new SYmbol pickling --- devito/types/basic.py | 60 +++++++++++++++++++++++++++------------ devito/types/dimension.py | 1 + devito/types/parallel.py | 6 ++-- tests/test_pickle.py | 2 +- 4 files changed, 47 insertions(+), 22 deletions(-) diff --git a/devito/types/basic.py b/devito/types/basic.py index bfb366a6d0..717db4d560 100644 --- a/devito/types/basic.py +++ b/devito/types/basic.py @@ -333,6 +333,10 @@ def _subs(self, old, new, **hints): _pickle_kwargs = ['name', 'dtype', 'is_const'] __reduce_ex__ = Pickable.__reduce_ex__ + def __getnewargs_ex__(self): + return ((self.name,), {**self.assumptions0, + **{i.lstrip('_'): getattr(self, i) for i in self._pickle_kwargs}}) + class Symbol(AbstractSymbol, Cached): @@ -536,20 +540,9 @@ def _new(cls, *args, **kwargs): newobj = super(AbstractTensor, cls)._new(args[2]) # Filter grid and dimensions - grids = {getattr(c, 'grid', None) for c in newobj.flat()} - {None} - dimensions = {d for c in newobj.flat() - for d in getattr(c, 'dimensions', ())} - {None} - # If none of the components are devito objects, returns a sympy Matrix - if len(grids) == 0 and len(dimensions) == 0: + grid, dimensions = newobj._infer_dims(newobj.flat()) + if grid is None and dimensions is None: return sympy.ImmutableDenseMatrix(*args) - elif len(grids) > 0: - dimensions = None - assert len(grids) == 1 - grid = grids.pop() - else: - grid = None - dimensions = tuple(dimensions) - # Initialized with constructed object newobj.__init_finalize__(newobj.rows, newobj.cols, newobj.flat(), grid=grid, dimensions=dimensions) @@ -569,14 +562,39 @@ def _fromrep(cls, rep): Standard new object go through `_new` but arithmetic operation directly us the representation one. This class method is only accessible from an existing AbstractTensor - that contains a grid. + that contains a grid or dimensions. """ newobj = super(AbstractTensor, cls)._fromrep(rep) - grids = {getattr(c, 'grid', None) for c in newobj.flat()} - {None} - grid = grids.pop() - newobj.__init_finalize__(newobj.rows, newobj.cols, newobj.flat(), grid=grid) + grid, dimensions = newobj._infer_dims(newobj.flat()) + try: + # This is needed when `_fromrep` is called directly in 1.9 + # for example with mul. + newobj.__init_finalize__(newobj.rows, newobj.cols, newobj.flat(), + grid=grid, dimensions=dimensions) + except TypeError: + # We can end up here when `_fromrep` is called through new line 553 + # above but input `comps` don't have grid or dimensions. For example + # `test_non_devito_tens` in `test_tensor.py`. + pass return newobj + def _infer_dims(self, mat): + grids = {getattr(c, 'grid', None) for c in mat} - {None} + dimensions = {d for c in mat + for d in getattr(c, 'dimensions', ())} - {None} + # If none of the components are devito objects, returns a sympy Matrix + if len(grids) == 0 and len(dimensions) == 0: + return None, None + elif len(grids) > 0: + dimensions = None + assert len(grids) == 1 + grid = grids.pop() + else: + grid = None + dimensions = tuple(dimensions) + + return grid, dimensions + def flat(self): try: return super(AbstractTensor, self).flat() @@ -605,7 +623,10 @@ def _eval_matrix_mul(self, other): if self.cols != 0 and other.rows != 0: self_cols = self.cols mat = self.flat() - other_mat = other.flat() + try: + other_mat = other.flat() + except AttributeError: + other_mat = other._mat for i in range(new_len): row, col = i // other.cols, i % other.cols row_indices = range(self_cols*row, self_cols*(row+1)) @@ -1067,6 +1088,7 @@ def __getitem__(self, index): # Pickling support _pickle_kwargs = ['name', 'dtype', 'halo', 'padding'] __reduce_ex__ = Pickable.__reduce_ex__ + __getnewargs_ex__ = Pickable.__getnewargs_ex__ @property def _pickle_reconstruct(self): @@ -1136,6 +1158,7 @@ def function(self): # Pickling support _pickle_args = ['name', 'dtype'] __reduce_ex__ = Pickable.__reduce_ex__ + __getnewargs_ex__ = Pickable.__getnewargs_ex__ class Object(AbstractObject, ArgProvider): @@ -1264,6 +1287,7 @@ def __getitem__(self, indices, **kwargs): # Pickling support _pickle_kwargs = ['label', 'shape', 'function'] __reduce_ex__ = Pickable.__reduce_ex__ + __getnewargs_ex__ = Pickable.__getnewargs_ex__ class BoundSymbol(AbstractSymbol): diff --git a/devito/types/dimension.py b/devito/types/dimension.py index c952df4fff..ce2b8a2e25 100644 --- a/devito/types/dimension.py +++ b/devito/types/dimension.py @@ -310,6 +310,7 @@ def _arg_check(self, args, size, interval): _pickle_args = ['name'] _pickle_kwargs = ['spacing'] __reduce_ex__ = Pickable.__reduce_ex__ + __getnewargs_ex__ = Pickable.__getnewargs_ex__ class BasicDimension(Dimension, Symbol): diff --git a/devito/types/parallel.py b/devito/types/parallel.py index 021b565d15..148b8ee9b7 100644 --- a/devito/types/parallel.py +++ b/devito/types/parallel.py @@ -33,7 +33,7 @@ class NThreadsBase(Scalar): is_Input = True is_PerfKnob = True - def __new__(cls, **kwargs): + def __new__(cls, *args, **kwargs): kwargs.setdefault('name', cls.name) kwargs['is_const'] = True return super().__new__(cls, **kwargs) @@ -71,7 +71,7 @@ class NPThreads(NThreadsBase): name = 'npthreads' - def __new__(cls, **kwargs): + def __new__(cls, *args, **kwargs): obj = super().__new__(cls, **kwargs) # Size of the thread pool @@ -388,7 +388,7 @@ class DeviceSymbol(Scalar): is_Input = True is_PerfKnob = True - def __new__(cls, **kwargs): + def __new__(cls, *args, **kwargs): kwargs['name'] = cls.name kwargs['is_const'] = True return super().__new__(cls, **kwargs) diff --git a/tests/test_pickle.py b/tests/test_pickle.py index f0372ef78b..c39173aa4b 100644 --- a/tests/test_pickle.py +++ b/tests/test_pickle.py @@ -579,7 +579,7 @@ def test_mpi_fullmode_objects(): for n, i in enumerate(new_obj.owned): d, v = list(i[1].items())[0] assert d is new_obj.arguments[n] - assert v[0] is d.symbolic_min + assert v[0] == d.symbolic_min assert v[1] == Min(d.symbolic_max, d.symbolic_min) From d41129babd237d33802642120c8e22a6fa42dc78 Mon Sep 17 00:00:00 2001 From: Mathias Louboutin Date: Tue, 26 Oct 2021 09:03:44 -0400 Subject: [PATCH 3/4] types: prevent unpickling to create a different hash --- .github/workflows/pytest-core-nompi.yml | 6 +--- devito/types/basic.py | 38 +++++++++++++------------ tests/test_pickle.py | 2 +- 3 files changed, 22 insertions(+), 24 deletions(-) diff --git a/.github/workflows/pytest-core-nompi.yml b/.github/workflows/pytest-core-nompi.yml index a399d612bc..49f42f1e6d 100644 --- a/.github/workflows/pytest-core-nompi.yml +++ b/.github/workflows/pytest-core-nompi.yml @@ -19,7 +19,6 @@ jobs: DEVITO_ARCH: "${{ matrix.arch }}" DEVITO_LANGUAGE: ${{ matrix.language }} OMP_NUM_THREADS: 2 - PYTHON_VERSION: "${{ matrix.python-version }}" strategy: # Prevent all build to stop if a single one fails @@ -147,10 +146,7 @@ jobs: run: | pip install --upgrade pip pip install -e . - - - name: Sympy version - if: matrix.name != 'pytest-docker-py36-gcc-omp' - run: pip install sympy==${{matrix.sympy}} + pip install sympy==${{matrix.sympy}} - name: Test with pytest run: | diff --git a/devito/types/basic.py b/devito/types/basic.py index 717db4d560..ac75076a70 100644 --- a/devito/types/basic.py +++ b/devito/types/basic.py @@ -220,6 +220,10 @@ class AbstractSymbol(sympy.Symbol, Basic, Pickable, Evaluable): def _filter_assumptions(cls, **kwargs): """Extract sympy.Symbol-specific kwargs.""" assumptions = {} + # pop predefined assumptions + for key in ('real', 'imaginary', 'commutative'): + kwargs.pop(key, None) + # extract sympy.Symbol-specific kwargs for i in list(kwargs): if i in _assume_rules.defined_facts: assumptions[i] = kwargs.pop(i) @@ -334,7 +338,7 @@ def _subs(self, old, new, **hints): __reduce_ex__ = Pickable.__reduce_ex__ def __getnewargs_ex__(self): - return ((self.name,), {**self.assumptions0, + return (tuple(), {**self.assumptions0, **{i.lstrip('_'): getattr(self, i) for i in self._pickle_kwargs}}) @@ -372,22 +376,23 @@ def _cache_key(cls, *args, **kwargs): return frozendict(key) def __new__(cls, *args, **kwargs): - key = cls._cache_key(*args, **kwargs) + assumptions, kwargs = cls._filter_assumptions(**kwargs) + key = cls._cache_key(*args, **{**assumptions, **kwargs}) obj = cls._cache_get(key) if obj is not None: return obj # Not in cache. Create a new Symbol via sympy.Symbol - name = kwargs.get('name') or args[0] - assumptions, kwargs = cls._filter_assumptions(**kwargs) + args = list(args) + name = kwargs.pop('name', None) or args.pop(0) # Note: use __xnew__ to bypass sympy caching newobj = sympy.Symbol.__xnew__(cls, name, **assumptions) # Initialization newobj._dtype = cls.__dtype_setup__(**kwargs) - newobj.__init_finalize__(*args, **kwargs) + newobj.__init_finalize__(name, *args, **kwargs) # Store new instance in symbol cache Cached.__init__(newobj, key) @@ -540,7 +545,7 @@ def _new(cls, *args, **kwargs): newobj = super(AbstractTensor, cls)._new(args[2]) # Filter grid and dimensions - grid, dimensions = newobj._infer_dims(newobj.flat()) + grid, dimensions = newobj._infer_dims() if grid is None and dimensions is None: return sympy.ImmutableDenseMatrix(*args) # Initialized with constructed object @@ -559,28 +564,28 @@ def _new(cls, *args, **kwargs): def _fromrep(cls, rep): """ This the new constructor mechanism for matrices in sympy 1.9. - Standard new object go through `_new` but arithmetic operation directly us - the representation one. + Standard new object go through `_new` but arithmetic operations directly use + the representation based one. This class method is only accessible from an existing AbstractTensor that contains a grid or dimensions. """ newobj = super(AbstractTensor, cls)._fromrep(rep) - grid, dimensions = newobj._infer_dims(newobj.flat()) + grid, dimensions = newobj._infer_dims() try: # This is needed when `_fromrep` is called directly in 1.9 # for example with mul. newobj.__init_finalize__(newobj.rows, newobj.cols, newobj.flat(), grid=grid, dimensions=dimensions) except TypeError: - # We can end up here when `_fromrep` is called through new line 553 - # above but input `comps` don't have grid or dimensions. For example + # We can end up here when `_fromrep` is called through the default _new + # when input `comps` don't have grid or dimensions. For example # `test_non_devito_tens` in `test_tensor.py`. pass return newobj - def _infer_dims(self, mat): - grids = {getattr(c, 'grid', None) for c in mat} - {None} - dimensions = {d for c in mat + def _infer_dims(self): + grids = {getattr(c, 'grid', None) for c in self.flat()} - {None} + dimensions = {d for c in self.flat() for d in getattr(c, 'dimensions', ())} - {None} # If none of the components are devito objects, returns a sympy Matrix if len(grids) == 0 and len(dimensions) == 0: @@ -597,7 +602,7 @@ def _infer_dims(self, mat): def flat(self): try: - return super(AbstractTensor, self).flat() + return super().flat() except AttributeError: return self._mat @@ -1088,7 +1093,6 @@ def __getitem__(self, index): # Pickling support _pickle_kwargs = ['name', 'dtype', 'halo', 'padding'] __reduce_ex__ = Pickable.__reduce_ex__ - __getnewargs_ex__ = Pickable.__getnewargs_ex__ @property def _pickle_reconstruct(self): @@ -1158,7 +1162,6 @@ def function(self): # Pickling support _pickle_args = ['name', 'dtype'] __reduce_ex__ = Pickable.__reduce_ex__ - __getnewargs_ex__ = Pickable.__getnewargs_ex__ class Object(AbstractObject, ArgProvider): @@ -1287,7 +1290,6 @@ def __getitem__(self, indices, **kwargs): # Pickling support _pickle_kwargs = ['label', 'shape', 'function'] __reduce_ex__ = Pickable.__reduce_ex__ - __getnewargs_ex__ = Pickable.__getnewargs_ex__ class BoundSymbol(AbstractSymbol): diff --git a/tests/test_pickle.py b/tests/test_pickle.py index c39173aa4b..f0372ef78b 100644 --- a/tests/test_pickle.py +++ b/tests/test_pickle.py @@ -579,7 +579,7 @@ def test_mpi_fullmode_objects(): for n, i in enumerate(new_obj.owned): d, v = list(i[1].items())[0] assert d is new_obj.arguments[n] - assert v[0] == d.symbolic_min + assert v[0] is d.symbolic_min assert v[1] == Min(d.symbolic_max, d.symbolic_min) From f35ac8aba9e9a27814ffa8ec7af36a6bb019f030 Mon Sep 17 00:00:00 2001 From: Fabio Luporini Date: Thu, 28 Oct 2021 10:05:29 +0200 Subject: [PATCH 4/4] compiler: Polish AbstractSymbol pickling --- devito/tools/abc.py | 2 +- devito/types/basic.py | 5 +++-- devito/types/constant.py | 6 +++++- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/devito/tools/abc.py b/devito/tools/abc.py index 9ce6837178..2a8905979c 100644 --- a/devito/tools/abc.py +++ b/devito/tools/abc.py @@ -147,7 +147,7 @@ def __reduce_ex__(self, proto): def __getnewargs_ex__(self): return (tuple(getattr(self, i) for i in self._pickle_args), - {i.lstrip('_'): getattr(self, i) for i in self._pickle_kwargs}) + {i: getattr(self, i) for i in self._pickle_kwargs}) class Singleton(type): diff --git a/devito/types/basic.py b/devito/types/basic.py index ac75076a70..e1b54a5006 100644 --- a/devito/types/basic.py +++ b/devito/types/basic.py @@ -338,8 +338,9 @@ def _subs(self, old, new, **hints): __reduce_ex__ = Pickable.__reduce_ex__ def __getnewargs_ex__(self): - return (tuple(), {**self.assumptions0, - **{i.lstrip('_'): getattr(self, i) for i in self._pickle_kwargs}}) + args, kwargs = Pickable.__getnewargs_ex__(self) + kwargs.update(self.assumptions0) + return args, kwargs class Symbol(AbstractSymbol, Cached): diff --git a/devito/types/constant.py b/devito/types/constant.py index c63bab1e42..0ae8e97042 100644 --- a/devito/types/constant.py +++ b/devito/types/constant.py @@ -55,6 +55,10 @@ def __dtype_setup__(cls, **kwargs): def is_const(self): return True + @property + def value(self): + return self.data + @property def data(self): """The value of the data object, as a scalar (int, float, ...).""" @@ -108,4 +112,4 @@ def _arg_check(self, args, intervals): except AttributeError: pass - _pickle_kwargs = DataSymbol._pickle_kwargs + ['_value'] + _pickle_kwargs = DataSymbol._pickle_kwargs + ['value']