dsl: misc patches from recent updates (interp, sympy args) - #3002
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3002 +/- ##
==========================================
+ Coverage 83.15% 83.63% +0.48%
==========================================
Files 257 257
Lines 54185 54221 +36
Branches 4626 4629 +3
==========================================
+ Hits 45056 45349 +293
+ Misses 8321 8075 -246
+ Partials 808 797 -11
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
e7893c2 to
3ffab6b
Compare
mloubout
changed the title
| return defaults | ||
| key = alias or self | ||
| coords = defaults.get(key.coordinates.name, key.coordinates.data) | ||
| coords = defaults.get(key.coordinates.name, self.coordinates.data) |
There was a problem hiding this comment.
this doesn't seem correct to me?
There was a problem hiding this comment.
why? key is the one used to build the operator, self is the runtime one
| # 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 |
| # 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` |
There was a problem hiding this comment.
But doesn't this matrix contain Devito dimensions or NODE usually? I suppose this is for the tuple case as in staggered=(x, y) on a particular field?
There was a problem hiding this comment.
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 sympy.ImmutableDenseMatrix._fromrep(newobj._rep)
| u = TimeFunction(name='u', grid=grid, space_order=8) | ||
| src = SparseTimeFunction(name='src', grid=grid, npoint=1, nt=2, **kw) |
There was a problem hiding this comment.
Nitpick - no need for these to be time-dependent
There was a problem hiding this comment.
it mimicks the erroring case from the recipes, why it's time dependent
|
|
||
| shift = -100. | ||
| u, src = setup((shift, shift)) | ||
| op.apply(time_M=0, u=u, src=src) |
There was a problem hiding this comment.
Is it worth also testing directly overriding o_x, o_y, etc? I suppose users don't do it very often, but still...
There was a problem hiding this comment.
The plain origin override is tested just above.
| sympify a `Staggering` away, otherwise it can no longer be fed back as the | ||
| `staggered` kwarg. | ||
| """ | ||
| grid = Grid(tuple([5]*3)) |
There was a problem hiding this comment.
Nitpick: grid = Grid((5,) * 3)
An Operator compiled against one model and applied to another overrides every symbol, sparse ones included, and the kernel then runs in the override's frame. `_arg_values` took the coordinates from the override but the origin from `self`, tabulating real coordinates against the compile-time frame. Every point landed off by the difference between the two origins, silently, since sources and receivers shift together.
Injects through an Operator built on a grid whose origin differs from the one it is applied to. The linear path regressed here; sinc computes its positions from the runtime `o_x` and did not.
`_arg_defaults` is already handed the runtime object as `self` and the compile-time symbols as `alias`, like the rest of the `_arg_*` protocol, so the frame to tabulate in was there all along. Take the names from `alias` and the data and Grid from `self`, rather than looking the override up in kwargs from `_arg_values`. `_arg_values` now only rebuilds the tables for an explicit `o_x`/`o_y` override, instead of redoing on every call what `_arg_defaults` has already tabulated correctly.
It sits next to `test_position`, which covers the same origin shift through an explicit `o_x`, rather than carrying a class of its own.
e50ed47 to
8f12021
Compare
3 participants
b=input to rotated lap function for born modeling