-
Notifications
You must be signed in to change notification settings - Fork 257
dsl: misc patches from recent updates (interp, sympy args) #3002
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
fe8961c
4f52a11
ccf9b73
50dd495
3101801
d65dab1
8f12021
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1008,20 +1008,24 @@ def _arg_defaults(self, alias=None, estimate_memory=False): | |
| if estimate_memory: | ||
| return defaults | ||
| key = alias or self | ||
| coords = defaults.get(key.coordinates.name, key.coordinates.data) | ||
| coords = defaults.get(key.coordinates.name, self.coordinates.data) | ||
| defaults.update(key.interpolator._arg_defaults(coords=coords, | ||
| sfunc=key)) | ||
| sfunc=self)) | ||
| return defaults | ||
|
|
||
| def _arg_values(self, estimate_memory=False, **kwargs): | ||
| values = super()._arg_values(estimate_memory=estimate_memory, **kwargs) | ||
| if estimate_memory: | ||
| return values | ||
|
|
||
| # Resolve the runtime grid origin (honours `o_x`/`o_y`/... overrides) | ||
| # and hand it to the interpolator so tables reflect the actual frame | ||
| # of reference used by the kernel. | ||
| # `super` has already tabulated through `_arg_defaults`, in the frame | ||
| # of whichever object supplied the runtime values. Only an explicit | ||
| # `o_x`/`o_y`/... override moves that frame again, and the tables then | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "frame"? |
||
| # have to be rebuilt against it. | ||
| onames = [o.name for o in self.grid.origin_symbols] | ||
| if not any(n in kwargs for n in onames): | ||
| return values | ||
|
|
||
| origin = tuple(kwargs.get(n, o) for n, o in | ||
| zip(onames, self.grid.origin, strict=True)) | ||
| coords = values.get(self.coordinates.name, self.coordinates.data) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,12 +24,15 @@ def staggering(stagg, i, j, d, dims): | |
| if stagg is None: | ||
| # No input | ||
| return NODE if i == j else (d, dims[j]) | ||
| elif isinstance(stagg, MatrixBase): | ||
| # From rebuild/tensor property. Indexed as a sympy Matrix. Note that this | ||
| # may be a plain Matrix rather than an AbstractTensor, as rebuilding a | ||
| # tensor component-wise downgrades it when the components aren't Devito | ||
| # objects, which is the case for a Matrix of `Staggering` | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But doesn't this matrix contain Devito dimensions or NODE usually? I suppose this is for the tuple case as in
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The matrix is not a devito object, not its elements. If it doesn't contain any dimension (so the staggering) it's plain sympy matrix |
||
| return stagg[i, j] | ||
| elif isinstance(stagg, (tuple, list)): | ||
| # User input as list or tuple | ||
| return stagg[i][j] | ||
| elif isinstance(stagg, AbstractTensor): | ||
| # From rebuild/tensor property. Indexed as a sympy Matrix | ||
| return stagg[i, j] | ||
|
|
||
|
|
||
| class TensorFunction(AbstractTensor): | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1077,6 +1077,37 @@ def test_position(self, shape): | |
|
|
||
| assert(np.allclose(rec.data, rec1.data, atol=1e-5)) | ||
|
|
||
| @pytest.mark.parametrize('interpolation,r', [('linear', 1), ('sinc', 4)]) | ||
| def test_position_override_grid(self, interpolation, r): | ||
| """ | ||
| Inject through an Operator built on a grid whose origin differs from | ||
| the one it is applied to, as when an Operator compiled against one | ||
| model is applied to another. The point must land where the runtime | ||
| origin puts it, not the compile-time one. | ||
| """ | ||
| shape, spacing, coord = (41, 41), (10., 10.), 120. | ||
| extent = tuple((s - 1) * h for s, h in zip(shape, spacing, strict=True)) | ||
| kw = dict(interpolation=interpolation, r=r) | ||
|
|
||
| def setup(origin): | ||
| grid = Grid(shape=shape, extent=extent, origin=origin) | ||
| u = TimeFunction(name='u', grid=grid, space_order=8) | ||
| src = SparseTimeFunction(name='src', grid=grid, npoint=1, nt=2, **kw) | ||
|
Comment on lines
+1094
to
+1095
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nitpick - no need for these to be time-dependent
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it mimicks the erroring case from the recipes, why it's time dependent |
||
| src.coordinates.data[0, :] = coord | ||
| src.data[:] = 1. | ||
| return u, src | ||
|
|
||
| u_build, src_build = setup((0., 0.)) | ||
| op = Operator(src_build.inject(field=u_build.forward, expr=src_build)) | ||
|
|
||
| shift = -100. | ||
| u, src = setup((shift, shift)) | ||
| op.apply(time_M=0, u=u, src=src) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it worth also testing directly overriding
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The plain origin override is tested just above. |
||
|
|
||
| expected = tuple(int((coord - shift) / h) for h in spacing) | ||
| peak = np.unravel_index(np.argmax(np.abs(u.data)), u.data.shape)[1:] | ||
| assert peak == expected | ||
|
|
||
| def test_sparse_first(self): | ||
| """ | ||
| Tests custom sprase function with sparse dimension as first index. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,7 @@ | |
| ) | ||
| from devito.symbolics import retrieve_derivatives | ||
| from devito.types import NODE | ||
| from devito.types.utils import Staggering | ||
|
|
||
|
|
||
| def dimify(dimensions): | ||
|
|
@@ -528,6 +529,25 @@ def test_diag_sympified_zeros(func1): | |
| assert all(isinstance(c, sympy.Expr) for c in f2.flat()) | ||
|
|
||
|
|
||
| @pytest.mark.parametrize('func1', [TensorFunction, TensorTimeFunction, | ||
| VectorFunction, VectorTimeFunction]) | ||
| def test_staggered_attribute_roundtrip(func1): | ||
| """ | ||
| Accessing an attribute rebuilds the tensor component-wise, which must not | ||
| sympify a `Staggering` away, otherwise it can no longer be fed back as the | ||
| `staggered` kwarg. | ||
| """ | ||
| grid = Grid(tuple([5]*3)) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nitpick: |
||
| f1 = func1(name="f1", grid=grid, time_order=1) | ||
|
|
||
| stagg = f1.staggered | ||
| assert all(isinstance(s, Staggering) for s in stagg.flat()) | ||
|
|
||
| f2 = func1(name="f2", grid=grid, time_order=1, staggered=stagg) | ||
| assert all(c1.staggered == c2.staggered | ||
| for c1, c2 in zip(f1.flat(), f2.flat(), strict=True)) | ||
|
|
||
|
|
||
| def test_non_expr_components(): | ||
| """ | ||
| A tensor may legitimately hold non-`Expr` components, which sympy deprecates | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this doesn't seem correct to me?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why? key is the one used to build the operator, self is the runtime one