api: make sure staggered FD interpolates shifts #2439
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2439 +/- ##
=======================================
Coverage 87.01% 87.02%
=======================================
Files 236 236
Lines 44901 44938 +37
Branches 8361 8369 +8
=======================================
+ Hits 39071 39107 +36
- Misses 5101 5102 +1
Partials 729 729 ☔ View full report in Codecov by Sentry. |
mloubout
force-pushed
the
fd-interp-x0
branch
2 times, most recently
from
7c92da6 to
0f557eb
Compare
| # Step 1: Evaluate derivatives within expression | ||
| # Step 1: Evaluate non-derivative x0. We currently enforce a simple 2nd order | ||
| # interpolation to avoid very expensive finite differences on top of it. | ||
| x0_interp = {k: v for k, v in self.x0.items() if k not in self.dims |
There was a problem hiding this comment.
this is hardly readable, I'd turn it into a plain for loop and separate statements
also, k -> d
| - 1: Evaluate derivatives within the expression. For example given | ||
| - 1: Interpolate non-derivative shifts. | ||
| I.e u[x, y].dx(x0={y: y + h_y/2}) requires to interpolate `u` in `y` |
| """ | ||
| # Step 1: Evaluate derivatives within expression | ||
| # Step 1: Evaluate non-derivative x0. We currently enforce a simple 2nd order | ||
| # interpolation to avoid very expensive finite differences on top of it. |
mloubout
force-pushed
the
fd-interp-x0
branch
5 times, most recently
from
101b343 to
c18c25b
Compare
| x0_expr = {d: v for d, v in x0.items() if v is not expr.indices_ref[d]} | ||
| if x0_expr and not expr.is_parameter: | ||
| dims = tuple((d, 0) for d in x0_expr) | ||
| fd_o = tuple([2]*len(dims)) |
FabioLuporini
left a comment
FabioLuporini
left a comment
There was a problem hiding this comment.
I see some DSE tests are failing due to a different op count. Why is the number of operations decreasing?
| return expr | ||
|
|
||
|
|
||
| @interp_for_fd.register(Add) |
There was a problem hiding this comment.
why not just the default behaviour? in the absence of args return expr otherwise this?
sounds safer
There was a problem hiding this comment.
Made general to sympy.Expr
Slightly different indices in some rare cases |
mloubout
force-pushed
the
fd-interp-x0
branch
2 times, most recently
from
a185b83 to
6b8446c
Compare
mloubout
force-pushed
the
fd-interp-x0
branch
2 times, most recently
from
cae161d to
7e372da
Compare
Fix an FD hack to use proper interpolation.