api: Revamp interpolation/injection #2128

FabioLuporini · 2023-05-15T13:28:36Z

Revamp sparse function.

Improve hierachy to include PrecomputedSparseFunction
Add MPI support to PrecomputedSparseFunction
Revamp interpolation to ease implementation of new Weighted interpolation
Introduce multi-inject/interp so the user can inject into multiple fields at once

Issue fixed:

codecov · 2023-05-15T13:35:36Z

Codecov Report

Merging #2128 (4d9abed) into master (7763d03) will decrease coverage by 0.01%.
The diff coverage is 95.19%.

@@            Coverage Diff             @@
##           master    #2128      +/-   ##
==========================================
- Coverage   87.06%   87.06%   -0.01%     
==========================================
  Files         228      228              
  Lines       40299    40539     +240     
  Branches     7357     7412      +55     
==========================================
+ Hits        35088    35295     +207     
- Misses       4625     4642      +17     
- Partials      586      602      +16

Files Changed	Coverage Δ
devito/tools/algorithms.py	`91.89% <ø> (ø)`
tests/test_gpu_common.py	`1.44% <0.00%> (ø)`
tests/test_gpu_openacc.py	`5.69% <ø> (ø)`
tests/test_gpu_openmp.py	`4.23% <ø> (ø)`
tests/test_ir.py	`94.54% <ø> (ø)`
tests/test_msparse.py	`99.17% <0.00%> (-0.83%)`	⬇️
devito/passes/iet/languages/openacc.py	`62.03% <50.00%> (+0.56%)`	⬆️
devito/types/basic.py	`92.17% <58.82%> (-1.08%)`	⬇️
devito/tools/data_structures.py	`72.67% <75.00%> (+<0.01%)`	⬆️
devito/types/dimension.py	`93.32% <90.00%> (-1.01%)`	⬇️
... and 25 more

... and 3 files with indirect coverage changes

mloubout

Sorry bunch of comments. I know this hasn't been touched in quite a few yers ut yes is needed to revamp and cleanup

devito/types/sparse.py

devito/types/grid.py

tests/test_pickle.py

tests/test_dle.py

devito/operations/interpolators.py

FabioLuporini · 2023-05-23T07:02:02Z

devito/operations/interpolators.py

+        return self.sfunction._point_symbols
+
+    @property
+    def _gdim(self):


imho, just "dimensions" as we do everywhere else

No these are not the dimensions these are the dimensions of the underlying grid which are different than the dimension of the Sparse object and associated interpolator

then _gdims at least (make it plural)

devito/types/sparse.py

FabioLuporini · 2023-05-23T07:25:22Z

devito/types/sparse.py

+        """
+        if self.gridpoints is not None:
+            ddim = self.gridpoints.dimensions[-1]
+            return tuple([self.gridpoints._subs(ddim, di) for di in range(self.grid.dim)])


I think I prefer n for numbers instead of di (or d) which is typically used for Dimensions instead

I like to use (namei, name) in zip(...) so that's why I have di since the "name" is d but can change to n

devito/types/sparse.py

georgebisbas · 2023-05-23T11:43:25Z

devito/operations/interpolators.py

+        return self.grid.dimensions
+
+    def implicit_dims(self, implicit_dims):
+        return as_tuple(implicit_dims) + self.sfunction.dimensions


Is it ok to have the same name of the function and the arg here?

georgebisbas · 2023-05-23T11:43:45Z

devito/operations/interpolators.py

+        return self.sfunction._point_symbols
+
+    @property
+    def _gdim(self):


devito/operations/interpolators.py

georgebisbas · 2023-05-23T11:59:17Z

devito/operations/interpolators.py

@@ -230,7 +232,7 @@ def interpolate(self, expr, offset=0, increment=False, self_subs={},
            interpolation expression, but that should be honored when constructing
            the operator.
        """
-        implicit_dims = as_tuple(implicit_dims) + self.sfunction.dimensions
+        implicit_dims = self.implicit_dims(implicit_dims)


Yeah, not sure I like much this ~~I want to get the implicit_dims of the implicit_dims~~

also "implicit_dims" should be a private method.

Maybe _augment_implicit_dims?

devito/operations/interpolators.py

…ceptr out of acc loop

mloubout · 2023-08-29T16:05:12Z

As discussed on slack, issue opened for SubFunction and merging, SUbFunction cleanup top of the list next.

georgebisbas · 2023-09-05T17:06:39Z

devito/ir/equations/algorithms.py


        # StencilDimensions are lowered subsequently through special compiler
        # passes, so they can be ignored here
        relation = tuple(d for d in relation if not d.is_Stencil)

-        return relation
+        # Make sure indices appearance order satisfies "parent" order. Foe example


For example

georgebisbas · 2023-09-05T17:52:44Z

devito/types/sparse.py

+        # Rebuild subfunctions first to avoid new data creation as we have to use `_data`
+        # as a reconstruction kwargs to avoid the circular dependency
+        # with the parent in SubFunction
+        # This is also necessary to avoid shaoe issue in the SubFunction with mpi


FabioLuporini · 2023-09-06T07:40:56Z

tests/test_gradient.py

@@ -15,6 +15,7 @@
 class TestGradient(object):

    @skipif(['chkpnt', 'cpu64-icc'])
+    @switchconfig(safe_math=True)


is it correct to say that with the new interpolation loop nest structure we're somehow losing some accuracy?

how significant is the failure without safe_math?

It's quite small, about 1e-5.

georgebisbas · 2023-09-06T10:06:34Z

devito/types/sparse.py

@@ -402,7 +724,7 @@ class SparseFunction(AbstractSparseFunction):
        Discretisation order for space derivatives. Defaults to 0.
    shape : tuple of ints, optional
        Shape of the object. Defaults to ``(npoint,)``.
-    dimensions : tuple of Dimension, optional
+    Dimensions : tuple of Dimension, optional


georgebisbas · 2023-09-06T10:09:43Z

devito/types/sparse.py

-        Shape of the object. Defaults to ``(npoint,)``.
-    dimensions : tuple of Dimension, optional
+        Shape of the object. Defaults to `(npoint,)`.
+    Dimensions : tuple of Dimension, optional


dimensions?

FabioLuporini added WIP Still work in progress API api (symbolics, types, ...) labels May 15, 2023

FabioLuporini requested a review from mloubout May 15, 2023 13:28

mloubout reviewed May 15, 2023

View reviewed changes

devito/types/sparse.py Outdated Show resolved Hide resolved

devito/types/sparse.py Outdated Show resolved Hide resolved

devito/types/sparse.py Outdated Show resolved Hide resolved

devito/types/sparse.py Outdated Show resolved Hide resolved

mloubout force-pushed the revamp-prec-sparse-func branch 15 times, most recently from 31f3850 to 1f61f3a Compare May 19, 2023 18:28

mloubout force-pushed the revamp-prec-sparse-func branch from 35f331d to bf3cd61 Compare May 22, 2023 17:16

mloubout removed the WIP Still work in progress label May 22, 2023

mloubout force-pushed the revamp-prec-sparse-func branch 4 times, most recently from e789aa7 to 2d7778d Compare May 22, 2023 22:29

FabioLuporini commented May 23, 2023

View reviewed changes

georgebisbas reviewed May 23, 2023

View reviewed changes

mloubout force-pushed the revamp-prec-sparse-func branch 2 times, most recently from 3be6f54 to a945ed4 Compare May 23, 2023 18:10

mloubout added 7 commits August 23, 2023 12:01

compiler: prevent inner custom dimension to be parallel to avoid devi…

351b4c3

…ceptr out of acc loop

CI: add test for and fixes #920

b51c488

mpi: drop halospots with empty iters

f2fc9b1

CI: add large radius precomputed interp test

84335d6

api: cleanup based on first review

0abe77b

api: move interp coefficient inside most inner loop

1aa35df

ci: new flake8 version fix

d7d2040

mloubout force-pushed the revamp-prec-sparse-func branch 4 times, most recently from f89d454 to 2752814 Compare August 25, 2023 17:28

mloubout force-pushed the revamp-prec-sparse-func branch 3 times, most recently from ea78099 to 0f08bfb Compare August 31, 2023 20:41

dsl: tweak reconstruction of sparse functions

92a60b8

mloubout force-pushed the revamp-prec-sparse-func branch 2 times, most recently from 6014418 to 3544a3f Compare September 5, 2023 16:59

georgebisbas reviewed Sep 5, 2023

View reviewed changes

mloubout force-pushed the revamp-prec-sparse-func branch 2 times, most recently from 590fda9 to e03a51a Compare September 6, 2023 02:13

FabioLuporini commented Sep 6, 2023

View reviewed changes

georgebisbas reviewed Sep 6, 2023

View reviewed changes

mloubout force-pushed the revamp-prec-sparse-func branch 2 times, most recently from 056b6ab to ab03930 Compare September 6, 2023 12:40

compiler: fix dimension sort determinism

4d9abed

mloubout force-pushed the revamp-prec-sparse-func branch from ab03930 to 4d9abed Compare September 6, 2023 13:41

mloubout merged commit 40975d2 into master Sep 6, 2023
32 checks passed

mloubout deleted the revamp-prec-sparse-func branch September 6, 2023 21:41

mloubout mentioned this pull request Sep 8, 2023

devito creates new time blocks when using conditional dimensions as implicit #2170

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

api: Revamp interpolation/injection #2128

api: Revamp interpolation/injection #2128

FabioLuporini commented May 15, 2023 •

edited by mloubout

Loading

codecov bot commented May 15, 2023 •

edited

Loading

mloubout left a comment

FabioLuporini May 23, 2023

georgebisbas May 23, 2023

mloubout May 23, 2023

FabioLuporini Jul 24, 2023

FabioLuporini May 23, 2023

mloubout Jun 9, 2023

georgebisbas May 23, 2023

georgebisbas May 23, 2023

georgebisbas May 23, 2023

FabioLuporini May 28, 2023

mloubout commented Aug 29, 2023

georgebisbas Sep 5, 2023

georgebisbas Sep 5, 2023

FabioLuporini Sep 6, 2023

mloubout Sep 6, 2023

georgebisbas Sep 6, 2023

georgebisbas Sep 6, 2023

api: Revamp interpolation/injection #2128

api: Revamp interpolation/injection #2128

Conversation

FabioLuporini commented May 15, 2023 • edited by mloubout Loading

codecov bot commented May 15, 2023 • edited Loading

Codecov Report

mloubout left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mloubout commented Aug 29, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FabioLuporini commented May 15, 2023 •

edited by mloubout

Loading

codecov bot commented May 15, 2023 •

edited

Loading