-
Notifications
You must be signed in to change notification settings - Fork 229
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
api: cleanup FD tools and support zeroth order derivative #2368
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2368 +/- ##
=======================================
Coverage 86.75% 86.75%
=======================================
Files 233 233
Lines 43720 43727 +7
Branches 8078 8072 -6
=======================================
+ Hits 37929 37937 +8
Misses 5080 5080
+ Partials 711 710 -1 ☔ View full report in Codecov by Sentry. |
fmean = .5 * (f + f._subs(y, y + y.spacing)) | ||
drv1x0 = drv1(x0={y: y+y.spacing/2}).evaluate | ||
|
||
assert simplify(fmean.dx2.evaluate - drv1x0) == 0 |
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.
It would also be a good idea to check equivalence between the Derivative(f, y, deriv_order=0, x0=y+y.spacing/2)
and Derivative(f, (y, 0), x0={y: y+y.spacing/2})
specifications.
There should also be tests for Derivative(f, (x, 0), (y, 0), x0={x: x+x.spacing/2, y: y+y.spacing/2})
and Derivative(f, x, y,, deriv_order=(0, 0), x0={x: x+x.spacing/2, y: y+y.spacing/2})
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.
Added
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
Great work! What about a unit test interpolating an analytic function like a sin or cos? |
There is a unit test checking coefficient correctness. |
e83e6e3
to
2fa1537
Compare
@@ -831,7 +831,7 @@ def test_solve(self, operate_on_empty_cache): | |||
# created by the finite difference (u.dt, u.dx2). We would have had | |||
# three extra references to u(t + dt), u(x - h_x) and u(x + h_x). | |||
# But this is not the case anymore! | |||
assert len(_SymbolCache) == 11 | |||
assert len(_SymbolCache) == 12 |
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.
Note: This is because now every order uses a StencilDImension even for space_order=1
while we were only giving the list of indices for that case before.
7da8195
to
7703a65
Compare
358b228
to
18d1a94
Compare
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.
two minor comments, addressable in a separate PR, because this is good to go in my opinion
No description provided.